The default bug view has changed. See this FAQ.

Linux: temporary download directory is not created specifically for each user

VERIFIED FIXED in Firefox 43, Firefox OS v2.5

Status

()

Core
Networking: File
VERIFIED FIXED
a year ago
a year ago

People

(Reporter: Matthias Schiffer, Assigned: Aidin Gharibnavaz)

Tracking

({regression})

43 Branch
mozilla46
Unspecified
Linux
regression
Points:
---

Firefox Tracking Flags

(firefox42 unaffected, firefox43+ fixed, firefox44+ verified, firefox45+ verified, firefox46+ verified, firefox-esr38 unaffected, b2g-v2.5 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

a year ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:43.0) Gecko/20100101 Firefox/43.0
Build ID: 20151216131022

Steps to reproduce:

We are using Firefox in a multi-user Ubuntu environment (but other distros like Arch are affected as well).

To reproduce:
1. Have one user log in
2. Start Firefox
3. Download a file
4. Logout
5. Log in as another user
6. Try to download a file again


Actual results:

The second download failed instantaneously.

Firefox had created a directory called /tmp/mozilla_mozillaUser0 for the first user. Then it tried to use the same temporary directory as the second user, which obviously failed.


Expected results:

The download should have worked.

Looking at the code, there are two different bugs in GetDownloadDirectory in uriloader/exthandler/nsExternalHelperAppService.cpp:

1. The $USER environment variable is not respected; the code replaces its value with "mozillaUser", if only $USER, but not $USERNAME or $LOGNAME are set. Possible fix:

--- a/uriloader/exthandler/nsExternalHelperAppService.cpp
+++ b/uriloader/exthandler/nsExternalHelperAppService.cpp
@@ -416,12 +416,12 @@ static nsresult GetDownloadDirectory(nsIFile **_directory,
     const char* userName = PR_GetEnv("USERNAME");
     if (!userName || !*userName) {
       userName = PR_GetEnv("USER");
-      if (!userName || !*userName) {
-        userName = PR_GetEnv("LOGNAME");
-      }
-      else {
-        userName = "mozillaUser";
-      }
+    }
+    if (!userName || !*userName) {
+      userName = PR_GetEnv("LOGNAME");
+    }
+    if (!userName || !*userName) {
+      userName = "mozillaUser";
     }
 
     nsAutoString userDir;

2. The permissions of the temporary directory are checked, but not the owner, so the fallback counter after the username isn't incremented even though the found directory is not usable. I don't know the code of Firefox well enough to propose a fix for this.
Component: Untriaged → Download Manager
Product: Firefox → Toolkit
Duplicate of this bug: 1233547
Component: Download Manager → Networking: File
Product: Toolkit → Core
Keywords: regression, regressionwindow-wanted

Comment 2

a year ago
[Tracking Requested - why for this release]: Break downloading file in multi-user linux

Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=8b63529bd739e7d629426fbb337408d76f9c4988&tochange=20b84c6d55a0befeebd372be64497db318e7081f

Regressed by:
b046fd76907f	Aidin Gharibnavaz — Bug 377630 - Preventing filename disclosure, by putting downloaded files in a private directory. r=bz
Blocks: 377630
Status: UNCONFIRMED → NEW
status-firefox42: --- → unaffected
status-firefox43: --- → affected
status-firefox44: --- → affected
status-firefox45: --- → affected
status-firefox46: --- → affected
status-firefox-esr38: --- → unaffected
tracking-firefox43: --- → ?
tracking-firefox44: --- → ?
tracking-firefox45: --- → ?
tracking-firefox46: --- → ?
Ever confirmed: true
Keywords: regressionwindow-wanted
OS: Unspecified → Linux

Updated

a year ago
Flags: needinfo?(aidin)

Comment 3

a year ago
Hi.

I did report the same bug in 1233547, but i did also suggest a quick fix. It is enough to create TMP environment variable to point to existing directory (ie. /tmp/userx), and this bug does not show itself.

What it is odd, is that defining TMP variable does not create this directory (mozilla_mozillaUser0).

Regards,

H.
(Assignee)

Comment 4

a year ago
Sorry for that! I'll fix it soon.
Assignee: nobody → aidin
Flags: needinfo?(aidin)
(Assignee)

Comment 5

a year ago
Created attachment 8700078 [details]
MozReview Request: Bug 1233434 - Fixing download failure on a multi-user GNU/Linux machine. r?bagder

Review commit: https://reviewboard.mozilla.org/r/28565/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/28565/
Attachment #8700078 - Flags: review?(mcmanus)
(Assignee)

Comment 6

a year ago
Boris (:bz) is on vaccation, so I asked Patrik to kindly review the patch.

The try server build is here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a7318522891

I will try to write more unit tests for it, tomorrow.
Attachment #8700078 - Flags: review?(mcmanus) → review?(daniel)
So it is still set on mcmanus in mozreview and I can't figure out how to move it to me, but it's a very small and straight-forward patch with a r+ from me, if I could set it.
(Assignee)

Comment 8

a year ago
Comment on attachment 8700078 [details]
MozReview Request: Bug 1233434 - Fixing download failure on a multi-user GNU/Linux machine. r?bagder

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28565/diff/1-2/
Attachment #8700078 - Attachment description: MozReview Request: Bug 1233434 - Fixing download failure on a multi-user GNU/Linux machine. r?mcmanus → MozReview Request: Bug 1233434 - Fixing download failure on a multi-user GNU/Linux machine. r?bagder
(Assignee)

Comment 9

a year ago
Thanks (:
I updated the board by editing the commit message and pushing another commit.
Attachment #8700078 - Flags: review?(daniel) → review+
Comment on attachment 8700078 [details]
MozReview Request: Bug 1233434 - Fixing download failure on a multi-user GNU/Linux machine. r?bagder

https://reviewboard.mozilla.org/r/28565/#review25477

LGTM!
(Assignee)

Comment 11

a year ago
Thanks for the review (:
Keywords: checkin-needed
Duplicate of this bug: 1234065

Comment 13

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/babff100e9f8
Keywords: checkin-needed

Comment 14

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/babff100e9f8
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox46: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
I think the patch needs to be uplifted to aurora, beta and release so the fix goes to 43.0.3.
Flags: needinfo?(aidin)
Indeed. Please fill an uplift request to aurora and beta.
tracking-firefox44: ? → +
tracking-firefox45: ? → +
tracking-firefox46: ? → +

Updated

a year ago
Duplicate of this bug: 1234814
(Assignee)

Comment 18

a year ago
Comment on attachment 8700078 [details]
MozReview Request: Bug 1233434 - Fixing download failure on a multi-user GNU/Linux machine. r?bagder

Approval Request Comment
[Feature/regressing bug #]: Bug 377630, fails in multi-user environments.
[User impact if declined]:
[Describe test coverage new/current, TreeHerder]: Manual test
[Risks and why]: low, changes have no side-effect on other things.
[String/UUID change made/needed]:none
Flags: needinfo?(aidin)
Attachment #8700078 - Flags: approval-mozilla-beta?
Attachment #8700078 - Flags: approval-mozilla-aurora?
Comment on attachment 8700078 [details]
MozReview Request: Bug 1233434 - Fixing download failure on a multi-user GNU/Linux machine. r?bagder

Seems like a key scenario, Beta44+, Aurora45+
Attachment #8700078 - Flags: approval-mozilla-beta?
Attachment #8700078 - Flags: approval-mozilla-beta+
Attachment #8700078 - Flags: approval-mozilla-aurora?
Attachment #8700078 - Flags: approval-mozilla-aurora+

Comment 20

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/18375100124a
status-firefox45: affected → fixed

Comment 21

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/0413edcf9ca4
status-firefox44: affected → fixed
Flags: qe-verify+
Reproduced the initial issue using Firefox 43 beta 9 on Ubuntu 14.04 64-bit, verified that using Firefox 44 beta 4, latest Developer Edition 45.0a2 and latest Nightly 46.0a1 the issue is fixed.
Status: RESOLVED → VERIFIED
status-firefox44: fixed → verified
status-firefox45: fixed → verified
status-firefox46: fixed → verified
Flags: qe-verify+

Comment 23

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/0413edcf9ca4
status-b2g-v2.5: --- → fixed
I didn't see this in time to get it into either 43.0.3. We have to rebuild 43.0.4 this afternoon so potentially could still take this.
tracking-firefox43: ? → +
Comment on attachment 8700078 [details]
MozReview Request: Bug 1233434 - Fixing download failure on a multi-user GNU/Linux machine. r?bagder

[Triage Comment]

Taking this for release as well as a ridealong for 43.0.4.
Attachment #8700078 - Flags: approval-mozilla-release+
https://hg.mozilla.org/releases/mozilla-release/rev/fa63deeed20e
status-firefox43: affected → fixed
This may not affect a huge percentage of installs relative to windows installs but seems likely to be something that affects university computer labs. The patch has been verified on other channels and has been on 44 for a couple of weeks without incident. 

Note also, this should have been an uplift request on the release channel as well; I missed it over the holidays in the backlog of tracking requests, which aren't usually a priority.
You need to log in before you can comment on or make changes to this bug.