Bug 172357 – Access-Control-Expose-Headers parsed incorrectly

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Anne van Kesteren]

Test: https://github.com/w3c/web-platform-tests/pull/6000.

In https://bugzilla.mozilla.org/show_bug.cgi?id=1364598 it was discovered that only Firefox handles this correctly out of the four browser engines. We'd appreciate if you could fix this. If you don't feel like you could fix this, please propose a change to the Fetch standard (and what change you'd like that to be) instead.

[Rob Buis]

Created attachment 378808 [details]
Patch

[Rob Buis]

Created attachment 378813 [details]
Patch

[Rob Buis]

Created attachment 378814 [details]
Patch

[Rob Buis]

Created attachment 378819 [details]
Patch

[Rob Buis]

Note that the test is all PASSes on Catalina, I assume because of a CFNetwork fix/change.

[youenn fablet]

Comment on attachment 378819 [details]
Patch

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

> Source/WebCore/platform/network/HTTPParsers.h:148
>      set.add(string.substring(start, end - start + 1));

We are creating a string twice, can we optimise this?
We could try to use a StringView for the isValidHTTPToken case.

> Source/WebCore/platform/network/HTTPParsers.h:162
> +                return set;

Why doing set.clear() and return Set.
Can we just do: return { }

[Rob Buis]

Created attachment 378942 [details]
Patch

[Rob Buis]

Comment on attachment 378819 [details]
Patch

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

>> Source/WebCore/platform/network/HTTPParsers.h:148
>>      set.add(string.substring(start, end - start + 1));
> 
> We are creating a string twice, can we optimise this?
> We could try to use a StringView for the isValidHTTPToken case.

Fixed.

>> Source/WebCore/platform/network/HTTPParsers.h:162
>> +                return set;
> 
> Why doing set.clear() and return Set.
> Can we just do: return { }

Right, I was not happy with this before, done.

[youenn fablet]

Comment on attachment 378942 [details]
Patch

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

> Source/WebCore/platform/network/HTTPParsers.h:149
> +    set.add(token);

WTFMove()

[Rob Buis]

Created attachment 378944 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 378944 [details]
Patch

Clearing flags on attachment: 378944

Committed r249946: <https://trac.webkit.org/changeset/249946>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/55430697>

[Russell Epstein]

(In reply to Rob Buis from comment #5)
> Note that the test is all PASSes on Catalina, I assume because of a
> CFNetwork fix/change.

Rebaselined for Catalina and iOS 13 in r249981.

[Rob Buis]

(In reply to Russell Epstein from comment #14)
> (In reply to Rob Buis from comment #5)
> > Note that the test is all PASSes on Catalina, I assume because of a
> > CFNetwork fix/change.
> 
> Rebaselined for Catalina and iOS 13 in r249981.

Thnx! I did not know platform/mac was already meant for Catalina.