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)

Product:
Core
Component:
DOM: Core & HTML
Version:
8 Branch
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED DUPLICATE of bug 612921

People

(Reporter: mgoodwin, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: perf)

Attachments

(1 file)

example content policy to show this is not a CSP specific issue
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
Blocks: CSP
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.
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
(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
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.
Version: 12 Branch → 8 Branch
Summary: CSP creates duplicate errors for violations → ContentPolicy shouldLoad is called multiple times for some resources
No longer blocks: CSP
I've narrowed this down to a change between FX7 and FX8 releases. I'll start checking nightlies to pinpoint the change.
Blocks: CSP
No longer blocks: 712859
(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.
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
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.
@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.
> 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
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.

Attachment

General

Creator:
Created:
Updated:
Size: