Closed Bug 1627971 Opened 4 years ago Closed 4 years ago

Investigate why dom/security/test/csp/test_frame_src.html is not working if we do not enforce contentSecurity Checks in the DocumentChannelChild

Categories

(Core :: DOM: Security, task, P2)

task

Tracking

()

RESOLVED FIXED
mozilla77
Tracking Status
firefox77 --- fixed

People

(Reporter: ckerschb, Assigned: mattwoodrow)

References

Details

(Whiteboard: [domsecurity-active])

Attachments

(3 files)

Priority: P3 → P2

Hey Matt, I removed the nsContentSecurityManager::doContentSecurityCheck within DocumentChannelChild::AsyncOpen and tried to narrow down the problem why test_frame_src.html is failing. So it seems that CSP handling itself in the parent is working correctly and the load gets blocked within nsCSPContext::ShouldLoad which propagates the error back to nsContentSecurityManager.

The problem is that the onerror handler for the testframe does not fire anymore. I don't know why that is the case. I assume that problem causes more csp tests to fail in your TRY run.

If you have any suggestions or ideas where to start debugging and why onerror events on iframes don't fire anymore, please let me know.

Flags: needinfo?(matt.woodrow)

So it appears nsFrameLoader::FireErrorEvent is responsible for firing the onerror event to iframe element, but it's only called if the frame loader has an in-process docshell, and the error is returned synchronously from nsDocShell::LoadURI.

Previously, this test was catching the error during AsyncOpen in the content process, so we successfully fired the error event.

Now that the error is detected asynchronously (in the parent), nothing lets the frame loader know :(

I'll look into this a bit, since it seems like it's already pretty broken with OOP iframes.

Flags: needinfo?(matt.woodrow)

WIP patches for fixing this: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8f24bea1cdbca48dfa3ea2df2cce27ad48570b61

With those patches, and the content process security checks removed, this test passed. I haven't checked any others.

Try run doesn't have the security checks moved, just to see what potential fallout there is as a baseline.

See Also: → 1629201

Trying to reason about which failures need an error event is super hard with the current code, so I've filed bug 1629201 for trying to refactor the code into a state where that's possible.

That's fairly involved though, and this bug is blocking fission, so I'll upload my patches for making this specific case work in the mean time.

Assignee: ckerschb → matt.woodrow
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/98c748c67725
Move docshell embedder unblock load to a helper. r=nika
https://hg.mozilla.org/integration/autoland/rev/732e95f852ef
Add option to MaybeFireEmbedderLoadEvents to also fire the error event to the embedder element. r=nika
https://hg.mozilla.org/integration/autoland/rev/c885d9bdcc53
Fire onerror event for CONTENT_BLOCKED in EndPageLoad. r=nika,ckerschb
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: