1847572 - Unix file path `~foo` should expand to `/home/foo`

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P3)

Description

[Vinny Diehl]

While implementing Bug 93141 I noticed that home directories for arbitrary users do not expand properly, e.g. ~foo/bar should expand to /home/foo/bar. We will need to modify nsLocalFile::InitWithNativePath.

Comment 1

[Vinny Diehl]

Attachment: Bug 1847572 - Expand home directory paths for arbitrary users r?nika,#xpcom-reviewers

Comment 2

[Vinny Diehl]

I've added a patch for this. The /home/<USERNAME> syntax is hardcoded and thus this relies on the default home directory having not been changed. We could parse the actual home directory out of /etc/passwd I suppose, but according to my testing Chrome doesn't go this far, so this patch achieves parity as implemented.

Comment 3

[Mike Hommey [:glandium]]

Posix provides getpwnam_r to find the home of a given user.

Comment 4

[Vinny Diehl]

(In reply to Mike Hommey [:glandium] from comment #3)

Posix provides getpwnam_r to find the home of a given user.

My latest revision implements this suggestion. Chromium explicitly avoids such an implementation, calling it a "nasty blocking call". Do you think there is any merit to this aversion? I don't see much of a risk here.

Comment 5

[Mike Hommey [:glandium]]

Yeah, there's merit to it, considering the password database might be coming from the NIS authentication server, with non-deterministic overhead. BUT, relying on something else than that would be unreliable.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/65e974428cdb
Expand home directory paths for arbitrary users r=xpcom-reviewers,nika

Comment 7

[Serban Stanca [:SerbanS]]

Backed out for causing gtest crashes.

• Backout link
• Push with failures
• Failure Log
• Failure line: PROCESS-CRASH | MOZ_ASSERT(mRawPtr != nullptr) (You can't dereference a NULL nsCOMPtr with operator->().) [@ nsCOMPtr<nsIFile>::operator->] | gtest

Comment 8

[Vinny Diehl]

Looks like these tests are busted on Android. Would you mind vouching for me for try access so I can work this out?

Comment 9

[Serban Stanca [:SerbanS]]

Hello Vinny! Sorry, but unfortunately I'm not able to vouch for you.. Maybe Aryx could help you in this matter.

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Hi Vinny. To be able to use Try, you need commit access. Please read here how to get access and mention the bug here or put me into the CC to make aware of it and I can then vouch for you.

Comment 11

[Vinny Diehl]

Hi :aryx, thank you! I've opened a commit access request at Bug 1852188.

Comment 12

[BugBot [:suhaib / :marco]]

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:vinny.diehl, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 13

[Vinny Diehl]

:nika I think this is ready to re-land :)

Edit: sorry, this ni? shouldn't have been cleared.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e0d4d00811af
Expand home directory paths for arbitrary users r=xpcom-reviewers,nika

Comment 15

[Nika Layzell [:nika] (ni? for response)]

re-landed :-)

Comment 16

[Norisz Fay [:noriszfay]]

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

Comment 17

[BugBot [:suhaib / :marco]]

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

Comment 18

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:vinny.diehl, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox119 to wontfix.

For more information, please visit BugBot documentation.

Comment 19

[Vinny Diehl]

This is a somewhat fringe feature, much less common than ~. This can ride the trains.