Summary: | Prevent non app-bound domain cookies from being read or set using API calls | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kate Cheney <katherine_cheney> | ||||||||
Component: | WebKit Misc. | Assignee: | Kate Cheney <katherine_cheney> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | beidson, bfulgham, cdumez, thorton, wilander | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Kate Cheney
2020-04-02 13:45:56 PDT
Created attachment 395312 [details]
Patch
Created attachment 395318 [details]
Patch
Comment on attachment 395318 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=395318&action=review > Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:42 > +#define HTTP_COOKIE_STORE_ADDITIONS_1 false > +#define HTTP_COOKIE_STORE_ADDITIONS_2 Working with Kate out-of-band on a better name for these > Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:65 > +void HTTPCookieStore::filterAppBoundCookies(const Vector<WebCore::Cookie>& cookies, CompletionHandler<void(Vector<WebCore::Cookie>&&)>&& completionHandler) I'd change this to: Vector<WebCore::Cookie> HTTPCookieStore::filterAppBoundCookies(const Vector<WebCore::Cookie>& cookies) {...} Have it return the filtered vector. And have the caller call the completion handler. > Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:96 > + filterAppBoundCookies(allCookies, WTFMove(completionHandler)); "Filter" is an operation completely distinct from calling a completion handler. Make my above change, and this becomes: completionHandler(filterAppBoundCookies(allCookies)); Reads nicer. > Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:103 > + filterAppBoundCookies(cookies, WTFMove(completionHandler)); Ditto. > Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:115 > + filterAppBoundCookies(cookies, WTFMove(completionHandler)); Ditto. > Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:121 > + filterAppBoundCookies(cookies, [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (auto&& appBoundCookies) mutable { Can get rid of the lambda here and just continue executing code on the resulting vector. (In reply to Brady Eidson from comment #4) > Comment on attachment 395318 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=395318&action=review > > > Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:42 > > +#define HTTP_COOKIE_STORE_ADDITIONS_1 false > > +#define HTTP_COOKIE_STORE_ADDITIONS_2 > > Working with Kate out-of-band on a better name for these > > > Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:65 > > +void HTTPCookieStore::filterAppBoundCookies(const Vector<WebCore::Cookie>& cookies, CompletionHandler<void(Vector<WebCore::Cookie>&&)>&& completionHandler) > > I'd change this to: > > Vector<WebCore::Cookie> HTTPCookieStore::filterAppBoundCookies(const > Vector<WebCore::Cookie>& cookies) {...} > > Have it return the filtered vector. > > And have the caller call the completion handler. I did not note this at the time - THere's an inner async lambda in here, necessitating passing in the outer lambda. NM! > > > Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:96 > > + filterAppBoundCookies(allCookies, WTFMove(completionHandler)); > > "Filter" is an operation completely distinct from calling a completion > handler. > > Make my above change, and this becomes: > completionHandler(filterAppBoundCookies(allCookies)); > > Reads nicer. > > > Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:103 > > + filterAppBoundCookies(cookies, WTFMove(completionHandler)); > > Ditto. > > > Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:115 > > + filterAppBoundCookies(cookies, WTFMove(completionHandler)); > > Ditto. > > > Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:121 > > + filterAppBoundCookies(cookies, [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (auto&& appBoundCookies) mutable { > > Can get rid of the lambda here and just continue executing code on the > resulting vector. Comment on attachment 395318 [details]
Patch
Change the names as we discussed then this is good
Created attachment 395415 [details]
Patch for landing
(In reply to Brady Eidson from comment #6) > Comment on attachment 395318 [details] > Patch > > Change the names as we discussed then this is good Thank you! Committed r259520: <https://trac.webkit.org/changeset/259520> All reviewed patches have been landed. Closing bug and clearing flags on attachment 395415 [details]. |