#54225 closed enhancement (fixed)
Request header key case inconsistencies
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.2 | Priority: | normal |
Severity: | normal | Version: | 2.7 |
Component: | HTTP API | Keywords: | has-patch has-unit-tests commit |
Focuses: | docs | Cc: |
Description
RFCs 7230 and 7540 define header keys as case-insensitive.
WordPress is consistently inconsistent when it comes to :
- Retrieving headers – always
lowercase
:wp_remote_retrieve_header( $response, 'content-md5' )`
- Setting headers – almost always
Capital-Case
:header( 'Content-Type: text/html; charset=utf-8' );
- Sitemaps code (from 5.5) uses
Title-case
:header( 'Content-type: application/xml; charset=UTF-8' );
(Thankfully, when it comes to actually using the wp_remote_retrieve_header()
function (and subsequently the Requests API and WP4.6) it resolves down to Requests_Utility_CaseInsensitiveDictionary
so it is all working as defined in the specification.)
Possible code/docs improvements here include:
- standardizing when core uses which case
- explicitly stating that header key parameters are case-insensitive
Attachments (1)
Change History (21)
#4
@
3 years ago
- Milestone changed from 6.0 to 6.1
With Beta 1 released yesterday, I'm moving this ticket to the 6.1 milestone.
#5
in reply to:
↑ description
@
3 years ago
Replying to johnjamesjacoby:
Possible code/docs improvements here include:
- standardizing when core uses which case
- explicitly stating that header key parameters are case-insensitive
Looking at the core files (excluding external libraries):
Content-Type
- 89 instances of
Content-Type
- 27 instances of
content-type
- 9 instances of
Content-type
- 89 instances of
Content-Disposition
- 13 instances of
Content-Disposition
- 2 instances of
content-disposition
- 13 instances of
Content-MD5
- 1 instance of
Content-MD5
- 1 instance of
content-md5
- 1 instance of
These numbers include DocBlocks and inline comments, so may not be 100% accurate, but should work as a quick overview. It looks like we can standardize on Content-Type
, Content-Disposition
, Content-Type
, etc.
#6
@
3 years ago
I've created a patch:
- Header creation now always used the capitalized forms, also with
$request->add_header
and$request->set_header
(mainly called in thetests
) - Calls to
wp_remote_retrieve_header
andwp_get_http_headers
now always use a capitalized term as the second argument. - Docblocks and inline comments are updated to reflect the fact that we use capitalized terms in the code.
- I also updated the deprecated files
src/wp-includes/class-json.php
andsrc/wp-includes/class-snoopy.php
for consistency. - The return value of
wp_get_http_headers
was updated to reflect that aRequests_Utility_CaseInsensitiveDictionary
is returned, rather than astring
.
This ticket was mentioned in PR #3243 on WordPress/wordpress-develop by mhkuu.
3 years ago
#7
- Keywords has-patch has-unit-tests added; needs-patch removed
Improves the consistency in capitalization of fetching and outputting of request headers. See the Trac ticket for details.
Trac ticket: https://core.trac.wordpress.org/ticket/54225
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
3 years ago
#10
@
3 years ago
- Milestone changed from 6.1 to 6.2
As discussed ahead of today's scheduled 6.1 Beta 1 release party (see the Slack link above), it was deemed that this ticket was not ready and should be punted to the 6.2 milestone to be reviewed and addressed then.
#12
@
2 years ago
- Keywords changes-requested added; 2nd-opinion removed
I've requested that two external library files are removed from PR 3243. Adding the changes-requested
keyword.
Also looks like the 2nd-opinion
keyword was added at the ticket's creation. As this ticket has commits, I think we're good to remove this keyword now.
#13
@
2 years ago
- Keywords changes-requested removed
Thanks @costdev for checking! I've removed the changes to these external library files. I also merged trunk
into the PR and resolved the merge conflicts.
This ticket was mentioned in Slack in #core by costdev. View the logs.
2 years ago
#15
@
2 years ago
- Keywords commit added
This ticket was discussed during the bug scrub. Feedback on the PR has been addressed and this looks ready for commit
consideration. Adding the appropriate keyword.
Additional props: @audrasjb @petitphp
#16
@
2 years ago
- Owner set to audrasjb
- Resolution set to fixed
- Status changed from new to closed
In 55210:
@audrasjb commented on PR #3243:
2 years ago
#17
Committed in https://core.trac.wordpress.org/changeset/55210
5.9 is in feature freeze and Beta 1 release is < 4 hours away, moving this to 6.0.