Closed Bug 1539666 Opened 7 months ago Closed 7 months ago

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

Categories

(Core :: DOM: Quota Manager, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: asuth, Assigned: tt)

Details

Attachments

(1 file)

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?

Flags: needinfo?(shes050117)
Flags: needinfo?(jvarga)

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

Flags: needinfo?(shes050117)

Taking this because this blocks the storage initialization (upgrades)

Assignee: nobody → shes050117
Status: NEW → ASSIGNED

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.

Pushed by shes050117@gmail.com:
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
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Yeah, good idea.

Flags: needinfo?(jvarga)
You need to log in before you can comment on or make changes to this bug.