Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#37722 closed enhancement (fixed)

wp_remote_retrieve_headers no longer an array in WordPress 4.6

Reported by: jstkrn's profile jstkrn Owned by: dd32's profile dd32
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.6
Component: HTTP API Keywords: good-first-bug has-patch
Focuses: docs Cc:

Description

With the introduction of Requests in PHP 4.6, headers are returned as a Requests_Utility_CaseInsensitiveDictionary object instead of a native PHP array, see wp-includes/class-wp-http-requests-response.php > get_headers. Although this object implements ArrayAccess, it technically is no longer an array and this causes a plugin of mine to break due to incompatible types, as I assert that an array is given.

Documentation and docblocks needs to be updated to reflect this change.

Attachments (3)

37722.diff (849 bytes) - added by mrahmadawais 8 years ago.
FIX: DocBlock return to object instead of headers' array & PHPCS Errors
37722.2.diff (906 bytes) - added by mrahmadawais 8 years ago.
FIX: DocBlock return to object instead of headers' array & PHPCS Errors
37722.3.diff (1.1 KB) - added by sudar 8 years ago.

Download all attachments as: .zip

Change History (14)

#1 follow-up: @swissspidy
8 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.6.1
  • Type changed from enhancement to defect (bug)

Previously, there was the same issue with responses, see [37989]. We should be consistent with return types, so I'd consider this a bug.

#2 in reply to: ↑ 1 @jstkrn
8 years ago

Replying to swissspidy:

Previously, there was the same issue with responses, see [37989]. We should be consistent with return types, so I'd consider this a bug.

This is indeed likely to break some more plugins, given that typehints fail and is_array now returns false, but I also embrace changes such as the introduction of Requests and headers now being case-insensitive is a nice feature. Hence the suggestion to keep the current implementation and updating the docs (although that may not fit with WordPress's compatibility policy, I wouldn't know)

#3 @dd32
8 years ago

Previously, there was the same issue with responses, see [37989]. We should be consistent with return types, so I'd consider this a bug.

We only really changed the return type for the main request object due to the number of plugins who would assume !is_array() was a failed HTTP request, even though that was a bad habit to be in.

We saw that this may also have the same issue, however deliberately chose to break this not-really-supported scenario, the benefits of the case insensitive dictionary are worth it, and we I don't think we could easily present the original-cased headers anyway.

As such, I consider this a documentation update, which doesn't need to be in 4.6.1 (unless someone else feels it should be)

#4 @swissspidy
8 years ago

  • Focuses docs added
  • Milestone changed from 4.6.1 to 4.7
  • Type changed from defect (bug) to enhancement

@dd32 Thanks for your input. I'm fine with anything that doesn't need to be in 4.6.1 :-) Can always reconsider of course if it turns out to be a bigger problem.

#5 @jorbin
8 years ago

  • Keywords good-first-bug added

Improving the documentation could be a good first bug.

@mrahmadawais
8 years ago

FIX: DocBlock return to object instead of headers' array & PHPCS Errors

#6 @mrahmadawais
8 years ago

  • Keywords has-patch dev-feedback added; needs-patch removed

Added a patch to fix the docs as well as two minor errors in accordance with PHPCS.

#7 follow-up: @swissspidy
8 years ago

  • Keywords dev-feedback removed

@mrahmadawais Thanks for the patch! It would be great if the exact object type (Requests_Utility_CaseInsensitiveDictionary) could be noted in the doc block instead of simply object.

Not sure if we need a @since 4.6.0 return value changed […] entry.

@mrahmadawais
8 years ago

FIX: DocBlock return to object instead of headers' array & PHPCS Errors

#8 in reply to: ↑ 7 ; follow-up: @mrahmadawais
8 years ago

Replying to swissspidy:

@mrahmadawais Thanks for the patch! It would be great if the exact object type (Requests_Utility_CaseInsensitiveDictionary) could be noted in the doc block instead of simply object.

Fixed that in the second patch.

Not sure if we need a @since 4.6.0 return value changed […] entry.

Lost you here, what are you talking about?

#9 in reply to: ↑ 8 @swissspidy
8 years ago

Replying to mrahmadawais:

Replying to swissspidy:

@mrahmadawais Thanks for the patch! It would be great if the exact object type (Requests_Utility_CaseInsensitiveDictionary) could be noted in the doc block instead of simply object.

Fixed that in the second patch.

Not sure if we need a @since 4.6.0 return value changed […] entry.

Lost you here, what are you talking about?

I was thinking if we need to add a note to inform developers that the return type changed.

#10 @sudar
8 years ago

Made the following changes to the patch

  • Added a note that return value got changed in v4.7.0
  • Tweaked the return value to reflect the class name.

@sudar
8 years ago

#11 @dd32
8 years ago

  • Owner set to dd32
  • Resolution set to fixed
  • Status changed from new to closed

In 38730:

HTTP: Document that the return value of wp_remote_retrieve_headers() changed from a simple array to an object which implements ArrayAccess.

Props mrahmadawais, sudar, swissspidy.
Fixes #37722

Note: See TracTickets for help on using tickets.