Bug 188609 - [Curl] Implement default cookie path handling correctly as outlined in RFC6265.
Summary: [Curl] Implement default cookie path handling correctly as outlined in RFC6265.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Basuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-15 11:54 PDT by Basuke Suzuki
Modified: 2018-08-15 15:04 PDT (History)
10 users (show)

See Also:

Attachments
PATCH (4.67 KB, patch)
2018-08-15 12:17 PDT, Basuke Suzuki 		no flags Details | Formatted Diff | Diff
PATCH (4.67 KB, patch)
2018-08-15 13:03 PDT, Basuke Suzuki 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Basuke Suzuki 2018-08-15 11:54:51 PDT 
Curl implementation of default cookie path was wrong so that some cookies cannot be accessible. It should be generated as outlined in:
    https://tools.ietf.org/html/rfc6265#section-5.1.4
Comment 1 Basuke Suzuki 2018-08-15 12:17:00 PDT 
Created attachment 347192 [details]
PATCH
Comment 2 Basuke Suzuki 2018-08-15 13:03:00 PDT 
Created attachment 347196 [details]
PATCH
Comment 3 Alex Christensen 2018-08-15 13:07:51 PDT 
Comment on attachment 347196 [details]
PATCH

View in context: https://bugs.webkit.org/attachment.cgi?id=347196&action=review

> Source/WebCore/platform/network/curl/CookieUtil.cpp:189
> +    if (!lastSlashPosition)

notFound
Comment 4 Christopher Reid 2018-08-15 14:16:16 PDT 
(In reply to Alex Christensen from comment #3)
> Comment on attachment 347196 [details]
> PATCH
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=347196&action=review
> 
> > Source/WebCore/platform/network/curl/CookieUtil.cpp:189
> > +    if (!lastSlashPosition)
> 
> notFound

I think this confusion is from the style checker complaining about comparison with 0 and !lastSlashPosition was done to make it happy.
We do actually want to check if the lastSlashPosition is 0 so '/path' returns '/' instead of empty string. We also know that there is always at least one slash in the path string from the condition above.

I think it should be changed to lastSlashPosition == 0 ignoring the style checker.
Comment 5 Alex Christensen 2018-08-15 14:35:59 PDT 
Comment on attachment 347196 [details]
PATCH

View in context: https://bugs.webkit.org/attachment.cgi?id=347196&action=review

>>> Source/WebCore/platform/network/curl/CookieUtil.cpp:189
>>> +    if (!lastSlashPosition)
>> 
>> notFound
> 
> I think this confusion is from the style checker complaining about comparison with 0 and !lastSlashPosition was done to make it happy.
> We do actually want to check if the lastSlashPosition is 0 so '/path' returns '/' instead of empty string. We also know that there is always at least one slash in the path string from the condition above.
> 
> I think it should be changed to lastSlashPosition == 0 ignoring the style checker.

If path is null, reverseFind returns notFound.  We already have a check for that above, so no problem here.
Comment 6 WebKit Commit Bot 2018-08-15 15:03:36 PDT 
Comment on attachment 347196 [details]
PATCH

Clearing flags on attachment: 347196

Committed r234901: <https://trac.webkit.org/changeset/234901>
Comment 7 WebKit Commit Bot 2018-08-15 15:03:37 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2018-08-15 15:04:34 PDT 
<rdar://problem/43352196>