Make sure a preflight fails if response headers are invalid
Created attachment 393260 [details] Patch
Created attachment 393351 [details] Patch
Created attachment 393362 [details] Patch
Ping review
Comment on attachment 393362 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393362&action=review > Source/WebCore/ChangeLog:11 > + Minor refactoring to return Expected/Optional for erro rhandlng instead of passing an out parameter. error > Source/WebCore/loader/CrossOriginAccessControl.cpp:273 > + CrossOriginPreflightResultCache::singleton().appendEntry(securityOrigin.toString(), request.url(), entry.moveToUniquePtr()); Do we want to do this if there was an error? We didn't before. > Source/WebCore/loader/CrossOriginPreflightResultCache.h:54 > + bool allowsCrossOriginMethod(const String&, StoredCredentialsPolicy, String& errorDescription) const; > + bool allowsCrossOriginHeaders(const HTTPHeaderMap&, StoredCredentialsPolicy, String& errorDescription) const; These should return Expected<void, String> instead of having an out parameter.
CrossOriginPreflightResultCache::singleton().appendEntry(securityOrigin.toString(), request.url(), entry.moveToUniquePtr()); > > Do we want to do this if there was an error? We didn't before. Yes, I think so, this is explained in the change log > > Source/WebCore/loader/CrossOriginPreflightResultCache.h:54 > > + bool allowsCrossOriginMethod(const String&, StoredCredentialsPolicy, String& errorDescription) const; > > + bool allowsCrossOriginHeaders(const HTTPHeaderMap&, StoredCredentialsPolicy, String& errorDescription) const; > > These should return Expected<void, String> instead of having an out > parameter. Sure, this is preexisting and could be dealt with as a follow-up. This patch moves these method from public to private so that at least we do not make this pattern more widespread.
Created attachment 393838 [details] Patch for landing
> Sure, this is preexisting and could be dealt with as a follow-up. > This patch moves these method from public to private so that at least we do > not make this pattern more widespread. https://bugs.webkit.org/show_bug.cgi?id=209224
Comment on attachment 393838 [details] Patch for landing Clearing flags on attachment: 393838 Committed r258631: <https://trac.webkit.org/changeset/258631>
All reviewed patches have been landed. Closing bug.
<rdar://problem/60584210>