Bug 171700

Summary: [Cocoa] Converting from WebCore::Cookie to NSHTTPCookie always marks cookies as secure
Product: WebKit Reporter: Blaze Burg <bburg>
Component: WebKit2Assignee: Blaze Burg <bburg>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, beidson, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch beidson: review+, beidson: commit-queue-

Blaze Burg
Reported 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].
Attachments
Patch (4.63 KB, patch)
2017-05-04 21:40 PDT, Blaze Burg 		beidson: review+
beidson: commit-queue-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Blaze Burg
Comment 1 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.
Blaze Burg
Comment 2 2017-05-04 17:34:31 PDT
<rdar://problem/29829930>
Blaze Burg
Comment 3 2017-05-04 21:40:21 PDT
Created attachment 309143 [details] Patch
Brady Eidson
Comment 4 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.
Alexey Proskuryakov
Comment 5 2017-05-04 22:37:11 PDT
Nice catch!
Blaze Burg
Comment 6 2017-05-05 10:04:28 PDT
<rdar://problem/32017975>
Blaze Burg
Comment 7 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.
Blaze Burg
Comment 8 2017-05-05 12:21:43 PDT
Committed r216258: <http://trac.webkit.org/changeset/216258>
Note You need to log in before you can comment on or make changes to this bug.