Closed
Bug 1161288
Opened 10 years ago
Closed 10 years ago
Support app:// origins on Fetch API
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: ferjm, Assigned: ferjm)
References
Details
(Whiteboard: [s1])
Attachments
(1 file, 3 obsolete files)
I get "TypeError: NetworkError when attempting to fetch resource" when trying to cache a resource via cache.addAll() within the install event handler. The same code works if I install the sw from an https:// origin.
Assignee | ||
Updated•10 years ago
|
Blocks: nga-toolkit-service-workers
Assignee | ||
Updated•10 years ago
|
Summary: Trying to cache a resource from a service worker installed from an app:// origin fails → Support app:// origins on Fetch API
Assignee | ||
Updated•10 years ago
|
Component: DOM: Workers → DOM
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → ferjmoreno
Assignee | ||
Updated•10 years ago
|
Whiteboard: [s1]
Assignee | ||
Comment 1•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•10 years ago
|
||
Attachment #8601456 -
Attachment is obsolete: true
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8602112 -
Attachment is obsolete: true
Attachment #8602185 -
Flags: review?(nsm.nikhil)
Attachment #8602185 -
Flags: review?(amarchesini)
Assignee | ||
Comment 4•10 years ago
|
||
Sorry for the spam. I forgot to add one file.
Attachment #8602185 -
Attachment is obsolete: true
Attachment #8602185 -
Flags: review?(nsm.nikhil)
Attachment #8602185 -
Flags: review?(amarchesini)
Attachment #8602188 -
Flags: review?(nsm.nikhil)
Attachment #8602188 -
Flags: review?(amarchesini)
Assignee | ||
Comment 5•10 years ago
|
||
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8602188 [details] [diff] [review]
v1
Review of attachment 8602188 [details] [diff] [review]:
-----------------------------------------------------------------
I didn't review the script security bit. Everything else looks good!
::: dom/fetch/FetchDriver.cpp
@@ +686,5 @@
>
> + // We simulate the http protocol for jar/app requests
> + uint32_t responseStatus = 200;
> + nsAutoCString statusText;
> + statusText.AssignLiteral("OK");
Replace this...
@@ +691,2 @@
>
> + response = new InternalResponse(responseStatus, statusText);
... with NS_LITERAL_CSTRING("OK") here.
Attachment #8602188 -
Flags: review?(nsm.nikhil) → review+
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8602188 [details] [diff] [review]
v1
Thank you Nikhil.
Bobby, could you take a look at the nsScriptSecurityManager change, please? Thanks!
Attachment #8602188 -
Flags: review?(bobbyholley)
Comment 9•10 years ago
|
||
Comment on attachment 8602188 [details] [diff] [review]
v1
Review of attachment 8602188 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/fetch/FetchDriver.cpp
@@ +14,5 @@
> #include "nsIHttpHeaderVisitor.h"
> #include "nsIScriptSecurityManager.h"
> #include "nsIThreadRetargetableRequest.h"
> #include "nsIUploadChannel2.h"
> +#include "nsIJARChannel.h"
alphabetic order.
@@ +294,5 @@
> return FailWithNetworkError();
> }
>
> if (scheme.LowerCaseEqualsLiteral("file")) {
> } else if (scheme.LowerCaseEqualsLiteral("http") ||
can you remove this empty if(scheme.LowerCaseEqualsLiteral("file")) ?
Attachment #8602188 -
Flags: review?(amarchesini) → review+
Comment 10•10 years ago
|
||
Comment on attachment 8602188 [details] [diff] [review]
v1
Review of attachment 8602188 [details] [diff] [review]:
-----------------------------------------------------------------
::: caps/nsScriptSecurityManager.cpp
@@ +756,5 @@
> nsIProtocolHandler::URI_DANGEROUS_TO_LOAD);
> +
> + // In order to be able to test packaged apps (app:// scheme) from
> + // mochitests (http:// scheme) we need to ignore the scheme mismatch.
> + if (!Preferences::GetBool("security.uri.allow_scheme_mismatch", false) &&
I'm not wild about this - this seems like an overly-broad hammer to apply here, and we're trying to make the URI loading permissions simpler rather than adding more special cases. Can you describe the use-case you're trying to solve in more detail?
Attachment #8602188 -
Flags: review?(bobbyholley) → review-
Comment 11•10 years ago
|
||
Assignee | ||
Comment 12•10 years ago
|
||
Talked to Bobby on IRC and he suggested a change in the test that solved the issue and avoids making the change in the ScriptSecurityManager.
Assignee | ||
Updated•10 years ago
|
Blocks: ServiceWorkers-B2G
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•