1838686 - Crash in [@ CheckAllowLoadByTriggeringRemoteType]

Status: RESOLVED INCOMPLETE

(Core :: DOM: Navigation, defect)

Description

[Gabriele Svelto [:gsvelto]]

Crash report: https://crash-stats.mozilla.org/report/index/b8781b51-ccb2-4142-a43b-e004d0230615

MOZ_CRASH Reason: Blocking document load of file URI by content process which may have otherwise succeeded [isSystemPrincipal=1]

Top 10 frames of crashing thread:

0  xul.dll  MOZ_Crash  mfbt/Assertions.h:264
0  xul.dll  CheckAllowLoadByTriggeringRemoteType  dom/security/nsContentSecurityManager.cpp:1459
0  xul.dll  nsContentSecurityManager::doContentSecurityCheck  dom/security/nsContentSecurityManager.cpp:1501
1  xul.dll  nsBaseChannel::AsyncOpen  netwerk/base/nsBaseChannel.cpp:636
2  xul.dll  mozilla::net::DocumentLoadListener::Open  netwerk/ipc/DocumentLoadListener.cpp:848
3  xul.dll  mozilla::net::DocumentLoadListener::OpenDocument  netwerk/ipc/DocumentLoadListener.cpp:933
4  xul.dll  mozilla::net::DocumentChannelParent::Init  netwerk/ipc/DocumentChannelParent.cpp:69
4  xul.dll  mozilla::net::NeckoParent::RecvPDocumentChannelConstructor  netwerk/ipc/NeckoParent.cpp:286
5  xul.dll  mozilla::net::PNeckoParent::OnMessageReceived  ipc/ipdl/PNeckoParent.cpp:1962
6  xul.dll  mozilla::dom::PContentParent::OnMessageReceived  ipc/ipdl/PContentParent.cpp:6773

This assertion seems to have been added in bug 1538028, Nika could you have a look?

Comment 2

[Nika Layzell [:nika] (ni? for response)]

Unfortunately it's very difficult to figure out what would be causing us to start a load for a file URI from a content process. Fortunately outside of MOZ_DIAGNOSTIC_ASSERT_ENABLED builds this would just be a network error rather than a crash, so this shouldn't be a severe issue outside of nightly/early beta.

I'll post a patch which adds some extra assertions to the content process side so perhaps we can get more information about what the actual cause of the load is. Marking this as S3 for now though because the crash shouldn't impact release.

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1838686 - Check for content-initiated file URI loads in content, r=smaug!

The goal of this change is to detect content-initiated file URI loads
earlier such that crash stacks give us a better idea of what is
responsible for starting these loads.

These assertions explicitly only fire for system-prinicipal triggered
loads of file URIs, as those should never be happening from within a web
content process.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8082fdab7a06
Check for content-initiated file URI loads in content, r=smaug,necko-reviewers,jesup

Comment 5

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/8082fdab7a06

Comment 7

[Dianna Smith [:diannaS]]

:nika since i don't see any crashes in nightly did you want to uplift this to 116 for better visibility?

Comment 8

[Nika Layzell [:nika] (ni? for response)]

These are diagnostic asserts, so they'll only be visible in nightly & early beta. I worry a bit that an uplift at this point in the beta cycle wouldn't win us much as there's not much early beta left iirc (I could be misremembering though).

The signature of the new crash would be different now, as we should be crashing in https://searchfox.org/mozilla-central/rev/7a4c08f2c3a895c9dc064734ada320f920250c1f/docshell/base/nsDocShellLoadState.cpp#910 instead of in the parent process, though I haven't found any crashes with that signature on crash-stats in a quick search. A sloppy query of crash pings unfortunately didn't turn up anything either. We might need to wait for the next early beta cycle.

Comment 9

[Donal Meehan [:dmeehan]]

:nika any updates now that 117 has been in early beta, not seeing any crashes there
This bug has a leave-open status, wondering if there is any follow-up needed/expected?

Comment 10

[Nika Layzell [:nika] (ni? for response)]

(In reply to Donal Meehan [:dmeehan] from comment #9)

:nika any updates now that 117 has been in early beta, not seeing any crashes there
This bug has a leave-open status, wondering if there is any follow-up needed/expected?

It's interesting that there's no crashes with the existing signature and also that I can't find any crashes with the new moz crash reason. Perhaps whatever was causing the failure has been changed, such that it's not happening anymore. I think we can probably close this for now until crashes appear again.

Comment 11

[Daniel Holbert [:dholbert]]

I can reproduce this crash by using DevTools (on desktop) to try to navigate a tab (on Android).

I'll file a new bug on that, instead of reopening, since this bug had a patch land and has been closed for a few years (which would make reopening/tracking a bit clumsier).

Comment 12

[Daniel Holbert [:dholbert]]

(In reply to Nika Layzell [:nika] (ni? for response) from comment #10)

It's interesting that there's no crashes with the existing signature and also that I can't find any crashes with the new moz crash reason.

Following up on this: Bug 1963915 has STR for triggering both versions of the crash signature, FWIW.

tl;dr:

• use the about:debugging devtools URL-bar to navigate from example.org to about:config to trigger the original signature [@ CheckAllowLoadByTriggeringRemoteType ]
• ...or navigate to file:/// to trigger the new signature [@ nsDocShellLoadState::AssertProcessCouldTriggerLoadIfSystem ] (with MOZ_CRASH(Unexpected system load of file URI from web content process)), the intentional crash added in the patch that we landed here (comment 4).

Comment 13

[:Gijs (he/him)]

There were a bunch of crashes early in may and one a few days ago. The ones early in may were for file uris without system principal, the more recent one is:

Blocking load of about uri by content process which may have otherwise succeeded [aboutModule=config, isSystemPrincipal=1]

Is there anything more to do here or are we confident these are all builds being debugged by outdated devtools (that don't have changes from bug 1963915) ?

Comment 14

[Daniel Holbert [:dholbert]]

(In reply to :Gijs (he/him) from comment #13)

the more recent one is:

Blocking load of about uri by content process which may have otherwise succeeded [aboutModule=config, isSystemPrincipal=1]

I think that's bug 1963915 (and so are the Android ones from early May).

I don't know about the windows ones from early May, though.

Comment 15

[Nika Layzell [:nika] (ni? for response)]

I don't feel confident necessarily, but I don't think there's any extra information in the crash reports which could help us distinguish the case of this being an old devtools debugger vs. something else, so I think we have to assume it's that case unless we see a new spike.