Bug 171700 - [Cocoa] Converting from WebCore::Cookie to NSHTTPCookie always marks cookies as secure
Summary: [Cocoa] Converting from WebCore::Cookie to NSHTTPCookie always marks cookies ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: BJ Burg
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-05-04 16:48 PDT by BJ Burg
Modified: 2017-05-05 12:21 PDT (History)
3 users (show)

See Also:


Attachments
Patch (4.63 KB, patch)
2017-05-04 21:40 PDT, BJ Burg
beidson: review+
beidson: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2017-05-04 16:48:23 PDT
Per <https://developer.apple.com/reference/foundation/nshttpcookiesecure>,

Passing NSHTTPCookieSecure with any value–including @NO–to +[NSHTTPCookie cookieWithProperties:] will create a cookie with the "Secure" flag. The code currently boxes a BOOL into an NSBoolean unconditionally, causing all cookies to have the secure flag.

This only seems to practically affect cookies created via WebCookieManager[Proxy].
Comment 1 BJ Burg 2017-05-04 17:34:00 PDT
(In reply to Brian Burg from comment #0)
>
> This only seems to practically affect cookies created via
> WebCookieManager[Proxy].

This affects the new WKHTTPCookieStore API as well, but a bug in the API test masked the round tripping failure. I'll post a patch fixing both.
Comment 2 BJ Burg 2017-05-04 17:34:31 PDT
<rdar://problem/29829930>
Comment 3 BJ Burg 2017-05-04 21:40:21 PDT
Created attachment 309143 [details]
Patch
Comment 4 Brady Eidson 2017-05-04 22:07:53 PDT
Comment on attachment 309143 [details]
Patch

Yay!

Looks like you missed the 3rd round of comparisons in the cookie API test, though.

R+ with fixing that.
Comment 5 Alexey Proskuryakov 2017-05-04 22:37:11 PDT
Nice catch!
Comment 6 BJ Burg 2017-05-05 10:04:28 PDT
<rdar://problem/32017975>
Comment 7 BJ Burg 2017-05-05 10:48:10 PDT
(In reply to Brady Eidson from comment #4)
> Comment on attachment 309143 [details]
> Patch
> 
> Yay!
> 
> Looks like you missed the 3rd round of comparisons in the cookie API test,
> though.
> 
> R+ with fixing that.

Will do, thanks.
Comment 8 BJ Burg 2017-05-05 12:21:43 PDT
Committed r216258: <http://trac.webkit.org/changeset/216258>