Closed Bug 1030856 Opened 6 years ago Closed 6 years ago

Update hotfix doesn't react well to Deny Write Attributes and Write Extended Attributes

Categories

(Firefox :: General, defect)

defect
Not set
normal
Points:
3

Tracking

()

VERIFIED FIXED
Iteration:
33.2

People

(Reporter: gps, Assigned: gfritzsche)

References

Details

Attachments

(2 files)

Report from Paul:

On win 7 setting special permissions for the Firefox folder that Deny Write Attributes and Write Extended Attributes will stop Firefox from updating( as expected) . When Firefox is restarted the version is checked and the install starts all over again. No error is prompted so every day the user will be prompted with the notification and Firefox will not be successfully updated.
Same behavior on Win XP limited account if Firefox was installed on Program Files folder - bug 1030911
Checking the launcher directly on Win7 so far, the launcher behavior is fine after changing the permissions and restarting Windows. In that case we don't get a install.log in the install directory, which means we end up with the launcher.log being:
> 8
> Failed to open install.log file.
... which is a fatal error in update.jsm:
https://bitbucket.org/indygreg/firefox-hotfixes/src/1aecc14ef0a61f2ff9d648476119b3dc4e633343/v20140527.01/resource/update.jsm?at=default#cl-1961

Only when i skipped restart first the installer appeared to run through fine (per install.log), but Firefox wasn't actually updated.
I don't understand comment 2.

In general the experiment was designed with the installer having three branches:

* user declined UAC
* install ok
* install failed

In this case, it seems that the response should be "install failed" and so we should show the SUMO page. Am I right, and what do we need to do to get to that state?
Flags: needinfo?(georg.fritzsche)
Please scratch comment 2, that was a "here is how it looks to me at the moment" comment.

I tested around some more in the mean-time and narrowed it down:
* If the mentioned permissions are denied and no install.log is present yet, the behavior is fine (install failed, SUMO pending bug 1030911, addon uninstalled)
* If there already is an old install.log around, we pick that up, declare success and run the notification again next launch

As we need to parse the install.log for install success/failure recognition, we need to take care of already existing install.log files.
Flags: needinfo?(georg.fritzsche)
Robert, i think the best option would be to check the install.log modification dates pre and and post running the installer (if that file is present). Does that sound good?

Other options considered:
* parsing the log file for the version number - that would require the launcher to get the target version passed, seems fragile
* deleting the log file before running the launcher - permissions issues as well
Flags: needinfo?(robert.strong.bugs)
ok, I guess this is your bug then Georg.
Assignee: nobody → georg.fritzsche
Can we just compare timestamps on the install.log file before and after we run the installer?
(In reply to Georg Fritzsche [:gfritzsche] from comment #5)
> Robert, i think the best option would be to check the install.log
> modification dates pre and and post running the installer (if that file is
> present). Does that sound good?
That sounds like the best option.
Flags: needinfo?(robert.strong.bugs)
Attachment #8447302 - Flags: review?(robert.strong.bugs) → review+
gps, i think the merge is on you?
This fix is launcher-only, there is no relevant behavior change for the rest of the hotfix.
Flags: needinfo?(gps)
Points: --- → 3
Flags: firefox-backlog+
Status: NEW → ASSIGNED
Added to Iteration 33.2
Iteration: --- → 33.2
QA Whiteboard: [qa?]
QA Whiteboard: [qa?] → [qa+]
Everything is checked into the canonical tree now. You should be able to push this to https://hg.mozilla.org/releases/firefox-hotfixes/
Flags: needinfo?(gps)
https://hg.mozilla.org/releases/firefox-hotfixes/rev/e5a48d5927f1
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Hi Florin, can a QA contact be assigned to verify this bug.
Flags: needinfo?(florin.mezei)
Flags: needinfo?(florin.mezei)
QA Contact: paul.silaghi
Hi Paul, the desktop iteration ends on Monday.  Is it possible to have this bug verified by then.
Flags: needinfo?(paul.silaghi)
https://people.mozilla.org/~gszorc/hotfix-v20140527.01-qa.xpi
Everything looks ok now in case of write attributes denial - the SUMO update fail page is displayed and the hotfix is uninstalled, but I found a regression in bug 1030911 and suggest re-testing after it gets fixed first.
Flags: needinfo?(paul.silaghi)
Depends on: 1030911
Unless I committed the patch incorrectly, this broke Windows 7 for normal users.

The launcher now requests UAC, gets it, runs the installer successfully, and exits with error code 8 complaining about log access issues. This causes the JS to think the upgrade was not successful. This causes the SUMO tab to open.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Slightly embarassing follow-up, sorry about that.
Attachment #8450084 - Flags: review?(robert.strong.bugs)
Attachment #8450084 - Flags: review?(robert.strong.bugs) → review+
I did some spot checking on this before landing. All looks well.
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
(In reply to Paul Silaghi, QA [:pauly] from comment #16)
> https://people.mozilla.org/~gszorc/hotfix-v20140527.01-qa.xpi
> Everything looks ok now in case of write attributes denial - the SUMO update
> fail page is displayed and the hotfix is uninstalled, but I found a
> regression in bug 1030911 and suggest re-testing after it gets fixed first.

Hi Paul, now that Bug 1030911 is fixed can you do a final verification for Bug 1030856.
Flags: needinfo?(paul.silaghi)
Everything looks ok now.
Tested FF 10, FF 28 Win 7 x64.
Status: RESOLVED → VERIFIED
Flags: needinfo?(paul.silaghi)
QA Whiteboard: [qa+] → [qa!]
You need to log in before you can comment on or make changes to this bug.