Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

__filename (and thus __dirname) have undefined behaviour for symlinks. #22602

Closed
Pomax opened this issue Aug 30, 2018 · 8 comments
Closed

__filename (and thus __dirname) have undefined behaviour for symlinks. #22602

Pomax opened this issue Aug 30, 2018 · 8 comments
Labels
doc Issues and PRs related to the documentations. module Issues and PRs related to the module subsystem.

Comments

@Pomax
Copy link

Pomax commented Aug 30, 2018

the __filename (and so also __dirname) values do not report the correct paths for files and dirs that have been symlinked/junctioned, given that their behaviour is currently undefined in the API, thus not officialy having a correct behaviour in these cases.

Reading through https://nodejs.org/docs/latest/api/modules.html#modules_filename the only description is that referencing the value will yield an absolute path, but the conventional absolute path for a symlink or junction is that path, as seen locally. Instead, __filename/__dirname yield the original resource path, which can be on completely different drives or even network shares.

Given that __filename's behaviour is currently undefined for symlinks/junctions, and that the actual behaviour that manifests is contrary to normal, expected behaviour for symlink/junction, what it's currently doing is close enough to a bug to merit fixing.

  • can we change __filename to return the proper absolute path, local to the filesystem path that the file was loaded from, and,
  • can we update the API documentation to define the behaviour as "this value is symlink/junction-safe"

I know about the https://nodejs.org/api/cli.html#cli_preserve_symlinks flag, but this should not be the exception to the rule: not following a symlink to its original resource unless explicitly told to is the whole reason symlinks and junctions work, where code that checks for locality should not break just because symlinks are suddenly resolved to wildly remote paths ("am I running in a dir parallel to my owner?", "is this resource in a subdir of my dir?", "is this web asset being loaded from the dir that my config says is my asset dir?" etc. etc.)

@addaleax addaleax added the module Issues and PRs related to the module subsystem. label Sep 2, 2018
@addaleax
Copy link
Member

addaleax commented Sep 2, 2018

These values do have defined behaviour in the sense that symlinks are resolved.

It’s not clear what you are asking for here – if you think that this behaviour is wrong, then be aware that “just” changing it would be too much of a breaking change to Node.js.

@Pomax
Copy link
Author

Pomax commented Sep 3, 2018

If this is defined, then the API documentation should probably need to explain what that behaviour is. That is not currently the case, so as a developer I have every reason to believe __filename is doing completely the wrong thing and is deep-resolving a symlink it should not be deep resolving (since the value of symlinks is that their paths appear as local paths).

So at the very least: please update the docs for __filename for each API version in which its behaviour was already defined?

Also I am well aware of the fact that nothing in developer land is "just" something, that is very much one of my own pet peeves, so you'll note that my writing does not contain a request to "just" do anything, anywhere. Basically: If the behaviour is not actually defined anywhere (except in code, which people cannot be expected to read through in order to learn what expected behaviour should be), and people have been writing code that takes advantage of undocumented behaviour, then we might be able to change its behaviour and document the now explicitly expected behaviour. If, on the other hand, the behaviour is defined already, then the docs definitely need updating, because right now as a developer I do not see any mention about what __filename does with symlinks/junctions just by reading the API docs on nodejs.org). I only see it do what appears to be the wrong thing (initially revealed through #22592 (comment))

@addaleax addaleax added the doc Issues and PRs related to the documentations. label Sep 3, 2018
@Trott
Copy link
Member

Trott commented Nov 22, 2018

It does seem that bash returns the symlink name if you do echo $0 in a symlinked file while node does not if you do console.log(__filename).

$ ls -l fhqwhgads.js
lrwxr-xr-x  1 trott  staff  7 Nov 21 19:14 fhqwhgads.js -> test.js
$ cat fhqwhgads.js
console.log(__filename);
$ node fhqwhgads.js 
/Users/trott/io.js/test.js
$ ls -l fhqwhgads.sh
lrwxr-xr-x  1 trott  staff  7 Nov 21 19:21 fhqwhgads.sh -> test.sh
$ cat fhqwhgads.sh 
echo $0
$ bash fhqwhgads.sh
fhqwhgads.sh
$

@sam-github
Copy link
Contributor

Behaves as documented.

This is the resolved absolute path of the current module file.

"Resolved" is the key, here. Its easy to not notice, or not know that the fs API docs use "resolve" to describe what occurs via fs.realpath().

The docs can always be improved, perhaps the OP has time to take a shot at this?

@Pomax
Copy link
Author

Pomax commented Nov 22, 2018

I'm already overextending myself in that respect, but maybe someone else who has more familiarity with the docs as a whole knows how to rephrase that such that it's clear to new (and long time?) users of Nodejs, there as well as in any other place where "resolve" is used to mean the absolute path.

Trott added a commit to Trott/io.js that referenced this issue Nov 24, 2018
Make it more explicit that symlinks are resolved in `__filename`.

Refs: nodejs#22602 (comment)
Trott added a commit to Trott/io.js that referenced this issue Nov 27, 2018
Make it more explicit that symlinks are resolved in `__filename`.

Refs: nodejs#22602 (comment)
PR-URL: nodejs#24587
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this issue Nov 27, 2018
Make it more explicit that symlinks are resolved in `__filename`.

Refs: #22602 (comment)
PR-URL: #24587
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
rvagg pushed a commit that referenced this issue Nov 28, 2018
Make it more explicit that symlinks are resolved in `__filename`.

Refs: #22602 (comment)
PR-URL: #24587
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
refack pushed a commit to refack/node that referenced this issue Jan 14, 2019
Make it more explicit that symlinks are resolved in `__filename`.

Refs: nodejs#22602 (comment)
PR-URL: nodejs#24587
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 11, 2019
Make it more explicit that symlinks are resolved in `__filename`.

Refs: #22602 (comment)
PR-URL: #24587
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
rvagg pushed a commit that referenced this issue Feb 28, 2019
Make it more explicit that symlinks are resolved in `__filename`.

Refs: #22602 (comment)
PR-URL: #24587
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Trott
Copy link
Member

Trott commented May 19, 2019

The docs now say "This is the current module file's absolute path with symlinks resolved." So I think this can be closed? Comment or re-open if I'm wrong about that.

@Trott Trott closed this as completed May 19, 2019
@Pomax
Copy link
Author

Pomax commented May 19, 2019

That's exactly the kind of clarification that no dev can misinterpret anymore. Thank you so much!

@romamik
Copy link

romamik commented Nov 24, 2022

Not sure if I have to open a new issue, but there is exactly the same problem with process.cwd(). It resolves symlinks if there are any and this is not documented. I believe this also should at least be clarified in the docs.

Also, I think there should be a way to have the ability to find out both cwd and __filename/__dirname without symlinks resolved. For cwd() that can be an optional argument, for __dirname that can be some new module variables like __dirnamePreserveSymlinks.

Or maybe these things should respect --preserve-symlinks cli option. They are not doing this at least on mac os.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. module Issues and PRs related to the module subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants