[webkit-reviews] review granted: [Bug 165508] Add wildcard to Access-Control-Allow-Methods and Access-Control-Allow-Headers : [Attachment 356815] Patch

[bugzilla-daemon at webkit.org]

Frédéric Wang (:fredw) <fred.wang at free.fr> has granted Rob Buis
<rbuis at igalia.com>'s request for review:
Bug 165508: Add wildcard to Access-Control-Allow-Methods and
Access-Control-Allow-Headers
https://bugs.webkit.org/show_bug.cgi?id=165508

Attachment 356815: Patch

https://bugs.webkit.org/attachment.cgi?id=356815&action=review




--- Comment #27 from Frédéric Wang (:fredw) <fred.wang at free.fr> ---
Comment on attachment 356815
  --> https://bugs.webkit.org/attachment.cgi?id=356815
Patch

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

> Source/WebCore/ChangeLog:3
> +	   Add wildcard to Access-Control-Expose-Headers,
Access-Control-Allow-Methods, and Access-Control-Allow-Headers

I guess you can update the bug title then

> Source/WebCore/ChangeLog:10
> +	   add this to the check. Same for ccess-Control-Allow-Headers (step
6.7).

Again, A is missing at ccess-Control-Allow-Headers

> Source/WebCore/loader/CrossOriginPreflightResultCache.cpp:86
> +    if (m_methods.contains(method) || (m_methods.contains("*") &&
storedCredentialsPolicy == StoredCredentialsPolicy::DoNotUse) ||
isOnAccessControlSimpleRequestMethodWhitelist(method))

So StoredCredentialsPolicy is the same as the spec's credentials mode
(https://fetch.spec.whatwg.org/#concept-request-credentials-mode)? If so then
probably we should use the same name conventions in the future. Also, here and
below steps 6.5 and 6.7 say we should really check that the credentials mode is
not "include" i.e. storedCredentialsPolicy != StoredCredentialsPolicy::Use so
that it still works when we have more than two values.