1715332 - Opening an about: page from a web content page produces warnings and IPDL errors

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Tom Ritter [:tjr]]

You can't open any about: pages (that I have found) from a web content page except about:blank and about:logo. The latter breaks an assumption and causes an error.

STR:

1. Make a debug build.
2. Go to https://ritter.vg/misc/ff/about.html
3. Open about:logo in a new tab

Warnings:

[Parent 917050, IPDL Background] WARNING: '!ClientIsValidPrincipalInfo(aClientInfo.principalInfo())', file /home/tom/Documents/moz/mozilla-unified-2/dom/clients/manager/ClientManagerParent.cpp:112
IPDL protocol error: Handler returned error code!

###!!! [Parent][DispatchAsyncMessage] Error: PClientManager::Msg_ExpectFutureClientSource Processing error: message was deserialized, but the handler returned false (indicating failure)

[Parent 917050, IPDL Background] WARNING: '!ClientIsValidPrincipalInfo(aClientInfo.principalInfo())', file /home/tom/Documents/moz/mozilla-unified-2/dom/clients/manager/ClientManagerParent.cpp:124
IPDL protocol error: Handler returned error code!

###!!! [Parent][DispatchAsyncMessage] Error: PClientManager::Msg_ForgetFutureClientSource Processing error: message was deserialized, but the handler returned false (indicating failure)

[Parent 917050, Main Thread] WARNING: '!ClientIsValidPrincipalInfo(mClientInfo.PrincipalInfo())', file /home/tom/Documents/moz/mozilla-unified-2/dom/clients/manager/ClientSource.cpp:183

Comment 1

[Mirko Brodesser (:mbrodesser-Igalia)]

To be clear, the about: pages can be opened, it's 'just' that those warnings and errors are emitted. Reproduced on Ubuntu 20.04. I guess this is not a recent regression.

Comment 2

[Mirko Brodesser (:mbrodesser-Igalia)]

My guess above might've been wrong. The first warning was added recently in bug 1584007.

Eden: does the warning require any changes in DOM : Services Workers code? I'm not familiar with this code and don't know whether DOM : Core & HTML is the correct component for this ticket.

Comment 3

[Eden Chuang[:edenchuang]]

It would be a regression caused by bug 1584007. Will fix it.

Comment 4

[Eden Chuang[:edenchuang]]

After running with the newest nightly build, the original warning message disappeared.
However, new warning messages show out as following

[Parent 66015, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x805303F4 (NS_ERROR_DOM_BAD_URI): file /Users/echuang/Firefox/mozilla-central/dom/security/nsContentSecurityManager.cpp:1291
[Parent 66015, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x805303F4 (NS_ERROR_DOM_BAD_URI): file /Users/echuang/Firefox/mozilla-central/dom/security/nsContentSecurityManager.cpp:1148
[Parent 66015, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x805303F4 (NS_ERROR_DOM_BAD_URI): file /Users/echuang/Firefox/mozilla-central/docshell/base/nsDocShell.cpp:10664
[Parent 66015, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x805303F4 (NS_ERROR_DOM_BAD_URI): file /Users/echuang/Firefox/mozilla-central/docshell/base/nsDocShell.cpp:888

And this blocks the page loading.

Changing the bug component back to DOM: Core & HTML.

Comment 5

[Eden Chuang[:edenchuang]]

NS_ERROR_DOM_BAD_URI comes from https://searchfox.org/mozilla-central/rev/2c991232499e826e46f9d976eb653817340ba389/caps/nsScriptSecurityManager.cpp#869-877. It wants to make sure the target URI has no URI_DANGEROUS_TO_LOAD flag.

However, https://searchfox.org/mozilla-central/source/netwerk/protocol/about/nsAboutProtocolHandler.cpp#82-87 is called to get the protocol flags for checking for about::xxx URIs. And URI_DANGEROUS_TO_LOAD flag is set, so the loading is blocked.

Mirko I guess you understand this better than me. Would you like to take a look at it?

Comment 6

[Mirko Brodesser (:mbrodesser-Igalia)]

Eden: just rebased my Gecko tree and the warning is still emitted:

zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz
[Parent 54921, Main Thread] WARNING: '!wc', file /home/mirko/src/firefox/gecko2/dom/base/ThirdPartyUtil.cpp:226
[Parent 54921, Main Thread] WARNING: '!wc', file /home/mirko/src/firefox/gecko2/dom/base/ThirdPartyUtil.cpp:226
[Parent 54921, Main Thread] WARNING: '!wc', file /home/mirko/src/firefox/gecko2/dom/base/ThirdPartyUtil.cpp:226
[Parent 54921, IPDL Background] WARNING: '!ClientIsValidPrincipalInfo(aClientInfo.principalInfo())', file /home/mirko/src/firefox/gecko2/dom/clients/manager/ClientManagerParent.cpp:112
IPDL protocol error: Handler returned error code!

###!!! [Parent][DispatchAsyncMessage] Error: PClientManager::Msg_ExpectFutureClientSource Processing error: message was deserialized, but the handler returned false (indicating failure)

[Parent 54921, IPDL Background] WARNING: '!ClientIsValidPrincipalInfo(aClientInfo.principalInfo())', file /home/mirko/src/firefox/gecko2/dom/clients/manager/ClientManagerParent.cpp:124
IPDL protocol error: Handler returned error code!

###!!! [Parent][DispatchAsyncMessage] Error: PClientManager::Msg_ForgetFutureClientSource Processing error: message was deserialized, but the handler returned false (indicating failure)

However, it's preceded by this warning.

baku: can you please shed some light on this? It seems that warning is related to its following warnings.

Comment 7

[Mirko Brodesser (:mbrodesser-Igalia)]

Eden: since the newly added warning still occurs, you might want to have a look again, too.

Comment 8

[Andrew Sutherland [:asuth] (he/him)]

One of the call-sites Eden links to in comment 4 is nsDocShell::OpenInitializedChannel where it appears that:

• We've added a ClientChannelHelper just above before the check that will return NS_ERROR_DOM_BAD_URI.

This makes sense to explain why ClientChannelHelper could be hearing about the principal which later gets mooted. And it does suggest this needs to be addressed unless we change the order of operations there which it's not clear would make sense in the success case.

But it does seem like the validation logic for the principal is unhappy. We do have some MozURL tests that cover about: scheme origins but maybe I'm missing something. :janv might have some thoughts, cc'ing him.

Comment 9

[Johann Hofmann [:johannh]]

Tim, it doesn't seem urgent, but could you take a look at this warning related to third party checks?

Comment 10

[Frederik Braun [:freddy]]

Different warning, these days?

[Child 114769, Main Thread] WARNING: NS_ENSURE_TRUE(info) failed: file extensions/permissions/PermissionDelegateHandler.cpp:348
[Parent 114514, Main Thread] WARNING: '!ClientIsValidPrincipalInfo(mClientInfo.PrincipalInfo())', file /dom/clients/manager/ClientSource.cpp:183
[Parent 114514, Main Thread] WARNING: 'aOwner->IsDiscarded()', file obj-x86_64-pc-linux-gnu/dist/include/mozilla/dom/SyncedContextInlines.h:94

Comment 11

[Daniel Veditz [:dveditz]]

Everytime we get the !ClientIsValidPrincipalInfo(mClientInfo.PrincipalInfo()) warning we should be shutting down the child process, so how does this even work?

https://searchfox.org/mozilla-central/rev/072f9e6b7f10a00e12d0a02ac713431d0a7ee368/dom/clients/manager/ClientSourceParent.cpp#221-225

  if (NS_WARN_IF(!ClientIsValidPrincipalInfo(mClientInfo.PrincipalInfo()))) {
    mService->ForgetFutureSource(mClientInfo.ToIPC());
    KillInvalidChild();
    return;
  }

The child version is similar, except Shutdown() instead of KillInValidChild(). So it's checking this at one point, and then at a later point it opens the document anyway?

Comment 12

[Eden Chuang[:edenchuang]]

Attachment: Bug 1715332 - correct ClientValidation for moz-safe-about scheme. r=#dom-worker-reviewers

Comment 13

[Eden Chuang[:edenchuang]]

(In reply to Daniel Veditz [:dveditz] from comment #11)

Everytime we get the !ClientIsValidPrincipalInfo(mClientInfo.PrincipalInfo()) warning we should be shutting down the child process, so how does this even work?

https://searchfox.org/mozilla-central/rev/072f9e6b7f10a00e12d0a02ac713431d0a7ee368/dom/clients/manager/ClientSourceParent.cpp#221-225

  if (NS_WARN_IF(!ClientIsValidPrincipalInfo(mClientInfo.PrincipalInfo()))) {
    mService->ForgetFutureSource(mClientInfo.ToIPC());
    KillInvalidChild();
    return;
  }

The child version is similar, except Shutdown() instead of KillInValidChild(). So it's checking this at one point, and then at a later point it opens the document anyway?

Currently, the warning message is from https://searchfox.org/mozilla-central/rev/980b50947e2a855c92f2df74209dadad3ee4d119/dom/clients/manager/ClientSource.cpp#183

However, the ClientSource::Shotdown() would not close the content process. It just closes the corresponding Client related things.
That is the reason why we still see the document is loaded.

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:jesup, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/01fa9222bb65
correct ClientValidation for moz-safe-about scheme. r=dom-worker-reviewers,asuth

Comment 16

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/01fa9222bb65