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.
Created attachment 378808 [details] Patch
Created attachment 378813 [details] Patch
Created attachment 378814 [details] Patch
Created attachment 378819 [details] Patch
Note that the test is all PASSes on Catalina, I assume because of a CFNetwork fix/change.
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 { }
Created attachment 378942 [details] Patch
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.
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()
Created attachment 378944 [details] Patch
Comment on attachment 378944 [details] Patch Clearing flags on attachment: 378944 Committed r249946: <https://trac.webkit.org/changeset/249946>
All reviewed patches have been landed. Closing bug.
<rdar://problem/55430697>
(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.
(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.