Update Requests library to version 2.0.0

[jrf]

The Requests library has released a new major version: 2.0.0.

This is a major release and contains breaking changes.

Most important changes to be aware of for this release:

• All code is now namespaced, though there is a full BC-layer available and the old class names are still supported, though using them will generate a deprecation notice (which can be silenced by plugins if they'd need to support multiple WP versions). An upgrade guide is available and I'd recommend for this change + a link to the upgrade guide to be included in the WP 5.9 dev-note.
• A lot of classes have been marked final. This should generally not affect userland code as care has been taken to not apply the final keyword to classes which are known to be extended in userland code.
• Extensive input validation has been added to Requests. When Requests is used as documented though, this will be unnoticable.
• A new WpOrg\Requests\Requests::has_capabilities() method has been introduced which can be used to address #37708
• A new WpOrg\Requests\Response::decode_body() method has been introduced which may be usable to simplify some of the WP native wrapper code.
• Remaining PHP 8.0 compatibility fixed (support for named parameters)
• PHP 8.1 compatibility

Full changelog: ​https://github.com/WordPress/Requests/releases/tag/v2.0.0

Website (updated): ​https://requests.ryanmccue.info/

It is recommended for WordPress to update the bundled version of Requests.

I've prepared a PR for the update and will link it to this ticket.

Previous: #33055, #47746, #49922, #53101, #53334

### External libraries: upgrade to Requests 2.0.0

### Upgrade references to the Requests library in the WP code

Trac ticket: https://core.trac.wordpress.org/ticket/54504

[jrf]

GH PR is ready for review and if found okay, for commit: ​https://github.com/WordPress/wordpress-develop/pull/1624

Marked as ready for review and updated with the final Requests 2.0.0 code.

[SergeyBiryukov]

In 52244:

External Libraries: Update the Requests library to version 2.0.0.

This is a major release and contains breaking changes.

Most important changes to be aware of for this release:

• All code is now namespaced. Though there is a full backward compatibility layer available and the old class names are still supported, using them will generate a deprecation notice (which can be silenced by plugins if they'd need to support multiple WP versions). See the ​upgrade guide for more details.
• A lot of classes have been marked final. This should generally not affect userland code as care has been taken to not apply the final keyword to classes which are known to be extended in userland code.
• Extensive input validation has been added to Requests. When Requests is used as documented though, this will be unnoticable.
• A new WpOrg\Requests\Requests::has_capabilities() method has been introduced which can be used to address #37708.
• A new WpOrg\Requests\Response::decode_body() method has been introduced which may be usable to simplify some of the WP native wrapper code.
• Remaining PHP 8.0 compatibility fixed (support for named parameters).
• PHP 8.1 compatibility.

Release notes: ​https://github.com/WordPress/Requests/releases/tag/v2.0.0

For a full list of changes in this update, see the Requests GitHub:
​https://github.com/WordPress/Requests/compare/v1.8.1...v2.0.0

Follow-up to [50842], [51078].

Props jrf, schlessera, datagutten, wojsmol, dd32, dustinrue, soulseekah, costdev, szepeviktor.
Fixes #54504.

[dd32]

Can we consider renaming the namespace from WpOrg to something more generic, such as WordPress or WP?

That appears to be the first WordPress-specific namespace used within WordPress, and setting the name as WpOrg seems like it might be rather ugly later on.

The inclusion of Org is also kind of close to the namespace used on WordPress.org of WordPressdotorg.. So if this was intended on being "Not WordPress, but a WordPress.org thing" then that longer format may have been more appropriate, I'm not sure.

I realise this has been something that has been decided "upstream" in Requests, but raising this here since it's where the merge has happened.

[SergeyBiryukov]

Replying to dd32:

Can we consider renaming the namespace from WpOrg to something more generic, such as WordPress or WP?

That appears to be the first WordPress-specific namespace used within WordPress, and setting the name as WpOrg seems like it might be rather ugly later on.

Thanks for bringing this up! That was my concern here too, but I didn't want it to block the merge.

The inclusion of Org is also kind of close to the namespace used on WordPress.org of WordPressdotorg.. So if this was intended on being "Not WordPress, but a WordPress.org thing" then that longer format may have been more appropriate, I'm not sure.

I realise this has been something that has been decided "upstream" in Requests, but raising this here since it's where the merge has happened.

For reference, it appears to be implemented in this PR: ​Requests 2.0.0: introduce namespaces:

Acronyms are "proper cased", i.e. first character uppercase, the rest of the acronym lowercase.

See ​this comment specifically for more details.

Either WordPressdotorg or WPorg, or simply WordPress or WP would be my preference too, though it slightly deviates from the "proper cased" rule used for other acronyms. Could it perhaps be an exception?

Reopening for discussion.

[costdev]

Using something like WordPress makes sense to me too. As it isn't an acronym, it should be fine to use CamelCaps.

WpOrg (lowercase or uppercase) is also a commonly used abbreviation for wordpress.org.

It's possible that WpOrg was used in an effort to say "This is created under the WordPress Organisation on GitHub". In this case, using WordPress may incorrectly imply that WordPress is a dependency for Requests.

It would be good to get some insight into the reason(s) why WpOrg was chosen or, more specifically, why WordPress wasn't chosen. It's quite possible that we're unaware of sound reasoning for it that we should consider when discussing an alternative namespace.

[jrf]

@dd32 I appreciate the concern, but Requests 2.0.0 has been released and the discussion about the namespace was had publicly in that repo, in open videocalls and in WP Slack months ago.
Input at the time would have been very welcome, but was not received and it's too late to change this now.

The choice for the namespace was discussed extensively and various options were considered during those discussions.

We originally were going to use just Requests\ as the "prefix", but that was causing problems with the BC-layer. In our opinion, including the BC-layer was imperative to allow for plugins supporting multiple versions of WP and using parts of Requests directly, so that meant we had to reconsider the namespace.

We considered using WordPress\Requests\, but as Requests is a stand-alone project, we did not want to create a situation were the namespace chosen by Requests could ever become a blocker for WordPress itself adopting namespaces at some point in the future.

We also considered that Requests is used by more projects than just WordPress and that the namespace prefix should not cause undue friction in other projects using Requests, so we ended up compromising on WpOrg\Requests\.

While it is, of course, perfectly possible to change it for the copied version included in WP, this would make all future merges of Requests a lot more complex, so I'd strongly recommend against that.

[SergeyBiryukov]

Replying to jrf:

We also considered that Requests is used by more projects than just WordPress and that the namespace prefix should not cause undue friction in other projects using Requests, so we ended up compromising on WpOrg\Requests\.

Thanks for the context! Since PHP namespaces are case-insensitive, is there any chance to capitalize that as WPorg\Requests\, so that the capitalization of WP is consistent in lines like this and across the project in general?

new WP_REST_Request( WPorg\Requests\Requests::POST, $route );

[jrf]

Since PHP namespaces are case-insensitive, is there any chance to capitalize that as WPorg\Requests\, so that the capitalization of WP is consistent in lines like this and across the project in general?

@SergeyBiryukov Good question. We did actually discuss that as well.

For the PSR-4 class name to path translation in the autoloaders (both the supported Composer autoloader as well as the custom autoloader), the namespace and class names have to be treated case-sensitively as otherwise this would break when code is run on case-sensitive OS-es, like *nix and MacOS.

To then treat the namespace prefix case-insensitively would be inconsistent, especially as this could only be supported when using the custom autoloader, but would break when using the Composer autoloader, which is why it was decided to not support that.

Closing as committed via changeset 52244.

[dd32]

while I still disagree with the namespacing choice chosen here, don't believe that the choices taken were correct (even if "publicly discussed"), and still strongly feel like Requests is using a completely wrong namespace choice (Which despite R2 being released, could be changed)

I'm happy to agree that the choice has been made and it's too late to really change that. I would like to see the Namespace properly capitalised though.

[SergeyBiryukov]

Similar feelings here, I would expect any naming changes that affect core and backward compatibility to be brought up in a dev chat for more feedback. I appreciate that one may not always have time or energy for that though.

[jrf]

Now look:

1. Requests is still an external dependency. Do you expect SimplePie or PHPMailer to bring something like this up in the WP Core Slack ? No, so don't expect it from Requests either.
2. The roadmap for Requests 2.0.0 was published well in time, the PR for the namespacing was open for more than a month, the release cycle was something like six months, with the release being delayed for an extra two months as well. Any feedback before the actual release would have been very welcome and would have been taken into account.
3. Changing the namespace now, even if just the case of the namespace, IS a breaking change, what with the autoloading being case-sensitive, and would require a 3.0 release.

The repo is public, conversations were had in public, there is a even a #core-http-api channel in the WP Slack for transparency for those interested, but nobody showed an interest.

It's easy to complain and criticize after the fact. If you wanted to have a say, you should have gotten involved.

[schlessera]

As @jrf already stated, Requests is still considered an external, third-party dependency, which moved from the ryanmccue personal GitHub account to the wordpress organization GitHub account. Requests has never adhered to the WordPress Core coding style. And now it has adopted PSR-4 and most PSR-12 conventions.

The correct way to name it according to the code style in place in the repository and the conventions in place in the larger PHP ecosystem would have been to use Wordpress\Requests (note the lower-case P, part of the convention for PSR-4). We wanted to avoid that for obvious reasons, as Requests is neither a part of the actual WordPress CMS, nor did we want to set a precedent for namespacing Core. What we have now is therefore under the 'wordpress' GH org as a namespace.

Thanks for the context! Since PHP namespaces are case-insensitive, is there any chance to capitalize that as WPorg\Requests\, so that the capitalization of WP is consistent in lines like this and across the project in general?

new WP_REST_Request( WPorg\Requests\Requests::POST, $route );

The above would actually be expected to use an import, which would turn the code into:

new WP_REST_Request( Requests::POST, $route );

The import itself can be automatically generated/managed by any decent IDE or code quality tool, so you would never type that namespace anyway.

Case-insensitive class names are often an issue if you work on multiple filesystems because any string comparisons to deal with the namespace (most notably in autoloading) tend to break. That's why most code quality tools will flag these as errors, and Composer explicitly does not even support them. I wouldn't be surprised if PHP decided to make them case-sensitive in a later version, as they are slowly getting rid of all and any ambiguities that might introduce bugs.

I see no reason why the class name would be problematic, as with proper usage, it becomes inconsequential.

Also, just as a final remark, @jrf and me (well @jrf, mostly) invested lots of time and energy in making a library that is a critical dependency of WordPress Core but had fallen into disrepair usable and maintainable again. A lot of thought went into everything, to ensure it works both as a dependency of WordPress Core as well as a useful package for any other PHP project that is depending on it, while maintaining backwards compatibility whenever we could. The entire endeavour was planned in such a way that pulling in the latest version is as frictionless as possible and can mostly just be automated. It would now be a total waste of contributor time to make changes just for the sake of it. However, I understand people feel strongly about the "WP brand/trademark". But I for one will not accept this additional effort be taken on by the Requests package. As a consumer of the Requests library, WordPress Core is of course free to render the process as efficient/inefficient as it wants, but I don't see any valid reason why the Requests package should adapt to that.

[SergeyBiryukov]

Replying to schlessera:

Thanks for the context! Since PHP namespaces are case-insensitive, is there any chance to capitalize that as WPorg\Requests\, so that the capitalization of WP is consistent in lines like this and across the project in general?

new WP_REST_Request( WPorg\Requests\Requests::POST, $route );

The above would actually be expected to use an import, which would turn the code into:

new WP_REST_Request( Requests::POST, $route );

The import itself can be automatically generated/managed by any decent IDE or code quality tool, so you would never type that namespace anyway.

Thanks for chiming in! That was just one of the examples of the current code as committed in [52244]:

new WP_REST_Request( WpOrg\Requests\Requests::POST, $route );

There are currently ~60 instances like that in core, though I guess they could use an import too.

Also, just as a final remark, @jrf and me (well @jrf, mostly) invested lots of time and energy in making a library that is a critical dependency of WordPress Core but had fallen into disrepair usable and maintainable again. A lot of thought went into everything, to ensure it works both as a dependency of WordPress Core as well as a useful package for any other PHP project that is depending on it, while maintaining backwards compatibility whenever we could. The entire endeavour was planned in such a way that pulling in the latest version is as frictionless as possible and can mostly just be automated.

There is no doubt about that, I highly appreciate all the efforts what went into the 2.0.0 release, and I'm sure that most users of the package will too.

I guess I do feel a bit strongly about capitalizing the WP acronym in a consistent way, but I'm happy to accept that the decision has been made and will not be addressed further. Re-closing this as fixed, as it looks like a consensus was reached here.

[SergeyBiryukov]

Follow-up: #54546, #54547.

[hellofromTonya]

Reopening to revert Requests 2.0.0 and move introducing it to the 6.0.0 cycle. Why?

The namespace and file case renaming in Requests 2.0.0 revealed 2 fundamental issues in the upgrader process:

• Not all of the current "from" WordPress version code is preloaded into memory before the upgrader changes the filesystem. See #54546 and #54589.
• The Filesystem does not identify or handle filename case changes on case-insensitive filesystems (see #54582 and #54547). This impacts Core and WP-CLI. It means on those systems the files are (a) not renamed and (b) are deleted when the old file removal deletes the files. The result is a fatal error.

These issues are not due to Requests library, but rather are due to the issues above in the filesystem and upgrader.

Reverting Requests back to its 5.8 version does not resolve the above issues but does restore back to 5.8 to avoid risky changes this late in the release cycle. Moving [52244] and [52315] along with the other tickets to 6.0 early gives breathing room and soak time for thoroughly investigating, refining, and multitiered testing.

Marking for needs-patch for reverting both [52244] and [52315]/#54562.

After the revert, then the milestone can be changed to 6.0 and marked as early.

Confidence check PR for ensuring all changesets are identified for revert.

Trac ticket: https://core.trac.wordpress.org/ticket/54504

[hellofromTonya]

In 52327:

HTTP API: Revert changeset [52315].

Reverting Requests 2.0.0 changes and moving to WordPress 6.0 cycle. Why? The namespace and file case renaming revealed 2 issues in Core's upgrader process.

See https://core.trac.wordpress.org/ticket/54504#comment:22 for more information.

See #54562, #54504.

[hellofromTonya]

Moving this ticket to 6.0. Once the branch is opened, let's get this committed early-like-actually-early. In doing so, it'll help with the upgrader enhancements for preloading and case-insensitive filesystem detection.

[azaozz]

Replying to schlessera:

As @jrf already stated, Requests is still considered an external, third-party dependency

If it is under the "WordPress umbrella" it should follow the WordPress best practices, i.e. backwards compatibility is a number one priority. Frankly it is unthinkable that a library developed as part of WordPress would not only break back-compat, but will cause fatal errors when used in core...

The correct way to name it according to the code style in place in the repository

If there is such "code style" requirement in that repository it should be removed as incompatible with WordPress. Also, generally, namespaces should be considered "evil" in open source software and avoided as much as possible. The reason it that they promote "lazy" names that go against the best practice in writing good code (poor self-documenting, confusing names, etc.).

Seems a possible/good way forward would be to release Requests 3.0 that will be incompatible with 2.0 but fully compatible with 1.0, then consider it for adding to the next WordPress release. The alternative would be to move the Requests repository out of WordPress and continue developing it as a "true" third-party library that is, unfortunately, incompatible with WordPress.

[SergeyBiryukov]

In 52328:

HTTP API: Revert changeset [52244].

Reverting Requests 2.0.0 changes and moving to WordPress 6.0 cycle. Why? The namespace and file case renaming revealed 2 issues in Core's upgrader process.

See https://core.trac.wordpress.org/ticket/54504#comment:22 for more information.

Follow-up to [52327].

See #54562, #54504.

[hellofromTonya]

Replying to @azaozz:

If it is under the "WordPress umbrella" it should follow the WordPress best practices, i.e. backwards compatibility is a number one priority. Frankly it is unthinkable that a library developed as part of WordPress would not only break back-compat, but will cause fatal errors when used in core...

@azaozz Requests 2.0.0 is fully backwards-compatible and both @jrf and @schlessera made sure that was a priority.

The fatal errors are not due to backwards compatibility, but rather are due to 2 issues identified in my comment above:

• Detecting and handling case-insensitive filesystems
• Preloading files into memory before upgrading the filesystem

Requests 2.0.0 revealed both of these upgrader problems. However, Requests itself is not the source of the problems.

Discovering that files must be preloaded means in 6.0 the upgrader can be made more robust and stable to ensure there's no co-mingling of "from" and "to" version code.

Discovering the limitations of not detecting and handling files in case-insensitive filesystems means Core also could never change the case of an existing file. That's limiting. But in 6.0, the Filesystem and/or upgrader could enhanced to detect this and properly handle it.

In talking with the Support team, there's a history of issues with the upgrade process which could be attributed to either of these upgrader issues and will be investigated. I'll also be doing a Trac search to identify other past tickets which might also be part of one or both of these issues.

The discovery of these problems is good as it means the upgrader can be enhanced in 6.0.

[TobiasBg]

The original merge changeset [52244] mentioned "PHP 8.1 compatibility".
Should relevant changes (if it's not too many?) maybe be backported to the reverted-to Requests version, to achieve (as much as possible) PHP 8.1 compatibility in WP 5.9 (released two months after PHP 8.1, after all)?

[costdev]

@hellofromTonya The discovery of these problems is good as it means the upgrader can be enhanced in 6.0.

+1 to this.

Heavy investigation and testing has been done through the past 5 days by multiple contributors and we have initial drafts of solutions to both problems. When we move to the 6.0 milestone, we'll revisit these to continue discussion and develop the solutions (and unit tests) more fully to get them ready for manual testing early in 6.0.

For now, we're focused back on 5.9 tickets.

[azaozz]

Replying to hellofromTonya:

@azaozz Requests 2.0.0 is fully backwards-compatible...
....
not detecting and handling files in case-insensitive filesystems means Core also could never change the case of an existing file.

Correct. Changing a file's name is a back-compat issue and should never happen unless completely unavoidable. That may be fixed in core in 6.0 but will remain a problem "forever" in other places :(

So my suggestion is to revert these file name changes and release a new version of Requests before updating it in core.

[SergeyBiryukov]

Replying to TobiasBg:

The original merge changeset [52244] mentioned "PHP 8.1 compatibility".
Should relevant changes (if it's not too many?) maybe be backported to the reverted-to Requests version, to achieve (as much as possible) PHP 8.1 compatibility in WP 5.9 (released two months after PHP 8.1, after all)?

Replying to azaozz:

Changing a file's name is a back-compat issue and should never happen unless completely unavoidable. That may be fixed in core in 6.0 but will remain a problem "forever" in other places :(

So my suggestion is to revert these file name changes and release a new version of Requests before updating it in core.

Both of these suggestions seem worth considering to me.

[costdev]

@azaozz Also, generally, namespaces should be considered "evil" in open source software and avoided as much as possible. The reason it that they promote "lazy" names that go against the best practice in writing good code (poor self-documenting, confusing names, etc.).

I'm not sure if this is the right place for a discussion on this, but I'd like to hear more about this at some point @azaozz if you're willing. My username's the same on Slack if that works for you. Most open source software has implemented, or is currently implementing, namespacing. I have never seen or heard that namespacing and open source software are, or should be, mutually exclusive or have a minimal relationship.

In terms of going against best practice, that really comes down to a more general issue. You can write poorly self-documenting code with or without namespaces, or you can write well self-documenting code with or without namespaces. There is no consensus in any community that namespaces are always poorly self-documenting. It's a difference of opinion within the development community, but the contemporary standard is to namespace, and that's true across multiple languages.

Best practice for the survival of open source software has always been to respect:

1. the wisdom that comes with longer-term contribution.
2. the right of software to exist alongside each other.
3. that open source software projects can only survive as long as they have contributors, which means that they must adapt to the tried-and-tested practices that fuel contemporary development.

Nearly everything else is down to consensus. Historically, every open source software project that has failed to do these is unfortunately no longer with us - all of us will have experience of a project we've contributed to being closed - It's a sad day when it happens.

Joomla, Drupal, etc. implement namespacing, as do most other similar open source software projects. While we are not those projects, we have a much larger reach and therefore rely even more heavily on a strong contributor base. Namespaces are only one contemporary standard, but dropping them sets a precedent for dropping other contemporary standards and in turn, threatens the size of our contributor base.

In any case, the topic of namespacing within Requests was discussed publicly long ago as @jrf said in an earlier comment. A lot of work has gone into implementing this and while Requests is under the WordPress umbrella, it has a downstream that will also be impacted. Experience tells me that if we start down the path of reverting contemporary standards, we might as well bundle Requests with Core and close the public Requests repo before we watch its usage and contributor base steadily reduce in the coming years, followed by Core some years after.

[azaozz]

Replying to costdev:

I'm not sure if this is the right place...

I'm pretty sure it is not. But as you started it, I'll have to answer your questions :)
I'll be happy to continue this conversation somewhere else at a later time.

In terms of going against best practice, that really comes down to a more general issue. You can write poorly self-documenting code with or without namespaces, or you can write well self-documenting code with or without namespaces.

Right, you can. However having a guarantee that your function or class name will "never" collide with anything is not a good "incentive" to try to come up with a good name. On the opposite, I've been hearing from many people over many years that one of the hardest part of coding is ... picking good descriptive names. My experience over the last 25 or so years is the same :)

the contemporary standard is to namespace, and that's true across multiple languages.

Yeah, namespacing seems to be used more and more (probably because you can easily pick repeating names with it, lol). It doesn't mean it is a good idea in all cases. It seems to be more useful/accepted in a "pure" OOP software (which is most of it), however WP is not OOP by a long shot. Namespacing for "mostly functions" doesn't seem to work that well.

Best practice for the survival of open source software has always been to respect:

Yes, you're right. However thinking this is getting very "philosophical". Whether recommending or not recommending one (mostly insignificant) feature does not affect any of it imho. It is just a recommendation after all.

On the other hand WP will never be able to implement namespacing to most of the PHP code as it will break it pretty badly. So having some parts with namespaces while most of the code is without is somewhat.... not that good? :)

Joomla, Drupal, etc. implement namespacing, as do most other similar open source software projects. While we are not those projects, we have a much larger reach and therefore rely even more heavily on a strong contributor base. Namespaces are only one contemporary standard, but dropping them sets a precedent for dropping other contemporary standards and in turn, threatens the size of our contributor base.

Not talking about dropping, but pretty much all of the current WP PHP code cannot be namespaced (technically not viable). On top of that namespaces do not promote better names. There can be requirements to "ignore" the namespacing and still try to give good, descriptive, unique names to all functions, but eventually these will be avoided or diminish in importance because that's one of the main features of namespacing: to let you get avay with repeating names :)

In any case, the topic of namespacing within Requests was discussed publicly...

That may be but it seems problematic to develop a library as part of WordPress and not take under consideration the established best practices and requirements for WordPress, whether that was discussed somewhere or not. Frankly this is the first time I'm hearing about it and I'm pretty sure it's the same for many other committers.

So in a hindsight: if namespaces cannot be added to WordPress is it a good idea to add them to a WordPress library? Perhaps it is but it would need (at least) a good standard of what function and class names are acceptable and what are not. And all of the community should be aware of it.

[jrf]

For everyone here now debatting the issue, while not having read the complete thread or the changelog, let me make something very clear:

The namespacing in Requests 2.0.0 and the file renames are NOT a BC-break.

Let me repeat that: The namespacing in Requests 2.0.0 and the file renames are NOT a BC-break.

The BC-break will come in Requests 4.0.0 a few years down the line.

Yes, the code in Requests is now namespaced, but there is a full backward-compatibility layer in place and using any of the old class names (like if plugins use Requests directly) will still work without problems.

When an old class name is requested, a deprecation notice is thrown. but for the lifetime of Requests 2.0.0, that deprecation notice can be silenced by setting a REQUESTS_SILENCE_PSR0_DEPRECATIONS constant. This has been implemented like this specifically with WP in mind, so plugins which use Requests directly and needing to support multiple WP versions can do so without issue.

Yes, there have been some file renames, but again, this is not a BC-break.

From the very beginning, Requests has had two "entry-points" for loading the files:

1. Autoloading via a Composer generated autoloader.
2. Autoloading via a custom autoloader which is included with the Requests code.

The file renames, including the case-only renames, are handled 100% correctly by the both autoloaders.

So where is the BC-break in this whole story ? Well, registering the custom autoloader has changed in Requests 2.0.0 and that change is effectively a two line change and was already included in the patch and will not affect plugins.

So what is this BC-break I mention above in Requests 4.0.0 ? Well, that is the version in which it is the intention to remove the BC-layer.

---

Now back to the problem at hand: what's breaking here is not Requests 2.0.0. What's breaking is that the WP upgrader process is trying to use a mix of files from Requests 1.x and 2.x during the upgrade process and isn't equipped to handle case-only file renames.

Am I surprised that that breaks things ? No, as that's a very clear bug and code smell. But it's a bug in the upgrader, not in Requests.

[azaozz]

Advantages of namespacing as per the PHP manual. Frankly I see both as being a disadvantages in some cases. Overriding internal funstion and class names would be super confusing for anybody trying to read the code. "Aliasing" long, descriptive names to short non-descriptive is... a bad thing, isn't it? :)

[azaozz]

Replying to jrf:

The namespacing in Requests 2.0.0 and the file renames are NOT a BC-break.

Yeah, I know. Sorry about it. Best to move the conversation to another place, perhaps at ​https://make.wordpress.org/core/.

Now back to the problem at hand: what's breaking here is not Requests 2.0.0. What's breaking is that the WP upgrader process is trying to use a mix of files from Requests 1.x and 2.x during the upgrade process and isn't equipped to handle case-only file renames.

Am I surprised that that breaks things ? No, as that's a very clear bug and code smell. But it's a bug in the upgrader, not in Requests.

I understand that, but on the other hand files in WordPress should never be renamed unless absolutely unavoidable. If this triggers a bug in WP is there a 100% guarantee it will never trigger a bug somewhere else?

As this is a WordPress (sub) project, I'd think it should follow the established rules/best practices. So the questing is: is this completely unavoidable? If not, I stand by my recommendation to revert and release a new version before updating it in WP.

[schlessera]

Replying to azaozz:

If it is under the "WordPress umbrella" it should follow the WordPress best practices, i.e. backwards compatibility is a number one priority. Frankly it is unthinkable that a library developed as part of WordPress would not only break back-compat, but will cause fatal errors when used in core...

If there is such "code style" requirement in that repository it should be removed as incompatible with WordPress. Also, generally, namespaces should be considered "evil" in open source software and avoided as much as possible. The reason it that they promote "lazy" names that go against the best practice in writing good code (poor self-documenting, confusing names, etc.).

Seems a possible/good way forward would be to release Requests 3.0 that will be incompatible with 2.0 but fully compatible with 1.0, then consider it for adding to the next WordPress release. The alternative would be to move the Requests repository out of WordPress and continue developing it as a "true" third-party library that is, unfortunately, incompatible with WordPress.

I will not bother to discuss the benefits of namespacing here, but I just want to state that I feel the above is painfully disrepectful of the many months of hard work that were invested into the Requests library to ensure it keeps working as a secure HTTP abstraction layer for the WordPress project.

Backwards-compatibility was the number one priority, and I can only guess now how @jrf must have felt reading the above, when she literally spent nights and weekends to ensure everything stays compatible.

I'm happy to discuss ways of fixing the upgrader process and will help test it and support it with WP-CLI. But as far as I am concerned, there will not be a Requests v3 to undo the work we did to make that library maintainable again.

[hellofromTonya]

Somehow this ticket became a software principles discussion.

Quoting @azaozz:

namespaces should be considered "evil" in open source software and avoided as much as possible

I've had couple of contributors reach out to me privately in slack asking why this statement was made. I'll share my comments to provide balance to opinions already shared.

Namespacing:

• is common practice across many programming languages including JavaScript, PHP, Python, C#, etc.
• is common practice in open source projects including PHPMailer (already embedded in Core), WooCommerce, WP-CLI, Drupal, Joomla, Laravel, Symfony, BigCommerce, Composer, PHPUnit, etc.
• works well for functions, classes, traits, interfaces, etc.

In my opinion:

• It's not evil.
• It does not promote "lazy" or bad naming practices.
• Yes, naming stuff is hard. It's always been hard and I've seen folks struggle to name things throughout my 35 years in engineering.

Quoting @azaozz:

On the other hand WP will never be able to implement namespacing to most of the PHP code as it will break it pretty badly.

I want to point out that actually WordPress could add namespaces if a BC-layer is also created (could follow Requests 2.0.0 example).

WordPress is not limited or held back from adding namespaces. Actually, they already exist in Core with the external SimplePie library.

For Core's existing code, decisions would need to be made about where it makes sense to use it. For new code, it could be added if the coding standards allowed it.

If there's interest in introducing namespacing into Core beyond external libraries like SimplePie and Requests, then that discussion deserves its own ticket.

[azaozz]

Replying to hellofromTonya:

Somehow this ticket became a software principles discussion.

Yeah, sorry, I shouldn't have responded to @costdev's comment, my bad.

I've had couple of contributors reach out to me privately in slack asking...

Well it was meant as a joke, mostly, hence the quotes. Interesting though why people have been reaching out to you and not to me to explain what I meant. Wouldn't it be better to get it "straight from the horse's mouth"? :)

Namespacing:
...
I want to point out that actually WordPress could add namespaces if a BC-layer is also created.

Lots of unneeded complexity for a seemingly small, insignificant improvement (that can actually be used to lower the code quality in the long run)? But lets take it to ​https://make.wordpress.org/core/, can we, please?

[azaozz]

Replying to schlessera:

the above is painfully disrepectful of the many months of hard work

I'm sorry you feel that way but these things (small errors that "upset the applecart") happen sometimes to everybody, me including.

So basically you're saying that changing the case in the names is an essential, irreplaceable part of Requests 2.0 and this cannot be changed again without throwing all the other work out? In this case, as I pointed out above, lets consider it an exception to the WordPress best practices rule for never changing file names and be done with it?

[schlessera]

Replying to azaozz:

I'm sorry you feel that way but these things (small errors that "upset the applecart") happen sometimes to everybody, me including.

Oh, I'm always up for "upsetting the apple cart". But your words seemed to imply that we just changed Requests on a whim and put no thoughts into it whatsoever, and I certainly hope that neither @jrf nor I are giving the impression that that is how we operate.

So basically you're saying that changing the case in the names is an essential, irreplaceable part of Requests 2.0 and this cannot be changed again without throwing all the other work out?

No, not at all. What I'm saying is that the current maintainers of Requests will not undo their work and change back the maintenance improvements they've made, as I'd consider this "making Requests worse to avoid having to make WordPress better".

As rigid as WordPress may be, at the very least its upgrader process (!) needs to account for change - the one constant that is always true for software projects. If it does not, it is failing its purpose.

[jrf]

#54967 was marked as a duplicate.

[costdev]

#54999 was marked as a duplicate.

[jrf]

FYI: Requests 2.0.1 has just been released - ​https://github.com/WordPress/Requests/releases/tag/v2.0.1

Re-opening as the commit was reverted.

This has got a merge conflict and and older version of Requests.

@jrfnl:

• do you still wish to get the requests bump in WP 6.0?
• if so, which version? I saw there was a release in the last 24 hours or so.

The trac ticket for upgrade is tagged early, and the beta is two weeks away so it would be good to see something soonish.

Mainly for my own notes: for the purposes of testing this is a randomly selected ​before zip file.

Sorry @jrfnl, I just noticed there isn't a patch for https://core.trac.wordpress.org/ticket/54582 yet, you may wish to hold of on decisions until that goes through.

This has got a merge conflict and and older version of Requests.

@jrfnl:

• do you still wish to get the requests bump in WP 6.0?
• if so, which version? I saw there was a release in the last 24 hours or so.

The trac ticket for upgrade is tagged early, and the beta is two weeks away so it would be good to see something soonish.

Mainly for my own notes: for the purposes of testing this is a randomly selected ​before zip file.

Sorry @jrfnl, I just noticed there isn't a patch for https://core.trac.wordpress.org/ticket/54582 yet, you may wish to hold of on decisions until that goes through.

@peterwilsoncc Thanks for the ping. As you already noticed, any patch to upgrade Requests to the 2.x series depends on Trac #54582 being fixed first.

Also be aware that this patch was created 7 months ago and will need more than just a rebase to fix merge conflicts.

• A new search will have to be done across the WP codebase to find all uses of the old Requests classes and replace them as new code referencing the old classes may have been introduced in the mean time.
• The Requests files will need an update to version 2.0.1 (or 2.0.2, though for the purposes of WP, 2.0.2 contains no changes from 2.0.1 other than the version number constant as the only difference is an updated certificate bundle and WP uses its own); or depending on when Trac#54582 is resolved, whatever the current version of Requests is by that time.

This patch can serve as a basis/inspiration for a new patch, but should by no means by regarded as a viable patch to go into Core as is, by now.

[peterwilsoncc]

I've removed this from the 6.0 milestone inline with #54582, which is a blocker for this ticket.

[kirasong]

A quick note to check out #54562, which I've also moved to Future Release, to see if it's still an issue after this is merged.

Edit: Wrong ticket number.

[costdev]

#55598 was marked as a duplicate.

[aristath]

#55727 was marked as a duplicate.

[azaozz]

Replying to schlessera:

Uhh, sorry for the very late reply, had some health problems and had to take some time off.

So basically you're saying that changing the case in the names is an essential, irreplaceable part of Requests 2.0 and this cannot be changed again without throwing all the other work out?

No, not at all. What I'm saying is that the current maintainers of Requests will not undo their work and change back the maintenance improvements they've made...

Seems we're still misunderstanding one another and mostly talking about different things...

Think this is pretty obvious by now, just adding it here mostly for posterity. As far as I see version 2.0 of the Requests library was refactored completely. The refactoring:

• Made the library compatible with PHP 8.x. That's a very welcome change.
• Significantly changed the way the code was written and organized. The "coding style" is quite better now, perhaps the "barrier to entry" is somewhat higher when comparing it to the rest of the WordPress code, but that's not a (big) issue.
• Some files were renamed where the new names only had capitalization changes. Unfortunately this is incompatible with how PHP handles (or actually cannot handle) the differences between case-sensitive and non-case-sensitive filesystems. That exposed a bug in the WP upgrade process (that will likely be fixed) but may affect other uses of the library.
• The refactoring broke backwards-compatibility.
• There are no new features and no major (user facing) enhancements. The changes were mostly to how the code is organized and written.

Seems that the last three points make this refactoring somewhat incompatible with WordPress. It is also not inline with the WordPress philosophy to always prioritize the needs of its users. While the library "looks" a lot better now, it doesn't seem to "work" better as its functionality is almost the same. The only significant changes in functionality were to make it compatible with PHP 8.x and the broken backwards compatibility.

In that terms is seems that the refactoring in version 2.0 was perhaps not a good idea at this time and only causes (both philosophical and code) incompatibilities.

As a result the Requests library is currently in "limbo": it is a WordPress project that doesn't follow established WordPress philosophies and cannot be used by WordPress because of some (good but mostly insignificant) changes.

[audrasjb]

As per today's bug scrub and Core Tech discussion, let's move this ticket to Future Release as it needs to be addressed alongside #54582 which is a blocker for this ticket.

[jrf]

#56612 was marked as a duplicate.

[hellofromTonya]

#54562 was marked as a duplicate.

[TimothyBlynJacobs]

#57009 was marked as a duplicate.

[schlessera]

I seem to be repeating myself here, but as the last meaningful comment on here was again stating incorrect conclusions, I'll go at it again.

Requests v2 does not break backwards compatibility with v1. Quite the contrary, most of the work that we did went into ensuring it stayed backwards compatibile despite the many changes that were required for improved maintenance and speedier security updates.

The reason why this was not cleanly integrated into WordPress Core is a very different one:
it turns out that the Upgrader logic in WordPress Core cannot properly handle changes in filenames.

That's it, that's the only remaining issue: a bug in the Upgrader process!

Requests v2 is not in limbo, it is more active than ever, and gets very regular security updates.

@azaozz I would be grateful if you would stop repeating such incorrect statements - pretty much everyone else has been very vocal about this not being the case. This doesn't help anyone, and we should focus our energy on fixing the Upgrader instead.

[swissspidy]

#57184 was marked as a duplicate.

[azaozz]

Replying to schlessera:

Requests v2 does not break backwards compatibility with v1.
...
The reason why this was not cleanly integrated into WordPress Core is a very different one:
...
@azaozz I would be grateful if you would stop repeating such incorrect statements - pretty much everyone else has been very vocal

Uhhh, it seems we are still talking about different things :(

I cannot understand what you mean by "v2 does not break backwards compatibility with v1". Tried replacing v1 with v2 in trunk and got this:

Fatal error: Interface 'WpOrg\Requests\HookManager' not found in /src/wp-includes/Requests/Hooks.php on line 19

How a fatal error is not breaking back-compat? If backwards compatibility was maintained, there wouldn't be any need to modify any other code outside of the Requests directory. However looking at ​https://github.com/WordPress/wordpress-develop/pull/1624/files#diff-339369deb37417feede85bbf7991a48f8ad44ec96e389ed42cecda99f2f2e98a there seem to be many changes to other WP files.

Another, perhaps larger part of the problems here is: what should an open source project do with contributions that do not follow its philosophy and seem to be disregarding its rules? As far as I've seen most open source projects do not accept contributions that refactor code without a good enough reason. The "improved maintainability" by renaming functions/methods/classes/files and adding namespaces seem more of a personal preference here. WordPress is mostly legacy PHP code, this library is not an exception.

As I said above, a refactoring would have been warranted if it added new, needed functionality or significantly improved the existing functionality. This is not the case in Requests 2, unfortunately.

So what should the WordPress Open Source project do with such contribution? Seems there are two options:

1. Fork it's own sub-project and maintain it as part of core. This means Requests 1 will need to be patched to be PHP 8+ compatible as part of core. It also means the Requests library will have to be moved outside of ​https://github.com/WordPress/. Then the maintainers will be able to do as many changes as they want and follow their own philosophy and rules. I'm not looking forward to any of this :(
2. The WordPress Open Source project would make an exception and accept a contribution that do not follow its philosophy and rules. This will be a bad precedent that may lead to other contributors attempting to do something similar in the future. Not looking forward to this either.

What would you do if you had to decide which is the "lesser evil"?

(Edit: Just to make sure this is not misunderstood, the "lesser evil" expression means "the less harmful or unpleasant of two bad choices or possibilities".)

[peterwilsoncc]

@azaozz @schlessera I think the best course of action here is to agree to disagree as to what this is called. There's an interaction between Requests, WordPress Core and PHP so concentrating on that will allow Requests to be updated to the latest, greatest and PHP 8 compatibile-ist.

@costdev have been doing some work for #54582, see ticket 54582 (comment) and @SergeyBiryukov has a ​draft pull request for that issue too.

[azaozz]

we should focus our energy on fixing the Upgrader instead.

@costdev have been doing some work for #54582

+1. Yea, hope everybody agrees on this. The upgrader bug needs to be fixed regardless.

[jrf]

@peterwilsoncc I appreciate what you are trying to do and I agree that we should concentrate on getting the Upgrader component fixed, but I can't let this stand.

Apologies in advance @azaozz, but enough is enough. I'm calling you out now: what you are saying is technically and factually incorrect.

You have been told so time and time again, but as you don't seem to want to understand it, I will now *proof* it to you in excruciating detail.

However looking at ​https://github.com/WordPress/wordpress-develop/pull/1624/files#diff-339369deb37417feede85bbf7991a48f8ad44ec96e389ed42cecda99f2f2e98a there seem to be many changes to other WP files.

I have rebased the above PR against trunk, updated the PR to Requests 2.0.5 and made some small updates to bridge the differences in trunk between when the original PR was created and now.

The original PR contained 2 commits:

1. The updates to the Requests native files (66 files changed, ++3062 --1765)
2. The updates to the WP Core files to upgrade to Requests 2.0.0. (12 files changed, ++58 --58)

I've now split the PR into 5 commits:

1. Only updates the files copied in from Requests (66 files changed, ++3178 --1882)
2. Updates WP to work with Requests 2.0, including silencing deprecation notices and updating 1 test (2 files changed, ++10 --2)
3. Upgrades WP to Requests 2.0 - code changes only (4 files changed, ++21 --28)
4. Upgrades WP to Requests 2.0 - test changes only (6 files changed, ++12 --13)
5. Upgrades WP to Requests 2.0 - documentation changes only (7 files changed, ++22 --22)

To do a minimal update, this means only commit 1 and 2 are needed, which effectively changes 8 (!!) lines in WP Core and that includes silencing deprecation notices about the use of the old class names.

Realistically, you could even get away with just making the change on line 21 and line 478 of the wp-includes/class-wp-http.php file, which means 2 lines of code changed in WP core. That's it. That's all that is required.

To do a full upgrade, 33 lines of code need to be updated in WP Core and that includes updates to the tests. 33 LOC changed for a complete upgrade to a new major of a dependency is still F* all.

what should an open source project do with contributions that do not follow its philosophy and seem to be disregarding its rules?

Requests is still an EXTERNAL dependency. An external dependency closely affiliated with WP, but an independent project used by some 5000 other projects aside from WP and it should be respected as such.

I mean you don't go making demands of the maintainers of SimplePie, PHPMailer or Sodium Compat to comply with arbitrary WP rules ?

Oh and just in case you missed it: PHPMailer introduced namespaces a while back and SimplePie has done so recently as well. Are you going to ask them to reverse these code improvements ?

Be grateful that the changes as made in Requests were done in such a considerate way towards WP instead of spreading misinformation about non-existing breaking changes.

[azaozz]

Replying to jrf:

what you are saying is technically and factually incorrect.
...
To do a minimal update, this means only commit 1 and 2 are needed, which effectively changes 8 (!!) lines in WP Core

Realistically, you could even get away with just making the change on line 21 and line 478 of the wp-includes/class-wp-http.php file, which means 2 lines of code changed in WP core. That's it. That's all that is required.

To do a full upgrade, 33 lines of code need to be updated in WP Core and that includes updates to the tests.

Thanks for the explanation! Yes, I understand, still a change is a change whether it is 2, 8, or 32 lines. But lets not concentrate on that.

Requests is still an EXTERNAL dependency.

I think this is the big misconception here. Requests used to be an external dependency before it was moved from rmccue/Requests to WordPress/Requests. After moving there it became an internal dependency/library. In effect the WordPress project "forked" the Requests library to ensure it is maintained (if I remember right).

This actually brings another question: why does the Requests library use WpOrg\ namespace? As @dd32 above I'm a bit unsure if that's the best choice. Of course, as a project under the WordPress umbrella it can use any form of the WordPress name, but perhaps may be better to use Requests\, similarly to other PHP libraries? Any other opinions on that?

(BTW, seems WP is using an older version of SimplePie which is not namespaced. The latest version seems to be using namespace SimplePie; which seems to be appropriate for a library.)

[jrf]

Replying to azaozz:

Replying to jrf:

Requests is still an EXTERNAL dependency.

I think this is the big misconception here. Requests used to be an external dependency before it was moved from rmccue/Requests to WordPress/Requests. After moving there it became an internal dependency/library. In effect the WordPress project "forked" the Requests library to ensure it is maintained (if I remember right).

Actually, no, this is not true.

The repo was moved (not forked) - at my request - to lessen the risk of such a vital dependency for WordPress being locked in on someone's personal GH account, while that person no longer showed any interest in maintaining it.

Personal accounts on GH can provide other people with commit rights - which both Alain and me already had by then -, but not with admin rights, which created a "one key to the castle" situation which is just plain risky for a project with the reach of Requests.

That and that alone was the reason to look for a new home for the repo.

Our hope was that with a move to the WP organisation, more WP committers/contributors would potentially get involved/show an interest, but so far that has not happened in any significant way and Alain and me (and an incidental outside contributor or two) remain the only active contributors/maintainers.

It was never discussed nor implied that the status of the project as external dependency would change and in my opinion, it hasn't changed, we've just ensured that the "one key to the castle" risk was mitigated.

This actually brings another question: why does the Requests library use WpOrg\ namespace? As @dd32 above I'm a bit unsure if that's the best choice. Of course, as a project under the WordPress umbrella it can use any form of the WordPress name, but perhaps may be better to use Requests\, similarly to other PHP libraries? Any other opinions on that?

Asked & answered before. The short answer is: technical reasons. See: https://core.trac.wordpress.org/ticket/54504?replyto=82#comment:12

(BTW, seems WP is using an older version of SimplePie which is not namespaced. The latest version seems to be using namespace SimplePie; which seems to be appropriate for a library.)

See: https://core.trac.wordpress.org/ticket/55604

[azaozz]

Replying to jrf:

The repo was moved (not forked) - at my request - to lessen the risk of...

That and that alone was the reason to look for a new home for the repo.

Thanks for the clarification!

Not sure it makes any difference whether it was moved, forked, copied or transferred (the latter is the official GitHub term afaik). The important bit is that it is now part of the WordPress repo, just like all the rest of the core related projects/repos there.

It was never discussed nor implied that the status of the project as external dependency would change and in my opinion, it hasn't changed, we've just ensured that the "one key to the castle" risk was mitigated.

Yes, this is where all differences come from. As far as I understand when something is part of the WordPress repository and GitHub account it is also part of the WordPress Open Source project which takes ownership of it. In that case the WordPress core team carries certain responsibilities for it (maintenance, support, preventing misuse, etc.) and has certain rights like for example to archive a repository if it's forked, transferred, or the project has ended, etc. In that terms all (sub)repositories under ​https://github.com/WordPress should uphold the project's values, philosophy and abide by the project's rules.

Since there are different opinions, I think it would be best to follow @peterwilsoncc's advice and "agree to disagree" for now. As this seems to perhaps have some legal implications (project ownership) I'll try to follow up and get some advice. Hopefully that will result in clarifying the status of software added to the WordPress Github repository, and any guidelines will perhaps be added to the contributor handbook.

With this I'd like to consider the matter closed for now. Sorry this is sooo much off-topic here.

[hellofromTonya]

Spoke with @jrf @azaozz @SergeyBiryukov today. Here's status:

Blockers are now unblocked:

• Namespace: consensus is to move forward without change to the external library.

• #54582: Juliette and I discussed a workaround by:
  • Retaining the original files with redirect code (i.e. the strategy @SergeyBiryukov did with other files, such as renaming wp-db.php to class-wpdb.php #56268 / [53749], [53750], [53755], [53756]).
  • Using Requests 2.x folder structure by loading the new library into src/wp-includes/Requests/src/.

• #54562 Fatal errors from old code not loaded into memory before upgrading. This is solvable by preloading the old files into memory.

The plan:

• Juliette and I will work together to get the PR ready (per above).
• Then @ironprogrammer and I will (and anyone else) will test and submit test reports.
• Goal: commit by the end of next week.

Any issues can be iterated on during the 6.2 alpha cycle.

I'm reassigning ownership to me.

[SergeyBiryukov]

The approach from comment:85 sounds good 👍

Replying to hellofromTonya:

Retaining the original files with redirect code (i.e. the strategy @SergeyBiryukov did with other files, such as renaming wp-db.php to class-wpdb.php #56268 / [53749], [53750], [53755], [53756]).

See comment:33:ticket:47632 for a general procedure with these renamings.

I think the last two commits are specific to the wpdb class, as it can be loaded outside of WordPress core, where ABSPATH, INC, and deprecated_file() may not be defined, so those commits addressed that.

Generally, these are all safe to use, see [45678] or [52026] for example (and [53518] as a follow-up).

[SergeyBiryukov]

Replying to hellofromTonya:

#54582: Juliette and I discussed a workaround by:

• Retaining the original files with redirect code (i.e. the strategy @SergeyBiryukov did with other files, such as renaming wp-db.php to class-wpdb.php #56268 / [53749], [53750], [53755], [53756]).
• Using Requests 2.x folder structure by loading the new library into src/wp-includes/Requests/src/.

#54562 Fatal errors from old code not loaded into memory before upgrading. This is solvable by preloading the old files into memory.

Thinking about this some more:

• Using the wp-includes/Requests/src/ directory for Requests 2.x should indeed work around #54582, as the $_old_files array would no longer have conflicting file names on case-insensitive filesystems.
• Preloading Requests 1.x files into memory should resolve #54546. In that ticket, a fatal error happened when cURL timed out and attempted to throw an exception when its file had not yet been loaded into memory. The exception class was changed, which would be handled by the "to" version autoloader but was not recognized by the "from" version autoloader in memory.

With the proposed plan, unless I'm missing something, the "from" and "to" Requests versions should no longer conflict, and the old version can be safely deleted as part of the general $_old_files cleanup after the update.

So I was wondering, do we still need to keep the old files in that case? Is it for plugins that may have included them directly? Or is there a scenario where preloading Requests 1.x files into memory before the update is not enough, and they are still needed after the update?

[jrf]

Replying to SergeyBiryukov:

Replying to hellofromTonya:
With the proposed plan, unless I'm missing something, the "from" and "to" Requests versions should no longer conflict, and the old version can be safely deleted as part of the general $_old_files cleanup after the update.

So I was wondering, do we still need to keep the old files in that case? Is it for plugins that may have included them directly? Or is there a scenario where preloading Requests 1.x files into memory before the update is not enough, and they are still needed after the update?

I had similar thoughts regarding yes/no leaving and redirecting the old Requests files.

I mean, Requests has always included an autoloader (from the very beginning), so aside from the "entry" file, which is currently wp-includes/class-requests.php, no other file from Requests should even need to be "manually" included via an include or require statement, not by WP Core, not by plugins, so I don't think we'd need to keep the old files around.

AFAICS the preloading of the "old" files should be sufficient for the upgrade process itself.

• For updates from a WP version containing Requests 1.x to a WP version containing Requests 2.x, Requests 2.x will be used during the upgrade, so loading all 1.x classes into memory should be enough.
• For updates in the future from a WP version containing Requests 2.x, the preloading doesn't do any harm as all 1.x classes have a 2.x counter-part. It may be a good idea though, just and only for the preloading, to set the REQUESTS_SILENCE_PSR0_DEPRECATIONS constant as the preloading would load the class names from 1.x, so would trigger the deprecation notices.

### External libraries: upgrade to Requests 2.0.0
### Upgrade references to the Requests library in the WP code

This PR:

• [x] Moves all new files to src/wp-includes/Requests/src/.
• [x] Updates the require statement for Autoload.php in src/wp-includes/class-wp-http.php.
• [x] Adds Requests 1.x files/directories to $_old_files.
• [x] Introduces _preload_old_requests_files() to preload Requests 1.x slated for deletion.
• [x] Adds a call to _preload_old_requests_files() at the beginning of update_core().

Trac ticket: https://core.trac.wordpress.org/ticket/54504

[costdev]

I've submitted ​PR 3732 with the changes mentioned above.

In local testing (each a clean attempt at upgrading from 6.1.1) to a custom package with these changes, the old files were preloaded, deleted, and the new files placed in wp-includes/Requests/src.

Closing this PR in favor of ​https://github.com/WordPress/wordpress-develop/pull/3732.

[ironprogrammer]

Test Report

Tested on PHP 7.4 per custom install package instructions ​provided here. Also added optional Requests 1.x preload logging ​found here.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/3732 (via custom install package).

Environment

• Hardware: MacBook Pro Apple M1 Pro
• OS: macOS 12.6.1 (case insensitive)
• Browser: Safari 16.1
• Server: nginx/1.23.2
• PHP: 7.4.33
• WordPress: 6.1.1 -> 6.2-alpha-54642-src (custom install package)
• Theme: twentytwentythree v1.0
• Active Plugins:
  • fake-wordpress-update v1.0.0 (provided in test instructions)

Actual Results

• ✅ Upgrade via Dashboard > Updates succeeded without error.
  • File/directory counts matched those expected.
  • Optional interface/class check logging shows Requests 1.x was preloaded into memory as expected.
  • General site activity did not generate errors. Tested: search and install plugins, update plugins, search and install themes, activate Gutenberg, add/view post, add media, add pattern from directory, add user.
• ❌ Upgrade via WP-CLI wp core update resulted in a fatal error.
  • On terminal:

    Updating to version 6.1.1-with-requests2 (en_US)...
    Downloading update from http://wp-54504-cli.test/wordpress-with-requests-2.zip...
    Unpacking the update...
    

  • In debug.log:

    PHP Fatal error:  Cannot declare class Requests_Cookie_Jar, because the name is already in use in
    ../wp-includes/Requests/Cookie/Jar.php on line 15
    

  • Site did not upgrade, and remained on 6.1.1.
  • wp core check-update returns 6.1.1, and not the version provided by the test plugin.
• ✅ Upgrade via automatic core update succeeded without error. Test plugin modified ​per these instructions.
  • File/directory counts matched those expected.
  • Optional interface/class check logging shows Requests 1.x was preloaded into memory as expected.
  • General site activity did not generate errors. Tested: search and install plugins, update plugins, search and install themes, activate Gutenberg, add/view post, add media, add pattern from directory, add user.

Additional Notes

• At one point after the Dashboard > Updates upgrade, I revisited the Updates page (/wp-admin/update-core.php) and it logged a ​PHP warning, but did not surface again during subsequent testing. The warning was:

PHP Warning: An unexpected error occurred. Something may be wrong with WordPress.org or this server&#8217;s configuration. If you continue to have problems, please try the <a href="​https://wordpress.org/support/forums/">support forums</a>. (WordPress could not establish a secure connection to WordPress.org. Please contact your server administrator.) in ../wp-includes/update.php on line 447

Supplemental Artifacts

See the ​test instructions for additional resources to validate this PR.

Test plugin active, reflecting available update for custom install package.

Update button on Dashboard > Updates.

[antonvlasenko]

Test Report

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/3732

Environment

• OS: macOS 12.6
• Web Server: Apache
• PHP: 8.1.9
• WordPress: 6.1.1
• Browser: Safari 16.1
• Theme: Twenty Twenty-Two
• Active Plugins:
  • fake-wordpress-update (see ​https://gist.github.com/hellofromtonya/bb0aa2d4b2311c8b40fa5ae8ada4dc19)

Steps to Test

I've followed the test instructions outlined here: ​https://gist.github.com/hellofromtonya/bb0aa2d4b2311c8b40fa5ae8ada4dc19

Actual Results

• ✅ Upgrade via the admin panel.

Everything works as expected.
The files have been updated, no errors during the upgrade process.
No errors on the frontend.

• ❌ Upgrade via ​WP-CLI.

I'm getting this console error:

PHP Fatal error:  Cannot declare class Requests_Cookie_Jar, because the name is already in use in /wp-includes/Requests/Cookie/Jar.php on  line 15

As I've learned during the debugging, this happens because the Requests_Cookie_Jar class is declared in the WP-CLI script. To be more specific, in phar:///path_to_the_wp-cli_phar/vendor/rmccue/requests/library/Requests/Cookie/Jar.php.

[jrf]

As the above reports both mentioned issues with WP-CLI, @schlessera and me have done some testing. Unfortunately, we've not been able to reproduce the issue, so the suspicion is that the problem is specific to Mac OS.

One thing I'd like to ask @antonvlasenko and @ironprogrammer : what version of WP-CLI did you use ?

For the record, this is how I tested:

Environment

• OS: Windows 10 Pro 22H2
• Web Server: Apache
• PHP: 7.4.33
• WordPress: 6.1.1
• WP-CLI: 2.7.1
• Active Plugins:
  • fake-wordpress-update (see ​https://gist.github.com/hellofromtonya/bb0aa2d4b2311c8b40fa5ae8ada4dc19)

The zip was created by Alain based on the instructions with the gist + one additional change:

• The $wp_version variable in wp-includes/version.php was set to 6.1.1-with-request-v2 to make it straight forward to verify the correct WP version was installed.

As for the fake plugin, that was used as provided in the gist + one additional change:

• Changed the $fake_update_location variable to point to a locally accessible URL on my system where the ZIP was placed.

I then ran the following commands from the command line:

wp core download
wp config create --dbname=trac54504 --dbuser=root --dbhost=localhost[ --dbpass=XXXX]
wp db create
wp core install --url=https://trac54504.dev --title=Trac54504 --admin_user=admin --admin_email=youremail@address.com --admin_password=password
** add the plugin in the `wp-content/plugins/` directoy **
wp plugin list
wp plugin activate fake-wordpress-update
wp core update --debug
wp core version

... and it then correctly displayed 6.1.1-with-request-v2 and a check of the file system also confirmed the update went through without a hitch.

I then downgraded again using wp core update --version=6.1.1 --force and that also worked without any issues, though the "new" Requests files in the wp-includes/Requests/src directory were not removed.

[schlessera]

Test Report

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/3732

Environment

• OS: Linux version 5.15.79.1-microsoft-standard-WSL2 on Windows 11 with Ubuntu 20.04.4 LTS
• Web Server: Built-in PHP Server using wp server
• PHP: 7.4.30
• WordPress: 6.1.1
• WP-CLI: 2.7.1
• Active Plugins:
  • fake-wordpress-update (see ​https://gist.github.com/hellofromtonya/bb0aa2d4b2311c8b40fa5ae8ada4dc19)

Steps to Test

I've followed the test instructions outlined here: ​https://gist.github.com/hellofromtonya/bb0aa2d4b2311c8b40fa5ae8ada4dc19

I used WP-CLI to set everything up and used WP-CLI to trigger the update and test its results.

Actual Results

• ✅ Upgrade via ​WP-CLI.

$ wp core update
Updating to version 6.1.1-with-requests2 (en_US)...
Using cached file '/home/alain/.wp-cli/cache/core/wordpress-with-requests-2-en_US.zip'...
Unpacking the update...
requests_auth was preloaded.
requests_hooker was preloaded.
requests_proxy was preloaded.
requests_transport was preloaded.
requests_cookie was preloaded.
requests_exception was preloaded.
requests_hooks was preloaded.
requests_idnaencoder was preloaded.
requests_ipv6 was preloaded.
requests_iri was preloaded.
requests_response was preloaded.
requests_session was preloaded.
requests_ssl was preloaded.
requests_auth_basic was preloaded.
requests_cookie_jar was preloaded.
requests_proxy_http was preloaded.
requests_response_headers was preloaded.
requests_transport_curl was preloaded.
requests_transport_fsockopen was preloaded.
requests_utility_caseinsensitivedictionary was preloaded.
requests_utility_filterediterator was preloaded.
requests_exception_http was preloaded.
requests_exception_transport was preloaded.
requests_exception_transport_curl was preloaded.
requests_exception_http_304 was preloaded.
requests_exception_http_305 was preloaded.
requests_exception_http_306 was preloaded.
requests_exception_http_400 was preloaded.
requests_exception_http_401 was preloaded.
requests_exception_http_402 was preloaded.
requests_exception_http_403 was preloaded.
requests_exception_http_404 was preloaded.
requests_exception_http_405 was preloaded.
requests_exception_http_406 was preloaded.
requests_exception_http_407 was preloaded.
requests_exception_http_408 was preloaded.
requests_exception_http_409 was preloaded.
requests_exception_http_410 was preloaded.
requests_exception_http_411 was preloaded.
requests_exception_http_412 was preloaded.
requests_exception_http_413 was preloaded.
requests_exception_http_414 was preloaded.
requests_exception_http_415 was preloaded.
requests_exception_http_416 was preloaded.
requests_exception_http_417 was preloaded.
requests_exception_http_418 was preloaded.
requests_exception_http_428 was preloaded.
requests_exception_http_429 was preloaded.
requests_exception_http_431 was preloaded.
requests_exception_http_500 was preloaded.
requests_exception_http_501 was preloaded.
requests_exception_http_502 was preloaded.
requests_exception_http_503 was preloaded.
requests_exception_http_504 was preloaded.
requests_exception_http_505 was preloaded.
requests_exception_http_511 was preloaded.
requests_exception_http_unknown was preloaded.
Warning: Checksums not available for WordPress 6.1.1-with-request-v2/en_US. Please cleanup files manually.
Success: WordPress updated successfully.

Both upgrade and downgrade work as expected with one caveat: the wp-includes/Requests/src folder is not removed on downgrading.

Observations

I tested this together with @jrf, and so far, our assumption is that the approach is sound and works fine, but that there is still an edge case that is not handled correctly on Mac OS X. Given that this system has a very peculiar filesystem regarding the handling of case sensitivity, my current bet is that there's an unlucky interaction between the case sensitivity of the autoloader and that of the filesystem here that breaks for Macs.

[antonvlasenko]

Replying to jrf:

One thing I'd like to ask what version of WP-CLI did you use ?

I used the latest WP-CLI version: 2.7.1..
It seems that @costdev has already fixed that error, so that's why you weren't able to see it.
I will add a new test report if I have time.

[costdev]

@schlessera there is still an edge case that is not handled correctly on Mac OS X. Given that this system has a very peculiar filesystem regarding the handling of case sensitivity, my current bet is that there's an unlucky interaction between the case sensitivity of the autoloader and that of the filesystem here that breaks for Macs.

I know this was an assumption, but a couple of questions:

• By "the autoloader", are you referring to _preload_old_requests_classes_and_interfaces(), or the Requests autoloader?
• What is the assumed error on Mac? Class not found, file not found, or?

[costdev]

@schlessera Both upgrade and downgrade work as expected with one caveat: the wp-includes/Requests/src folder is not removed on downgrading.

When downgrading, would WP-CLI normally remove files introduced in the "later" version? If so, how does it determine the new files - from $_new_bundled_files, or?

[antonvlasenko]

Test Report

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/3732
This test report is valid for the most recent version of that PR (head commit: ​https://github.com/WordPress/wordpress-develop/pull/3732/commits/c6a4e10aa37b96d969d2aa7606f2d8c254b79072).

Environment

• OS: macOS 12.6
• Web Server: Apache
• PHP: 8.1.9
• WordPress: 6.1.1
• Browser: Safari 16.1
• Theme: Twenty Twenty-Two
• Active Plugins:
  • fake-wordpress-update (see ​https://gist.github.com/hellofromtonya/bb0aa2d4b2311c8b40fa5ae8ada4dc19)

Steps to Test

I've followed the test instructions outlined here: ​https://gist.github.com/hellofromtonya/bb0aa2d4b2311c8b40fa5ae8ada4dc19

Actual Results

• ✅ Upgrade via the admin panel.

No errors during the upgrade process.
No errors on the frontend.

• ✅ Upgrade via ​WP-CLI.

No errors during the upgrade process.
No errors on the frontend.

[jrf]

Replying to costdev:

@schlessera Both upgrade and downgrade work as expected with one caveat: the wp-includes/Requests/src folder is not removed on downgrading.

When downgrading, would WP-CLI normally remove files introduced in the "later" version? If so, how does it determine the new files - from $_new_bundled_files, or?

I honestly don't know... 🤷🏻‍♀️ (first time I ever tried a downgrade)

[ironprogrammer]

Test Report

Tested on PHP 7.4 per custom install package instructions ​provided here.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/3732 latest (via custom install package).

Environment

• Hardware: MacBook Pro Apple M1 Pro
• OS: macOS 12.6.1 (case insensitive)
• Browser: Safari 16.1
• Server: nginx/1.23.2
• PHP: 7.4.33
• CLI: WP-CLI 2.7.1
• Active Plugins:
  • fake-wordpress-update v1.0.0 (provided in test instructions)

Actual Results

These results include the following tests per version:

• Upgrade via Dashboard > Updates succeeded without error.
• Upgrade via WP-CLI wp core update succeeded without error.

WordPress Version:

• ✅ 6.1.1
• ✅ 6.1
• ✅ 6.0.3
• ✅ 5.9.5
• ✅ 5.8.6
• ✅ 5.7.8
• ✅ 5.6.10
• ✅ 5.5.11
• ✅ 5.4.12
• ✅ 5.3.14
• ✅ 5.2.17
• ✅ 5.1.15
• ✅ 5.0.18

[SergeyBiryukov]

#57325 was marked as a duplicate.

[costdev]

@schlessera In terms of the wp-includes/Requests/src files not being cleaned up when downgrading via WP-CLI, i.e.:

Warning: Checksums not available for WordPress 6.2-alpha-54642-src/en_US. Please cleanup files manually.

Am I right that this is because Core_Command::cleanup_extra_files() ​compares checksums based on api.wordpress.org, and that the cleanup should work fine after final release when api.wordpress.org has a version to compare to?

[ironprogrammer]

Test Report

This is an automatic updates follow up to my latest test report: comment:104.

Environment

• Hardware: MacBook Pro Apple M1 Pro
• OS: macOS 12.6.1 (case insensitive)
• Browser: Safari 16.1
• Server: nginx/1.23.2
• PHP: 7.4.33
• Active Plugins:
  • fake-wordpress-update v1.0.0 (modified per ​these instructions)
  • wp-crontrol v1.15.0

Actual Results

These results include the following tests per version:

• Upgrade via automatic core update succeeded without error.

WordPress Version:

• ✅ 6.1.1
• ✅ 6.1
• ✅ 6.0.3
• ✅ 5.9.5
• ✅ 5.8.6
• ✅ 5.7.8
• ✅ 5.6.10
• ✅ 5.5.11
• ✅ 5.4.12
• ✅ 5.3.14
• ✅ 5.2.17
• ✅ 5.1.15
• ✅ 5.0.18

Additional Notes

For versions below 5.5, the following deprecations are logged after the automatic update completes:

PHP Deprecated: class-phpmailer.php is <strong>deprecated</strong> since version 5.5.0! Use wp-includes/PHPMailer/PHPMailer.php instead. The PHPMailer class has been moved to wp-includes/PHPMailer subdirectory and now uses the PHPMailer\PHPMailer namespace. in ../wp-includes/functions.php on line 4909
PHP Deprecated: class-smtp.php is <strong>deprecated</strong> since version 5.5.0! Use wp-includes/PHPMailer/SMTP.php instead. The SMTP class has been moved to the wp-includes/PHPMailer subdirectory and now uses the PHPMailer\PHPMailer namespace. in ../wp-includes/functions.php on line 4909

These did not appear following the manual or CLI updates previously tested.

[hellofromTonya]

@costdev, @ironprogrammer and @antonvlasenko confirm their testing with the latest version of ​PR 3732 works as expected. In addition, @schlessera and @jrf confirm it works in their testing.

I'm moving this ticket into 6.2 milestone and doing a final code review. I'll then prepare the commit. In doing so, this upgrade to Requests will be in Core for early testing and feedback in the 6.2 cycle.

[hellofromTonya]

My testing also confirms ​PR 3732 patch works as expected. The PR/patch LGTM. Preparing the commit now.

[hellofromTonya]

In 54997:

External Libraries: Update Requests library to version 2.0.0.

This is a major release and contains breaking changes.

Most important changes to be aware of for this release:

• All code is now namespaced. Though there is a full backward compatibility layer available and the old class names are still supported, using them will generate a deprecation notice (which can be silenced by plugins if they'd need to support multiple WP versions). See the ​upgrade guide for more details.
• A lot of classes have been marked final. This should generally not affect userland code as care has been taken to not apply the final keyword to classes which are known to be extended in userland code.
• Extensive input validation has been added to Requests. When Requests is used as documented though, this will be unnoticable.
• A new WpOrg\Requests\Requests::has_capabilities() method has been introduced which can be used to address #37708.
• A new WpOrg\Requests\Response::decode_body() method has been introduced which may be usable to simplify some of the WP native wrapper code.
• Remaining PHP 8.0 compatibility fixed (support for named parameters).
• PHP 8.1 compatibility.

Release notes: ​https://github.com/WordPress/Requests/releases/tag/v2.0.0

For a full list of changes in this update, see the Requests GitHub:
​https://github.com/WordPress/Requests/compare/v1.8.1...v2.0.0

This commit also resolves 2 blocking issues which previously caused the revert of [52244]:

• New Requests files are loaded into wp-includes/Requests/src/, matching the location of the library. In doing so, filesystems that are case-insensitive are not impacted (see #54582).
• Preload: During a Core update, the old Requests files are preloaded into memory before the update deletes the files. Preloading avoids fatal errors noted in #54562.

Follow-up to [50842], [51078], [52244], [52315], [52327], [52328].

Props jrf, schlessera, datagutten, wojsmol, dustinrue, soulseekah, szepeviktor. costdev, sergeybiryukov, peterwilsoncc, ironprogrammer, antonvlasenko, hellofromTonya, swissspidy, dd32, azaozz, TobiasBg, audrasjb.
Fixes #54504.
See #54582, #54562.

Commit via changeset https://core.trac.wordpress.org/changeset/54997.

Commit via changeset https://core.trac.wordpress.org/changeset/54997.

[hellofromTonya]

Apologies for 2 props issues in the changeset:

• . between szepeviktor. costdev instead of a ,.
• Missed mukesh27.

These prop issues were fixed in the props editor.

Thank you everyone for your contributions!

[bjorsch]

We found one bug in this. Details in #57341. Should be easy to fix, I just don't know how exactly you'll want to do it.

[danielbachhuber]

I haven't had a chance to look into it fully yet, but I'm seeing ​this failure in WP-CLI's tests:

 002 Scenario: Installing respects WP_PROXY_HOST and WP_PROXY_PORT # features/plugin-install.feature:86
      Then STDERR should contain:                                 # features/plugin-install.feature:96
        $ wp --require=invalid-proxy-details.php plugin install edit-flow
        
        PHP Fatal error:  Class 'WpOrg\Requests\Proxy\HTTP' not found in /tmp/wp-cli-test-run--63bb77003f5099.33183249/wp-includes/class-wp-http.php on line 382
        Fatal error: Class 'WpOrg\Requests\Proxy\HTTP' not found in /tmp/wp-cli-test-run--63bb77003f5099.33183249/wp-includes/class-wp-http.php on line 382
        Error: There has been a critical error on this website.Learn more about troubleshooting WordPress. There has been a critical error on this website.
        cwd: /tmp/wp-cli-test-run--63bb77003f5099.33183249/
        run time: 0.55791783332825
        exit status: 255 (Exception)

Maybe ​this class reference needs to be updated to WpOrg\Requests\Proxy\Http?

[jrf]

Fix case-issue in Requests reference

[jrf]

Replying to danielbachhuber:

Maybe ​this class reference needs to be updated to WpOrg\Requests\Proxy\Http?

Oh dear, how did that slip in ? Thanks for catching this @danielbachhuber ! I 100% agree that should be fixed and I've uploaded a patch to fix it.

[SergeyBiryukov]

In 55046:

HTTP API: Use correct class reference for Requests' HTTP Proxy in WP_Http::request().

Renaming the class was missed in [54997] when updating changes in WP_Http::request() for the Requests 2.0.0 external library upgrade. The HTTP class no longer exists and caused a fatal error:

PHP Fatal error: Class 'WpOrg\Requests\Proxy\HTTP' not found in wp-includes/class-wp-http.php on line 382

This commit renames the class to Http and resolves the fatal error.

Follow-up to [52244], [52315], [52327], [52328], [54997], [55007].

Props danielbachhuber, jrf.
See #54504.

[swissspidy]

#57511 was marked as a duplicate.

Intentional preloading of old Requests classes and interfaces should not produce deprecation notices.

This PR defines REQUESTS_SILENCE_PSR0_DEPRECATIONS as true in _preload_old_requests_classes_and_interfaces().

Trac ticket: https://core.trac.wordpress.org/ticket/54504

[costdev]

Upgrading to ​the nightly package produces the deprecation notice in Requests:

PHP Deprecated: The PSR-0 Requests_... class names in the Request library are deprecated. Switch to the PSR-4 WpOrg\Requests\... class names at your earliest convenience.

This deprecation notice is expected for extenders. However, as the preloading is intended to run in Core upgrades for the foreseeable future, it doesn't make sense to have this deprecation notice during the Core upgrade. It would also lead to numerous unnecessary support tickets/posts.

I ran a test with a modified upgrade package, and added this to the top of _preload_old_requests_classes_and_interfaces(). The test was successful: The deprecation notice was silenced during the upgrade.

if ( ! defined( 'REQUESTS_SILENCE_PSR0_DEPRECATIONS' ) ) {
        define( 'REQUESTS_SILENCE_PSR0_DEPRECATIONS', true );
}

Could I get a confidence check on this and on ​PR 3998?

---

• Props to @afragen for discovering this while testing other 6.2 features.

Tested and it works!

Aside from the description being not completely correct, the actual fix looks good to me.

{{{diff
-Intentional preloading of old Requests classes and interfaces should not produce deprecation notices.
+Intentional preloading of Requests 2.x classes and interfaces using their old/Requests 1.x names, should not produce deprecation notices.
}}}

[SergeyBiryukov]

In 55225:

Upgrade/Install: Silence Requests deprecations before preloading.

Intentional preloading of Requests 2.x classes and interfaces using their old (Requests 1.x) names should not produce deprecation notices.

This commit defines REQUESTS_SILENCE_PSR0_DEPRECATIONS as true in _preload_old_requests_classes_and_interfaces().

Follow-up to [54997], [55007], [55046].

Props costdev, afragen, jrf.
Fixes #54504.

Thanks for the PR! Merged in r55225.

[hellofromTonya]

#57738 was marked as a duplicate.

[hellofromTonya]

#57738 was marked as a duplicate.

[SergeyBiryukov]

#57820 was marked as a duplicate.

[costdev]

#57972 was marked as a duplicate.

[SergeyBiryukov]

Replying to drzraf:

Deprecated: Return type of Requests_Cookie_Jar::offsetExists($key) should either be compatible with ArrayAccess::offsetExists(mixed $offset): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /wp/wp-includes/Requests/Cookie/Jar.php on line 63

Hmm, that file no longer exists at that path in 6.2, it was moved to the wp-includes/Requests/src/Cookie/ directory, and line 63​ no longer has any code in it.

Might be a good idea to confirm that the site did in fact successfully update to 6.2, e.g. by doing a full file comparison.

[drzraf]

Replying to SergeyBiryukov:

Might be a good idea to confirm that the site did in fact successfully update to 6.2, e.g. by doing a full file comparison.

You're right, it wasn't. Sorry for the noise.

[SergeyBiryukov]

No worries, thanks for the follow-up!

[afragen]

I appreciate that this ticket is closed and all references to the following issue that I currently have are marked as duplicates of this ticket.

Bug Report

Description

Still getting numerous PHP Deprecated notices in my error log from Requests

Environment

• WordPress: 6.4-beta2-56791
• PHP: 8.1.16
• Server: nginx/1.22.0
• Database: mysqli (Server: 8.0.30 / Client: 8.1.16)
• Browser: Safari 17.0
• OS: macOS
• Theme: Frost 1.0.5
• MU Plugins:
  • SpinupWP Debug Log Path 1.0
  • Test MU
• Plugins:
  • Add Custom Header Images 2.3.3
  • Auto-Flush Object Cache 0.7.0
  • Core Rollback 1.3.5
  • Embed PDF Viewer 2.3.1
  • Git Remote Updater 3.1.1
  • Git Updater - Bitbucket 2.0.3
  • Git Updater - Gist 2.0.3
  • Git Updater - GitLab 2.0.3
  • Git Updater 12.3.0.2
  • Handbook Callout Blocks 1.0.0-beta5
  • Limit Login Attempts Reloaded 2.25.25
  • Local Development 2.8.3
  • Login Logout Primary Menu Item 0.5.0
  • oEmbed Gists and Files 1.0.1
  • Plugin Dependencies 3.0.0
  • Query Monitor 3.13.1
  • RAU Edge Case - Defining Class in main plugin file. 0.1.0
  • RAU Edge Case - Defining Constants in main plugin file. 0.1.0
  • Rollback Update Failure 6.3.1
  • Site Testing 0.4
  • Skip Updates 1.2.0
  • SpinupWP 1.5.1
  • Stop Auto-update Success Email 0.2.0
  • Test Reports 0.3.1
  • Updates API Inspector 0.1.1
  • User Switching 1.7.0
  • What The Cron 0.1.2
  • WordPress Beta Tester 3.5.4
  • WP Author Slug 4
  • WP Crontrol 1.15.3
  • WP Debugging 2.11.22
  • WP Mail SMTP 3.9.0
  • WP Mail SMTP Settings 0.5.1
  • WP Site Tweaks 1.0.10

Steps to Reproduce

Just running the site and updating plugins and core.

Expected Results

No more PHP Deprecation notices

Actual Results

[06-Oct-2023 03:31:09 UTC] PHP Deprecated: Return type of Requests_Cookie_Jar::offsetExists($key) should either be compatible with ArrayAccess::offsetExists(mixed $offset): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/local/bin/wp/vendor/rmccue/requests/library/Requests/Cookie/Jar.php on line 63
[06-Oct-2023 03:31:09 UTC] PHP Deprecated: Return type of Requests_Cookie_Jar::offsetGet($key) should either be compatible with ArrayAccess::offsetGet(mixed $offset): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/local/bin/wp/vendor/rmccue/requests/library/Requests/Cookie/Jar.php on line 73
[06-Oct-2023 03:31:09 UTC] PHP Deprecated: Return type of Requests_Cookie_Jar::offsetSet($key, $value) should either be compatible with ArrayAccess::offsetSet(mixed $offset, mixed $value): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/local/bin/wp/vendor/rmccue/requests/library/Requests/Cookie/Jar.php on line 89
[06-Oct-2023 03:31:09 UTC] PHP Deprecated: Return type of Requests_Cookie_Jar::offsetUnset($key) should either be compatible with ArrayAccess::offsetUnset(mixed $offset): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/local/bin/wp/vendor/rmccue/requests/library/Requests/Cookie/Jar.php on line 102
[06-Oct-2023 03:31:09 UTC] PHP Deprecated: Return type of Requests_Cookie_Jar::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/local/bin/wp/vendor/rmccue/requests/library/Requests/Cookie/Jar.php on line 111
[06-Oct-2023 03:31:09 UTC] PHP Deprecated: Return type of Requests_Utility_CaseInsensitiveDictionary::offsetExists($key) should either be compatible with ArrayAccess::offsetExists(mixed $offset): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/local/bin/wp/vendor/rmccue/requests/library/Requests/Utility/CaseInsensitiveDictionary.php on line 40
[06-Oct-2023 03:31:09 UTC] PHP Deprecated: Return type of Requests_Utility_CaseInsensitiveDictionary::offsetGet($key) should either be compatible with ArrayAccess::offsetGet(mixed $offset): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/local/bin/wp/vendor/rmccue/requests/library/Requests/Utility/CaseInsensitiveDictionary.php on line 51
[06-Oct-2023 03:31:09 UTC] PHP Deprecated: Return type of Requests_Utility_CaseInsensitiveDictionary::offsetSet($key, $value) should either be compatible with ArrayAccess::offsetSet(mixed $offset, mixed $value): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/local/bin/wp/vendor/rmccue/requests/library/Requests/Utility/CaseInsensitiveDictionary.php on line 68
[06-Oct-2023 03:31:09 UTC] PHP Deprecated: Return type of Requests_Utility_CaseInsensitiveDictionary::offsetUnset($key) should either be compatible with ArrayAccess::offsetUnset(mixed $offset): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/local/bin/wp/vendor/rmccue/requests/library/Requests/Utility/CaseInsensitiveDictionary.php on line 82
[06-Oct-2023 03:31:09 UTC] PHP Deprecated: Return type of Requests_Utility_CaseInsensitiveDictionary::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/local/bin/wp/vendor/rmccue/requests/library/Requests/Utility/CaseInsensitiveDictionary.php on line 91
[06-Oct-2023 03:31:09 UTC] PHP Deprecated: Return type of Requests_Utility_FilteredIterator::unserialize($serialized) should either be compatible with ArrayIterator::unserialize(string $data): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/local/bin/wp/vendor/rmccue/requests/library/Requests/Utility/FilteredIterator.php on line 53
[06-Oct-2023 03:31:09 UTC] PHP Deprecated: Return type of Requests_Utility_FilteredIterator::__unserialize($serialized) should either be compatible with ArrayIterator::__unserialize(array $data): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/local/bin/wp/vendor/rmccue/requests/library/Requests/Utility/FilteredIterator.php on line 60
[06-Oct-2023 03:31:09 UTC] PHP Deprecated: Return type of Requests_Utility_FilteredIterator::current() should either be compatible with ArrayIterator::current(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in phar:///usr/local/bin/wp/vendor/rmccue/requests/library/Requests/Utility/FilteredIterator.php on line 40

[afragen]

Sorry, It seems this was coming from my server's version of WP-CLI which was 2.7.1. I've updated.