Skip to content

PHP 8.5: Prevent deprecation notices for curl_close#947

Merged
schlessera merged 1 commit intoWordPress:developfrom
TobiasBg:patch-1
Nov 20, 2025
Merged

PHP 8.5: Prevent deprecation notices for curl_close#947
schlessera merged 1 commit intoWordPress:developfrom
TobiasBg:patch-1

Conversation

@TobiasBg
Copy link
Copy Markdown
Contributor

@TobiasBg TobiasBg commented Sep 1, 2025

curl_close is deprecated in PHP 8.5+, and hasn't been doing anything since PHP 8.0.

To prevent deprecation warnings it should therefore be called on older versions of PHP only.

See https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_no-op_functions_from_the_resource_to_object_conversion.

Pull Request Type

  • I have checked there is no other PR open for the same change.

This is a:

  • Bug fix
  • New feature
  • Documentation improvement
  • Code quality improvement

Quality assurance

  • This change does NOT contain a breaking change (fix or feature that would cause existing functionality to change).
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added unit tests to accompany this PR.
  • The (new/existing) tests cover this PR 100%.
  • I have (manually) tested this code to the best of my abilities.
  • My code follows the style guidelines of this project.

Documentation

For new features:

  • I have added a code example showing how to use this feature to the examples directory.
  • I have added documentation about this feature to the docs directory.
    If the documentation is in a new markdown file, I have added a link to this new file to the Docs folder README.md file.

Copy link
Copy Markdown
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TobiasBg Thanks for this PR.

The change on line 326 looks valid, but the other two changes are redundant. The conditions already check if $this->handle is a resource and that condition would return false as of PHP 8.0 (as it would be an object, not a resource), so adding an additional version comparison will not make a difference...

@jrfnl
Copy link
Copy Markdown
Member

jrfnl commented Sep 1, 2025

P.s.: probably better to change the version comparison on line 326 to a check for is_resource() as well for stability (if that is not checked earlier in the logic, in which case, the change on line 326 would be redundant too....)

`curl_close` is deprecated in PHP 8.5+, and hasn't been doing anything since PHP 8.0, when handles were switched from `resource` to `object`.

To prevent deprecation warnings it should therefore be called on older versions of PHP only, where handles are `resource`s.

See https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_no-op_functions_from_the_resource_to_object_conversion.
@TobiasBg
Copy link
Copy Markdown
Contributor Author

TobiasBg commented Sep 2, 2025

Oh, thanks @jrfnl! Good to know about the resource/object switch.

I have updated the PR, to only leave that one line, with an is_resource check.

@jrfnl
Copy link
Copy Markdown
Member

jrfnl commented Sep 2, 2025

Good to know about the resource/object switch.

It helps to understand a deprecation before addressing it... Not sure if you pulled similar PRs elsewhere, but if so, you may want to update those too...

@TobiasBg
Copy link
Copy Markdown
Contributor Author

TobiasBg commented Sep 2, 2025

Yes, checked those already. Thanks.

I think that including the PHP version check can be helpful in the future, when support for older versions of PHP is dropped in projects.

@jrfnl
Copy link
Copy Markdown
Member

jrfnl commented Sep 2, 2025

I think that including the PHP version check can be helpful in the future, when support for older versions of PHP is dropped in projects.

I don't agree. That's what comments are for, or one could open an issue with a tasklist as a reminder of things which would need to be done in the future, but the code itself should be based on what makes it most stable. Considering that people can compile custom versions of PHP with different version of extensions (no matter how unlikely it is for anyone to do that in this case), the is_resource() check creates the most stable code.

@TobiasBg
Copy link
Copy Markdown
Contributor Author

TobiasBg commented Sep 3, 2025

Great points and arguments! Changed my mind :-) Thanks!

@TobiasBg TobiasBg requested a review from jrfnl September 12, 2025 10:23
@jrfnl jrfnl removed their request for review September 12, 2025 14:07
@jrfnl jrfnl added this to the 2.0.x Next milestone Nov 14, 2025
@jrfnl jrfnl mentioned this pull request Nov 14, 2025
25 tasks
@schlessera schlessera merged commit bc011fd into WordPress:develop Nov 20, 2025
21 of 22 checks passed
@schlessera
Copy link
Copy Markdown
Member

Thanks for your PR, @TobiasBg !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants