[Webkit-unassigned] [Bug 26196] Fix the problem that worker's importScripts fails if the script URL is redirected from different origin.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26196


levin at chromium.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #30957|review?                     |review+
               Flag|                            |




------- Comment #2 from levin at chromium.org  2009-06-08 18:03 PDT -------
(From update of attachment 30957)
Just a few things to fix up on landing.

> diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
> index 40d853a..6b451d0 100644
> --- a/LayoutTests/ChangeLog
> +++ b/LayoutTests/ChangeLog
> @@ -1,3 +1,13 @@
> +2009-06-04  Jian Li  <jianli at chromium.org>
> +
> +        Reviewed by NOBODY (OOPS!).
> +

I think the current style is to add the bug title here.
> +        https://bugs.webkit.org/show_bug.cgi?id=26196
> +        Add a test case in the importScripts layout test to cover the scenario that the import script URL can come from different redirect origin.

Interestingly, the changelog text is typically wrapped.

Perhaps 
  "can come from different origin through a redirect."
would be slightly clearer.

> +
> +        * http/tests/workers/resources/worker-importScripts.js:
> +        * http/tests/workers/worker-importScripts-expected.txt:
> +
>  2009-06-02  Alexey Proskuryakov  <ap at webkit.org>
>  
>          Land correct results for a test I just added (forgot to regenerate them after adding new
> diff --git a/LayoutTests/http/tests/workers/resources/worker-importScripts.js b/LayoutTests/http/tests/workers/resources/worker-importScripts.js
> index ddf479c..364c502 100644
> --- a/LayoutTests/http/tests/workers/resources/worker-importScripts.js
> +++ b/LayoutTests/http/tests/workers/resources/worker-importScripts.js
> @@ -7,6 +7,7 @@ postMessage("PASS: importScripts(), exists, is a function, and doesn't throw whe
>  var source1 = "worker-importScripts-source1.js";
>  var source2 = "worker-importScripts-source2.js";
>  var differentOrigin = "http://localhost:8000/workers/resources/worker-importScripts-differentOrigin.js";
> +var differentRedirectOrigin = "/resources/redirect.php?url=http://localhost:8000/workers/resources/worker-importScripts-differentOrigin.js";
>  var syntaxErrorSource = "worker-importScripts-syntaxError.js";
>  var fakeSource = "nonexistant";
>  var loadedSource1 = false;
> @@ -29,6 +30,16 @@ if (differentOriginLoaded)
>  
>  resetLoadFlags();
>  
> +try {
> +    importScripts(differentRedirectOrigin)
> +} catch(e) {
> +    postMessage("FAIL: Threw " + e + " when attempting cross origin load on redirection");
Add "."

> +}
> +if (differentOriginLoaded)
> +    postMessage("PASS: executed script from different redirect origin");
I think this could be re-worded slight (similar to the ChangeLog and add a
".").

Add an else to print a FAIL message so that one can tell that it failed just by
running it in a browser.


> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index b119fa6..bf8ebc3 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,34 @@

Same fix up in this changelog as suggested above.

>  
> -DocumentThreadableLoader::DocumentThreadableLoader(Document* document, ThreadableLoaderClient* client, const ResourceRequest& request, LoadCallbacks callbacksSetting, ContentSniff contentSniff, StoredCredentials storedCredentials)
> +DocumentThreadableLoader::DocumentThreadableLoader(Document* document, ThreadableLoaderClient* client, const ResourceRequest& request, LoadCallbacks callbacksSetting, ContentSniff contentSniff, StoredCredentials storedCredentials, RedirectOriginCheck redirectOriginCheck)
>      : m_client(client)
>      , m_document(document)
>      , m_allowStoredCredentials(storedCredentials == AllowStoredCredentials)
>      , m_sameOriginRequest(document->securityOrigin()->canRequest(request.url()))
> +    , m_checkRedirectOrigin(redirectOriginCheck == RequireSameRedirectOrigin)
>  {

It would be nice to assert that redirectOriginCheck has one of the two allowed
values (and feel free to add it for storedCredentials which I should have
done).


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.