WordPress.org

Make WordPress Core

Opened 4 months ago

Last modified 4 months ago

#43231 assigned defect (bug)

Wp_Http_Cookie host-only flag

Reported by: soulseekah Owned by: soulseekah
Milestone: 5.0 Priority: normal
Severity: normal Version:
Component: HTTP API Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

The Wp_Http_Cookie as parsed from the Requests_Cookie does away with some very important information - the host-only flag.

Consider the following example header that a remote server sends:

Set-Cookie: test=12345; domain=".example.org"

This is transformed into a Requests_Cookie with the domain attribute set as "example.org" (as per RFC 6265 https://tools.ietf.org/html/rfc6265#section-5.3 point 6 (domain-attribute storage model)) and a "host-only" flag is set to false, which means that the cookie can be sent to any subdomains within the domain.

Then the Request_Cookie is transformed into a Wp_Http_Cookie, which doesn't store any of the flags at all. This results in a cookie that is only valid for the main domain, since "host-only" is a true-by-default flag.

It is now impossible to send the same Wp_Http_Cookie instance to the next request as it gets filtered out by Request_Cookie domain-matching https://github.com/rmccue/Requests/blob/master/library/Requests/Cookie.php#L131) and never gets sent.

The only workaround is to null the domain property of the Wp_Http_Cookie to allow it to be sent to a subdomain.

So in short, Wp_Http_Cookie is losing very important information, i.e. the difference between ".example.org" and "example.org".

Attachments (3)

43231.tests.patch (1.1 KB) - added by soulseekah 4 months ago.
Initial tests
43231.1.patch (4.0 KB) - added by soulseekah 4 months ago.
Initial patch
43231.2.tests.patch (1.4 KB) - added by soulseekah 4 months ago.
Reworked tests without remote call

Download all attachments as: .zip

Change History (7)

#1 @johnbillion
4 months ago

  • Keywords needs-patch needs-unit-tests added

@soulseekah
4 months ago

Initial tests

@soulseekah
4 months ago

Initial patch

#2 @soulseekah
4 months ago

  • Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed

#3 @johnbillion
4 months ago

  • Owner set to soulseekah
  • Status changed from new to assigned

Thanks for the patch, @soulseekah! This looks like something that should be able to be unit tested without the need to perform an external HTTP request. What do you think? Can you take a look?

@soulseekah
4 months ago

Reworked tests without remote call

#4 @SergeyBiryukov
4 months ago

  • Milestone changed from Awaiting Review to 5.0
Note: See TracTickets for help on using tickets.