1937098 - Profile contains an empty XPI file after an add-on update

Status: NEW

(Toolkit :: Add-ons Manager, defect, P3)

Description

[William Durand [:willdurand]]

I've investigated an internal bug report yesterday where the user had an add-on installed and apparently enabled, but it wasn't working. The extension could not be located in the Firefox UI (nav-bar or extensions panel), and it wasn't listed in about:debugging either. That being said, the extension was listed in about:addons (as enabled, without any error/warning message) and in about:support (marked as enabled too). The extensions process was also running (in about:processes).

Some browser console logs highlighted an error during startup (by disabling then re-enabling the extension). The error was thrown at https://searchfox.org/mozilla-central/rev/d1e731d931b7b46237175de1701849a7cf5c8579/toolkit/components/extensions/Extension.sys.mjs#1164

Therefore, I asked the user for the checksum of the file and it wasn't the expected one. The user confirmed that the file was different than the same XPI downloaded from AMO separately. We then noticed that the XPI file in the user's profile was empty.

The user uses Firefox for desktop on Linux. Luca and I looked into how we could end up with an XPI file that is empty, despite the use of a SafeInstallOperation instance. In fact, this class is interesting for two reasons:

1. It uses nsIFile.copyTo() or nsIFile.moveTo().
   • We very likely use moveTo(), which either calls rename() OR CopyToNative if the call to rename fails.
   • It's interesting too, because CopyToNative() starts by creating an empty file before writing to it here: https://searchfox.org/mozilla-central/rev/d1e731d931b7b46237175de1701849a7cf5c8579/xpcom/io/nsLocalFileUnix.cpp#1081-1083.
   • If the rest fails, it is likely that we end up with an empty file in the extensions/ directory.
   • An error is probably thrown at this point, but then what? Good question, see below.
2. When the call to moveTo() throws, we catch the error and then we re-throw it: https://searchfox.org/mozilla-central/rev/d1e731d931b7b46237175de1701849a7cf5c8579/toolkit/mozapps/extensions/internal/XPIInstall.sys.mjs#1144,1147-1156
   • It's great because we can rollback, right? Riiiiight?
   • It doesn't look like the rollback will work because we only rollback things that we put in a list, but that list isn't updated when we re-throw: https://searchfox.org/mozilla-central/rev/d1e731d931b7b46237175de1701849a7cf5c8579/toolkit/mozapps/extensions/internal/XPIInstall.sys.mjs#1158,1226
   • Sigh.

The current theory is therefore that the add-on was updated incorrectly. The download of the XPI went well, the integrity check on the file was fine, the signature check was also fine. All of this seems to happen in the staged directory. After that, the file was moved from the staged directory to the extensions/ one, and that's when the XPI file became an empty file. As for the rest, the metadata/xpi database was correctly updated because that's pulled from AMO and/or updated before we moved the file to the extensions/ directory. That's why the add-on "looks" enabled in about:addons.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Toolkit::Add-ons Manager' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Rob Wu [:robwu]]

I've seen this kind of issue before (more than once!), e.g. bug 1670149.

Since we has looked into this and this is relatively fresh, perhaps it would be good to take a look at a patch at the start of the new year.

Comment 3

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

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

For more information, please visit BugBot documentation.

Comment 4

[Rob Wu [:robwu]]

(clearing needinfo, to be triaged at the next meeting)

Comment 5

[mmortal03]

I'm seeing something similar to this happen again just now, where, despite Bug 1639163 having been fixed a few months ago, the \extensions\trash folder in my Firefox profile currently contains an xpi (not empty) from what I believe was an automated, possibly deferred, update attempt of the extension that must have failed, which is now blocking further automated and manual update attempts of that extension, unless the browser is restarted.

Each time Firefox checks for updates in the background for this extension, it tries and fails to extract the downloaded xpi to update it (because the trash folder item remains in the way), and then the extension gets removed from the address bar, exactly as described above, effectively deactivating it, but where it still shows up as enabled under "Manage your extensions".

Just a theory, but maybe the fix for Bug 1639163 was incomplete, and it also needs to include a clearing of such trash items whenever an automated update of an extension fails for whatever reason, and whenever a subsequent manual update (even from file) is initiated?

You can see my comments from a few months ago on Bug 1627495, which it now looks like the fix of Bug 1639163 has apparently not resolved: https://bugzilla.mozilla.org/show_bug.cgi?id=1627495#c38