309183 - Software update fails if user lacks write permissions on the "update" folder/directory

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050916 
Firefox/1.4

Software Update fails if the write permissions on the 'update' folder are 
removed. To reproduce:

1. Remove write permissions on the "update" folder
2. "Help" | "Check for Updates..." or just wait for auto update

Firefox detects that an update is available and offers the download. I spoke 
with Darin and he would like the lack of write permissions to completely 
disable the update feature. Instead, here is what happens:

1. Clicking on the "Download & Install Now" button begins the download. 
HOWEVER: the buttons at the bottom of the wizard are not updated (they still 
say "Download & Install Now" and "Later"). The wizard spins forever with the 
status message "Connecting to the update sever..."
2. Clicking "Later" has no effect.
3. Clicking "Download and Install Now" again causes the wizard to report that 
the download was successful and that a restart is required.
4. Checking the update folder nothing was actually downloaded. In fact, 
no "update.xml" or "active-update.xml" files were created either.
5. The menu item says "Check for updates" still...

Comment 1

[Ryan Flint [:rflint] (ping via IRC for reviews)]

*** Bug 309367 has been marked as a duplicate of this bug. ***

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Bug 309367 is not a duplicate of this bug. That one deals with incorrectly 
enabled preferences. This one deals with a failed update.

Comment 3

[Asa Dotzler [:asa]]

Darin, what do you think we can do here in time for 1.5?

Comment 4

[Darin Fisher]

Ensuring that the update service behaves properly when the installation
directory is readonly is a key requirement for Firefox 1.5 IMO.  Taking bug.

Comment 5

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Just to clarify, I had no trouble using the update service when the 'update'
folder was set as read-only. I experienced the hang when I removed write
permissions from ntfs...

Comment 6

[Ben Turner (not reading bugmail, use the needinfo flag!)]

http://lxr.mozilla.org/mozilla/source/toolkit/mozapps/update/src/nsUpdateServic
e.js.in

61 const KEY_APPDIR          = "XCurProcD";
...
1166   get canUpdate() {
1167     try {
1168       var file = getFile(KEY_APPDIR, [FILE_PERMS_TEST]);
1169       if (!file.exists()) {
1170         file.create(nsILocalFile.NORMAL_FILE_TYPE, PERMS_FILE);
1171         file.remove(false);
1172       }
1173     }
...

The problem is that we only check the KEY_APPDIR folder (the main firefox app 
dir) for write permissions. The updates.xml and the active-update.xml files 
are stored in this directory, so this check is vital. The actual .mar updates, 
however, are downloaded to the KEY_APPDIR/Updates/#num# folder (replace #num# 
with 0, 1, 2, ...) and they are *not* checked for write access. If the user 
lacks permissions for the "Updates" folder (or it's subfolders) then the 
update process will fail.

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

After thinking about it for a little while, I guess we have two choices for 
how to fix this bug. The desired behavior, of course, is to disable the entire 
update mechanism when the user lacks write permissions.

Obviously we can't check updates/0, updates/1, updates/2, etc. out to beyond 
updates/9054 for permissions... And, in an ordinary user scenario, I can't 
imagine how someone would have write permissions for the firefox directory and 
not it's subfolders. I think we should definitely check the updates folder for 
write permissions, but beyond that I don't know.

And that would just be a band-aid type fix for the underlying problem: What 
happens is that, at some point, the downloader tries to create a file, fails, 
and software update hangs. That's the real issue. Disabling the UI 
intelligently is a good idea, but SU shouldn't hang if the downloader has no 
write permissions.

So should this bug be split into two? The fix for the UI disabling should be 
relatively easy. I'll have to dig a little more to figure out where the 
downloader is failing. Darin, what would you like to see here?

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: verify that user has write perms for 'updates/0' directory

Spoke with Darin more today, and he noted that 'updates/0' is the only
directory we use at the moment for downloading updates. This patch checks to
make sure that the user can write to that directory before allowing the update
UI to display.

Comment 9

[Darin Fisher]

Comment on attachment 197646 [details] [diff] [review]
verify that user has write perms for 'updates/0' directory

looks good, r=darin

Comment 10

[Darin Fisher]

Actually, please verify that this patch functions properly when the "updates/0"
directory does not exist.  Thanks!

Comment 11

[Darin Fisher]

It turns out that getUpdatesDir creates the folder if needed, so there is no
concern there.

Comment 12

[Darin Fisher]

fixed-on-trunk

Comment 13

[Ben Turner (not reading bugmail, use the needinfo flag!)]

See bug 310263 for followup to the second part of this bug that will not be 
fixed for FF1.5b2.

Comment 14

[Asa Dotzler [:asa]]

Darin, can you get this into the branch ASAP? Thanks.

Comment 15

[Darin Fisher]

fixed1.8