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

Status: VERIFIED FIXED

(Core :: Networking: File, defect)

Description

[Matthias Schiffer]

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.

Comment 2

[Alice0775 White]

[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

Comment 3

[hrvoje.habjanic]

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.

Comment 4

[Aidin Gharibnavaz]

Sorry for that! I'll fix it soon.

Comment 5

[Aidin Gharibnavaz]

Attachment: 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/

Comment 6

[Aidin Gharibnavaz]

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.

Comment 7

[Daniel Stenberg [:bagder]]

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.

Comment 8

[Aidin Gharibnavaz]

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/

Comment 9

[Aidin Gharibnavaz]

Thanks (:
I updated the board by editing the commit message and pushing another commit.

Comment 10

[Daniel Stenberg [:bagder]]

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!

Comment 11

[Aidin Gharibnavaz]

Thanks for the review (:

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/babff100e9f8

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/babff100e9f8

Comment 15

[Kohei Yoshino]

I think the patch needs to be uplifted to aurora, beta and release so the fix goes to 43.0.3.

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Indeed. Please fill an uplift request to aurora and beta.

Comment 18

[Aidin Gharibnavaz]

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

Comment 19

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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+

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/18375100124a

Comment 21

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/0413edcf9ca4

Comment 22

[Bogdan Maris, Desktop Test Engineering]

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.

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/0413edcf9ca4

Comment 24

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 25

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 26

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-release/rev/fa63deeed20e

Comment 27

[Liz Henry (:lizzard) (relman/hg->git project)]

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.