1539666 - StorageOperationBase::ProcessOriginDirectories should treat MozURL::Init failures as obsolete origins

Status: RESOLVED FIXED

(Core :: Storage: Quota Manager, enhancement)

Description

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

It turns out that MozURL::Init will fail for the URL "https://smaug----.github.io" with NS_ERROR_MALFORMED_URI because rust-url is incorrectly applying a rule and treating the hostname as illegal. See https://github.com/servo/rust-url/issues/483 for some discussion and https://github.com/servo/rust-url/pull/484 for the pending fix.

This broke my QuotaManager on the v2.1 -> v2.2 schema upgrade.

While I think we want MozURL to get the fix via rust-url, I think it makes sense that at https://searchfox.org/mozilla-central/rev/3d469329a42644b41e66944d8da22d78d8c0f5ec/dom/quota/ActorsParent.cpp#8401 instead of returning an error, we should convert the eContent origin to an eObsolete origin so the deletion pass below removes the obsolete origin.

Thoughts?

Comment 1

[Tom Tung [:tt, :ttung]]

Sounds reasonable to me.

Note that this would also affect the case when restoring the metadata. And this issue might be the case that failing to restore the metadata (there are around 80k errors/week for Nightly users on that). I think at least we should propagate a proper error message (e.g. with the URL or leafName) to the console for these three cases [1-3].

[1] https://searchfox.org/mozilla-central/rev/3d469329a42644b41e66944d8da22d78d8c0f5ec/dom/quota/ActorsParent.cpp#3728
[2] https://searchfox.org/mozilla-central/rev/3d469329a42644b41e66944d8da22d78d8c0f5ec/dom/quota/ActorsParent.cpp#5549
[3] https://searchfox.org/mozilla-central/rev/3d469329a42644b41e66944d8da22d78d8c0f5ec/dom/quota/ActorsParent.cpp#8399

Comment 2

[Tom Tung [:tt, :ttung]]

Taking this because this blocks the storage initialization (upgrades)

Comment 3

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1539666 - Add more warning message while a URL is not recognized by MozURL and mark these directries as obsolete to pass the upgrade;

We found this issue by finding out a url with trailing "-" cannot parsed by
MozURL (more specifically, rust-url). This patch tries to fix all related
unparsed url issues at once. So, the first thing we want to fix here is to make
it more easier to be debugged. Thus, this patch adds more QM_WARNING while
MozURL::Init() is failing to parse the url. Secondly, if failures happen during
metadata restoring or upgrading, breaking the whole initialization or upgrades
is not the thing we want. Ideally, the best approach would be somehow keep the
directory and wait until the problem on MozURL to be fixed. Then, upgrade the
directroy. However, it's relative hard to do and might have many edge cases.
Therefore, this patch take the approach of removing them in these situation.
Note that restoring and upgrading should be rarly happens.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b2f1ef671ea2
Add more warning message while a URL is not recognized by MozURL and mark these directries as obsolete to pass the upgrade; r=asuth

Comment 5

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/b2f1ef671ea2

Comment 6

[Jan Varga [:janv]]

Yeah, good idea.