Summary: | Cross origin Beacon requests with a ArrayBuffer / ArrayBufferView payload should not do a CORS preflight | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||
Component: | WebCore Misc. | Assignee: | Chris Dumez <cdumez> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | buildbot, commit-queue, dbates, ggaren, japhet, webkit-bug-importer, youennf | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=175635 | ||||||
Bug Depends on: | 175679 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
Chris Dumez
2017-08-16 08:58:51 PDT
Created attachment 318290 [details]
Patch
Comment on attachment 318290 [details]
Patch
r=me
Comment on attachment 318290 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=318290&action=review > Source/WebCore/loader/cache/CachedResourceRequest.cpp:44 > + , m_originalRequestHeaders(m_resourceRequest.httpHeaderFields()) This field seems unnecessary in CachedResourceRequest, as well as the header map copy for all requests. Why not using an optional<header-map> in CachedResource and do the copy only for keep alive and/or beacon requests at CachedResourceLoader level? Comment on attachment 318290 [details] Patch Clearing flags on attachment: 318290 Committed r220817: <http://trac.webkit.org/changeset/220817> All reviewed patches have been landed. Closing bug. (In reply to youenn fablet from comment #4) > Comment on attachment 318290 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=318290&action=review > > > Source/WebCore/loader/cache/CachedResourceRequest.cpp:44 > > + , m_originalRequestHeaders(m_resourceRequest.httpHeaderFields()) > > This field seems unnecessary in CachedResourceRequest, as well as the header > map copy for all requests. > Why not using an optional<header-map> in CachedResource and do the copy only > for keep alive and/or beacon requests at CachedResourceLoader level? I hesitated to do this, I was trying to avoid special casing keepalive/beacon too much. I can follow-up to see how it looks. We should try to remove the header map copy. DTL is already doing this copy and other resources will not do preflights. Ideally, this copy should be only visible/manipulated by CachedResourceLoader/CachedResource. It might be easier to have CachedResourceRequest convey this information between the two but other users of CachedResourceRequest should not be tempted to start using it. (In reply to Chris Dumez from comment #7) > (In reply to youenn fablet from comment #4) > > Comment on attachment 318290 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=318290&action=review > > > > > Source/WebCore/loader/cache/CachedResourceRequest.cpp:44 > > > + , m_originalRequestHeaders(m_resourceRequest.httpHeaderFields()) > > > > This field seems unnecessary in CachedResourceRequest, as well as the header > > map copy for all requests. > > Why not using an optional<header-map> in CachedResource and do the copy only > > for keep alive and/or beacon requests at CachedResourceLoader level? > > I hesitated to do this, I was trying to avoid special casing > keepalive/beacon too much. I can follow-up to see how it looks. https://bugs.webkit.org/show_bug.cgi?id=175679 |