Closed
Bug 766536
Opened 12 years ago
Closed 12 years ago
ContentPolicy shouldLoad is called multiple times for some resources
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 612921
People
(Reporter: mgoodwin, Unassigned)
References
(Blocks 1 open bug)
Details
(Keywords: perf)
Attachments
(1 file)
|
1.46 KB,
application/x-javascript
|
Details |
Issue:
Multiple error console items appear for each individual img-src (and possibly other directives) violation.
STR:
1) Open the error console
2) Clear the error console
3) Navigate to a page with a CSP violation: e.g. https://csptest.computerist.org/
4) Observe multiple console entries for a single violation
|
Reporter | |
Comment 1•12 years ago
|
||
This doesn't look like it's a CSP problem; multiple calls are being made to shouldLoad on nsIContentPolicy.
This impacts on CSP in a couple of ways:
1) Duplicate errors / warnings are presented via the error console in the event of policy violations
2) If report-uri is being used, multiple reports are being sent per violation.
It's possible this is causing other issues elsewhere.
|
|
Reporter | |
Comment 2•12 years ago
|
||
using this:
1) create a simple document, e.g. <html><body><img src="http://example.com/test.jpg"></body></html>
2) Run the attached code in a chrome scratchpad
3) load the sample document
4) observe duplicate console messages for SHOULD LOAD for the image resource
|
||
Comment 3•12 years ago
|
||
(In reply to Mark Goodwin [:mgoodwin] from comment #1)
> It's possible this is causing other issues elsewhere.
If nothing else this is going to be a perf hit, a little with our own internal content policies (of which we've got 4 or 5) and bigger with things checking against huge lists likd AdBlock Plus and Ghostery.
Keywords: perf
|
|
Reporter | |
Comment 4•12 years ago
|
||
Observe the difference between documents such as:
<html><script src="http://example.com/test.js"></html>
and
<html><script src="http://example.com/test.js"></script></html>
the latter results in 2 invocations of shouldLoad, the former, just one. Image loads always result in 2.
|
|
Reporter | |
Updated•12 years ago
|
Version: 12 Branch → 8 Branch
|
|
Reporter | |
Updated•12 years ago
|
Summary: CSP creates duplicate errors for violations → ContentPolicy shouldLoad is called multiple times for some resources
|
|
Reporter | |
Comment 5•12 years ago
|
||
I've narrowed this down to a change between FX7 and FX8 releases. I'll start checking nightlies to pinpoint the change.
|
||
Comment 6•12 years ago
|
||
(I think) This is caused by the preloader: it tries to preload images, and that is blocked by the content policy. Then the contentsink tries to load the image again, sees that it is not in cache, and checks with contentpolicy again.
For example, if we do "document.write('<img src=uri-that-violates-csp>');" then we only get to see 1 warning in the console. It is only for resources that the HTML parser tries to preload that we get this warning.
Not sure if we want to fix this. Double warnings do suck, but this seems to be intended behavior. I am keeping this open for bz (who wrote the preloader) to comment, but I suspect we want to resolve wontfix.
|
|
||
Comment 7•12 years ago
|
||
A note about multiple warnings: since some addons calling contentpolicy directly, I think they might also be generating repeated warnings. So this isn't a fix that can be "just fix the preloader." So the only fix I can think of is for the CSP code to cache reports already sent. Thus, this bug should go back to being a CSP bug, and possibly blocking 493857
|
||
Comment 8•12 years ago
|
||
Related to this: bug 533247. CSP is one thing but other content policy implementations actually rely heavily on this behavior. Maybe CSP could detect duplicate reports and suppress them.
This is probably due to prefetching during parsing. I.e. we first hit the network while the parser is blocked to prefetch the resource. Once the parser is unblocked we create the content node which normally would just use the prefetched resource, but since that was blocked we try to hit the network.
There's a similar bug filed on not doing this second request if the prefetched failed for other reasons, though I believe that bug is specific for images. I can't find that bug off the top of my head though.
Also note that the reason that
<html><script src="http://example.com/test.js"></html>
only hits the network once is that there is no ending </script> tag, so when the content node is created we intentionally tell it to not execute (and thus not attempt to load) any scripts. So basically that is just a red herring here.
|
|
||
Comment 10•12 years ago
|
||
@Wladimir: that's probably the correct solution. Maybe the CSP reportviolation code (which is already async) can setTimeout before actually reporting, and dedup all violations in that time period.
|
||
Comment 11•12 years ago
|
||
> This is probably due to prefetching during parsing.
Yes.
> though I believe that bug is specific for images.
There's bug 612921 about scripts, which this one seems like an exact duplicate of.
CSP should perhaps not report anything about blocked script loads where the load context is not an element....
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
|
|
||
Comment 12•12 years ago
|
||
And again, we could also change the API to indicate when the conpol check is for a speculative preload.
You need to log in
before you can comment on or make changes to this bug.
Description
•