Closed Bug 1663238 Opened 4 years ago Closed 4 years ago

Assertion failure: originalPrincipalToInherit->Equals(flattenedPrincipalToInherit), at src/ipc/glue/BackgroundUtils.cpp:629

Categories

(Core :: DOM: Navigation, defect)

defect

Tracking

()

RESOLVED FIXED
82 Branch
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox80 --- unaffected
firefox81 --- unaffected
firefox82 --- fixed

People

(Reporter: tsmith, Assigned: annyG)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: assertion, regression, testcase)

Attachments

(3 files)

Assertion failure: originalPrincipalToInherit->Equals(flattenedPrincipalToInherit), at src/ipc/glue/BackgroundUtils.cpp:629

#0 0x7f208e4d121e in mozilla::ipc::LoadInfoArgsToLoadInfo(mozilla::Maybe<mozilla::net::LoadInfoArgs> const&, nsINode*, mozilla::net::LoadInfo**) src/ipc/glue/BackgroundUtils.cpp:628:9
#1 0x7f208e4ce773 in mozilla::ipc::LoadInfoArgsToLoadInfo(mozilla::Maybe<mozilla::net::LoadInfoArgs> const&, nsINode*, nsILoadInfo**) src/ipc/glue/BackgroundUtils.cpp:559:7
#2 0x7f208e3a8011 in mozilla::net::DocumentChannelChild::RecvRedirectToRealChannel(mozilla::net::RedirectToRealChannelArgs&&, nsTArray<mozilla::ipc::Endpoint<mozilla::extensions::PStreamFilterParent> >&&, std::function<void (nsresult const&)>&&) src/netwerk/ipc/DocumentChannelChild.cpp:192:3
#3 0x7f208e792795 in mozilla::net::PDocumentChannelChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PDocumentChannelChild.cpp:281:64
#4 0x7f208e69bdcd in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PContentChild.cpp:8797:32
#5 0x7f208e52363e in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2150:25
#6 0x7f208e51fdff in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2074:9
#7 0x7f208e521206 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1922:3
#8 0x7f208e521e2b in mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1953:13
#9 0x7f208dc1341f in mozilla::RunnableTask::Run() src/xpcom/threads/TaskController.cpp:242:16
#10 0x7f208dc1149a in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:512:26
#11 0x7f208dc105f4 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:371:15
#12 0x7f208dc107a7 in mozilla::TaskController::ProcessPendingMTTask(bool) src/xpcom/threads/TaskController.cpp:168:36
#13 0x7f208dc18156 in operator() src/xpcom/threads/TaskController.cpp:83:37
#14 0x7f208dc18156 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_4>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:577:5
#15 0x7f208dc2b55f in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1234:14
#16 0x7f208dc30f0a in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:513:10
#17 0x7f208e528f36 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:87:21
#18 0x7f208e49bb53 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:334:10
#19 0x7f208e49ba6d in RunHandler src/ipc/chromium/src/base/message_loop.cc:327:3
#20 0x7f208e49ba6d in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:309:3
#21 0x7f209212d278 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
#22 0x7f2093903b43 in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:913:20
#23 0x7f208e529cf9 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:237:9
#24 0x7f208e49bb53 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:334:10
#25 0x7f208e49ba6d in RunHandler src/ipc/chromium/src/base/message_loop.cc:327:3
#26 0x7f208e49ba6d in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:309:3
#27 0x7f2093903728 in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:744:34
#28 0x55a613516957 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
#29 0x55a613516957 in main src/browser/app/nsBrowserApp.cpp:303:18

I can't seem to get a test case to reduce easily. Hopefully a Pernosco session is all that is needed.

A Pernosco session is available here: https://pernos.co/debug/QmRzPJFpHeNGG3f0JJwPcw/index.html

I don't seem to have permissions to access that. Emilio, would you be interested in taking a look before this expires?

Flags: needinfo?(emilio)

This doesn't seem print-related. This assert comes from bug 1589102 which landed recently.

Anny, can you take a look?

Component: Print Preview → DOM: Navigation
Flags: needinfo?(emilio) → needinfo?(agakhokidze)
Regressed by: 1589102
Has Regression Range: --- → yes

Set release status flags based on info from the regressing bug 1589102

Looking into this!

Assignee: nobody → agakhokidze
Flags: needinfo?(agakhokidze)

Ok I added some entries to the notebook section. Looks like at first we are loading "http://127.0.0.1:39181/testcase.html" in one browsing context, and we save the principal to inherit for this load, and then later, when we are trying to load a data: URI in the same browsing context we end up using "principal to inherit" from the first load, because we haven't updated it to be anything else.

There are two ways of fixing this:

  1. Reset saved principals each time we deserialize a LoadInfoArgs in the content parent and try to retrieve the saved principals. This should be a point when we are done with the load and don't need the saved principals anymore.
  2. Remove the assertion and only use the saved "principal to inherit" if it's equal to the flattened one. The only problem with this is the browsing context will still carry around old principals from previous loads, unless replaced with new ones, and it might lead to bugs if someone tries to use them.

:tsmith is the test case simple enough for me to run it locally? I want to make sure my solution does in fact fix this crash. Alternatively, I can upload a patch here and you could apply it locally and see if the test still crashes.

Flags: needinfo?(twsmith)

(In reply to Anny Gakhokidze [:annyG] from comment #6)

:tsmith is the test case simple enough for me to run it locally? I want to make sure my solution does in fact fix this crash. Alternatively, I can upload a patch here and you could apply it locally and see if the test still crashes.

I'd be happy to verify the fix if you can upload the patch.

Flags: needinfo?(twsmith)
Attached patch solution1.patchSplinter Review
Attached patch solution2.patchSplinter Review

I uploaded two different solutions, could you please try both of them, one at a time?

With only solution1.patch applied I am able to reproduce the issue.
With only solution2.patch applied I am not able to reproduce the issue.

Hmm, I guess my first solution is not complete. When we are starting a load we might be saving principals in the browsing context in process A, and then end up targeting process B for the load, so during deserialization of the LoadInfoArgs we would be clearing principals for that BC in process B, whereas we should be doing that in process A.

I think the easiest thing to do right now is to remove the assertion and only use the saved principal if it's equal to the flattened one. I will add a note saying that the saved principals in the browsing context might correspond to a previous load that happened in this browsing context in this process. This is not an issue from a security perspective, because with fission enabled, the principals we save in the browsing context are corresponding to the loads that take place within that the process that the browsing context is in, according to our site isolation policy.

With fission enabled, when we are starting a load, we might be saving
principals for a specific browsing context in process A, and then end up
targetting process B for the load, so during deserialization of the
LoadInfoArgs struct, we will end up using principals that were saved during a
previous load targetting that browsing context (with the same id) but in
process B.

Therefore, we cannot assert (without clearing the saved principals in the
original browsing context when a load is done, which can be done as a follow up
work) that the saved principal will be equal to the serialized one from
LoadInfoArgs.

Pushed by agakhokidze@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/33a1a3a2d64b
Only use the principal to inherit if it's equal to the flattened one, r=nika
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: