Bug 19242 - Data URLs should set an Access-Control-Origin of "null"
Summary: Data URLs should set an Access-Control-Origin of "null"
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: XML (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-05-25 00:35 PDT by Adam Barth
Modified: 2008-06-12 03:59 PDT (History)
3 users (show)

See Also:


Attachments
untested patch (3.96 KB, patch)
2008-05-25 00:37 PDT, Adam Barth
sam: review-
Details | Formatted Diff | Diff
patch with test (10.16 KB, patch)
2008-06-07 01:39 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
patch (10.16 KB, patch)
2008-06-11 01:37 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
patch (12.24 KB, patch)
2008-06-11 01:54 PDT, Adam Barth
sam: review-
Details | Formatted Diff | Diff
updated and tweaked (7.81 KB, patch)
2008-06-11 14:11 PDT, Sam Weinig
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2008-05-25 00:35:37 PDT
Data URLs should set an Access-Control-Origin of "null".  Currently, we're setting the header to the empty string.
Comment 1 Adam Barth 2008-05-25 00:37:06 PDT
Created attachment 21332 [details]
untested patch

Here's a patch.  It's untested at the moment.  I really wish I had a Mac...  :)
Comment 2 Sam Weinig 2008-05-26 14:33:38 PDT
Comment on attachment 21332 [details]
untested patch

I think the logic for:
     String accessControlOrigin = m_doc->securityOrigin()->toString();
+    if (accessControlOrigin.isEmpty())
+        accessControlOrigin = "null";
should be lifted out into a separate function.  There is one other places that put the access-control-origin into the request that you have missed, in handleAsynchronousMethodCheckResult where the helper should be used.
Comment 3 Adam Barth 2008-06-07 01:39:51 PDT
Created attachment 21554 [details]
patch with test

This turned out to be a bit more involved than I expected, but here's an improved patch.
Comment 4 Adam Barth 2008-06-07 01:56:47 PDT
Comment on attachment 21554 [details]
patch with test

Actually, I'm not sure this patch is right w.r.t. document that have set their document.domain property.  Let me test how this works in other browsers.
Comment 5 Adam Barth 2008-06-11 01:37:09 PDT
Created attachment 21619 [details]
patch

This patch handles document.domain properly (and adds a test for this behavior).  As a side effect, this should also fix Bug 15100.
Comment 6 Adam Barth 2008-06-11 01:54:51 PDT
Created attachment 21620 [details]
patch

Oops.  Attached the old version of the patch before.
Comment 7 Sam Weinig 2008-06-11 14:08:19 PDT
Comment on attachment 21620 [details]
patch

This includes some document.domain changes that I think should be landed separately.
Comment 8 Sam Weinig 2008-06-11 14:11:44 PDT
Created attachment 21644 [details]
updated and tweaked

Adam, I took the chunk of your patch that relates to this specific bug and tweaked it a bit.  If something looks amiss, please let me know.
Comment 9 Adam Barth 2008-06-11 14:56:07 PDT
> Adam, I took the chunk of your patch that relates to this specific bug and
> tweaked it a bit.  If something looks amiss, please let me know.

Yeah that looks right.  Thanks.
Comment 10 Adam Barth 2008-06-12 03:59:37 PDT
Fixed in r34504.