1616254 - Crash in [@ mozilla::dom::ServiceWorkerManager::RegistrationDataPerPrincipal::ScopeContainer::MatchScope]

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect, P2)

Description

[Pascal Chevrel:pascalc]

This bug is for crash report bp-64b88d71-391c-4dfa-817d-474260200218.

Top 10 frames of crashing thread:

0 xul.dll mozilla::dom::ServiceWorkerManager::RegistrationDataPerPrincipal::ScopeContainer::MatchScope const dom/serviceworkers/ServiceWorkerManager.cpp:239
1 xul.dll static mozilla::dom::ServiceWorkerManager::FindScopeForPath dom/serviceworkers/ServiceWorkerManager.cpp:1813
2 xul.dll mozilla::dom::ServiceWorkerManager::GetServiceWorkerRegistrationInfo const dom/serviceworkers/ServiceWorkerManager.cpp:1716
3 xul.dll mozilla::dom::ServiceWorkerManager::GetServiceWorkerRegistrationInfo const dom/serviceworkers/ServiceWorkerManager.cpp:1700
4 xul.dll mozilla::dom::ServiceWorkerManager::IsAvailable dom/serviceworkers/ServiceWorkerManager.cpp:2270
5 xul.dll mozilla::dom::ServiceWorkerInterceptController::ShouldPrepareForIntercept dom/serviceworkers/ServiceWorkerInterceptController.cpp:44
6 xul.dll mozilla::net::HttpBaseChannel::ShouldIntercept netwerk/protocol/http/HttpBaseChannel.cpp:2836
7 xul.dll mozilla::net::nsHttpChannel::Connect netwerk/protocol/http/nsHttpChannel.cpp:742
8 xul.dll mozilla::net::nsHttpChannel::ContinueOnBeforeConnect netwerk/protocol/http/nsHttpChannel.cpp:709
9 xul.dll mozilla::net::nsHttpChannel::OnBeforeConnect netwerk/protocol/http/nsHttpChannel.cpp:635

First crash started with 20200215095617
Changelog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f8ec5d8b4718a6456446b56c40ea32c50c631d48&tochange=02b1aa498dd2269ea45fb47e7e50f3d45d26bd3c

Comment 1

[Jens Stutte [:jstutte]]

Perry, these are the diagnostic asserts introduced by bug 1609665. Does this help?

Comment 2

[Perry Jiang [:perry] [no longer employee, use ni?]]

Yeah, this definitely looks like it will help.

Comment 3

[Daniel Holbert [:dholbert]]

I'm hitting a full-browser crash with this signature, 100% reliably, when trying to visit https://finance.yahoo.com/quote/%5EGSPC?p=^GSPC (which is the URL you land on if you visit finance.yahoo.com and click "S&P 500"). I crash right away when the page finishes loading.

Sample crash report (in a fresh profile): bp-16b0b62e-e7bf-4c30-8790-d74f30200224

Given the stock market drop today, I wouldn't be surprised if we start seeing this a lot, for folks checking graphs on Yahoo Finance...

Perry, maybe you could take a look (particularly if this URL ^^ helps you reproduce/investigate)?

Comment 4

[Perry Jiang [:perry] [no longer employee, use ni?]]

(In reply to Daniel Holbert [:dholbert] from comment #3)

I'm hitting a full-browser crash with this signature, 100% reliably, when trying to visit https://finance.yahoo.com/quote/%5EGSPC?p=^GSPC (which is the URL you land on if you visit finance.yahoo.com and click "S&P 500"). I crash right away when the page finishes loading.

Sample crash report (in a fresh profile): bp-16b0b62e-e7bf-4c30-8790-d74f30200224

Given the stock market drop today, I wouldn't be surprised if we start seeing this a lot, for folks checking graphs on Yahoo Finance...

Perry, maybe you could take a look (particularly if this URL ^^ helps you reproduce/investigate)?

Thanks for STR, I'm looking into it now

Comment 5

[Daniel Holbert [:dholbert]]

FWIW my STR aren't triggering the crash for me on a different machine (still fresh profile, latest nightly), so it might depend on precisely which ads you get served (and I got "lucky" earlier on when I was crashing).

Comment 6

[Perry Jiang [:perry] [no longer employee, use ni?]]

It tries to check that "https://finance.yahoo.com/quote/%5EGSPC?p=^GSPC" and "https://finance.yahoo.com" have equals principals, but "https://finance.yahoo.com/quote/%5EGSPC?p=^GSPC" fails to parse as a principal because the "^GSPC" suffix is being interpreted as an invalid origin attributes key-value pair. The "^" is supposed to be encoded though, according to https://www.ietf.org/rfc/rfc1738.txt on [Page 2]:

Other characters are unsafe because gateways and other transport agents are known to sometimes modify such characters. These characters are "{", "}", "|", "", "^", "~", "[", "]", and "`".

All unsafe characters must always be encoded within a URL.

Comment 7

[Perry Jiang [:perry] [no longer employee, use ni?]]

Attachment: Bug 1616254 - MatchScope should check same-origin using ScopeToPrincipal helper r?#dom-workers-and-storage,asuth

Fixes an issue where directly converting a string to principal causes the
string to fail to be parsed due to "^" being interpreted as the start of
origin attributes (and the rest of the string isn't able to be parsed as
origin attributes).

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6a748b335653
MatchScope should check same-origin using ScopeToPrincipal helper r=dom-workers-and-storage-reviewers,asuth

Comment 9

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/6a748b335653

Comment 10

[Jens Stutte [:jstutte]]

If this is not risky: Should we think about uplifting?

Comment 11

[Perry Jiang [:perry] [no longer employee, use ni?]]

It's not risky, but I don't think we need to worry about uplifting because the MOZ_DIAGNOSTIC_ASSERT is disabled for beta (and the assertion failure appears to have been a false positive anyways).