Skip to content

Node 6 fs.realpath behavior changes #7175

Description

@isaacs
  • Version: 6
  • Platform: Tested on Darwin and Linux
  • Subsystem: fs

Previously, looping symbolic links and long path names were handled by fs.realpath without issue. The changelog says that fs.realpath "can throw new errors", but fails to identify that it also is less powerful (and also does not identify what those new errors are).

Seems like it'd be possible to maintain backwards compatibility if Node could fall back to the slower but more resilient implementation when ELOOP or ENAMETOOLONG are raised.

If this is not desirable, then the changelog should be updated to reflect the reduction in functionality.

I have no strong opinion about whether or not Node is capable of resolving the realpath of long looping symbolic links, but I would very much like a thing to point to when people complain about glob being broken. Thanks.

$ node -v
v5.3.0

$ cat rp.js
var fs = require('fs')

var l = 128

var long = 'rptest/' + Array(l).join('a/b/') + 'a'

clean()
process.on('exit', clean)
function clean () {
  try { fs.unlinkSync('rptest/a/b') } catch (e) {}
  try { fs.rmdirSync('rptest/a') } catch (e) {}
  try { fs.rmdirSync('rptest') } catch (e) {}
}

fs.mkdirSync('rptest')
fs.mkdirSync('rptest/a')
fs.symlinkSync('..', 'rptest/a/b')

console.log(fs.realpathSync(long))
fs.realpath(long, console.log)

$ node rp.js
/home/isaacs/rptest/a
null '/home/isaacs/rptest/a'

$ nave use 6

$ node rp.js
fs.js:1568
  return binding.realpath(pathModule._makeLong(path), options.encoding);
                 ^

Error: ELOOP: too many symbolic links encountered, realpath 'rptest/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a/b/a'
    at Error (native)
    at Object.realpathSync (fs.js:1568:18)
    at Object.<anonymous> (/home/isaacs/rp.js:19:16)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)
    at Function.Module.runMain (module.js:575:10)
    at startup (node.js:160:18)

Activity

  1. added
    fsIssues and PRs related to file-system APIs and the fs module.
    on Jun 6, 2016
  2. ChALkeR commented on Jun 6, 2016

    @ChALkeR
    Member

    @isaacs Is this related to npm/npm#5082 fix being blocked?

  3. jhamhader commented on Jun 6, 2016

    @jhamhader
    Contributor

    Introduced in #3594
    By the way, what happens when you try to fs.stat() on the link?

  4. addaleax commented on Jun 6, 2016

    @addaleax
    Member

    #6861 and #7044 are also issues that stem from the new realpath implementation.

    Falling back to a JS-based realpath implementation sounds like a good idea to me.

  5. Fishrock123 commented on Jun 6, 2016

    @Fishrock123
    Contributor

    @thealphanerd citgm tests glob, right? @isaacs Did glob have tests for it previously?

  6. jasnell commented on Jun 6, 2016

    @jasnell
    Member

    @Fishrock123 ... the primary issue with glob is two fold: (a) fs.realpath now throws errors when it previously did not and (b) when those errors are thrown is platform specific.

    Previously, if I use glob with realpath on a circular symlink, it would recurse until a name too long error would occur, which is being caught and intentionally used by glob. This error is essentially the signal that glob should stop recursing and just returns the result up to that point. On certain operating systems (e.g. OSX), the libuv realpath implementation now throws ELOOP errors before the other error is thrown. Since glob does not ignore the ELOOP errors, the app dies. On Linux, the eloop and name too long errors throw at the same depth so the problem is not encountered because the ignored error comes first and the loop inside glob exits before the eloop can occur. On OSX, however, the eloop happens first.

    The glob code was written to assume that fs.realpath would never throw an errors of it's own (there's no defensive error handling logic around the call to realpath).

  7. Fishrock123 commented on Jun 6, 2016

    @Fishrock123
    Contributor

    The glob code was written to assume that fs.realpath would never throw an errors of it's own (there's no defensive error handling logic around the call to realpath).

    I think glob's assumptions here are ok in terms of errors, fwiw.

    Edit: As long as you don't pass it obviously invalid parameters

  8. Fishrock123 commented on Jun 6, 2016

    @Fishrock123
    Contributor

    As a note, the realpath change is listed on the breaking change doc: https://github.com/nodejs/node/wiki/Breaking-changes-between-v5-and-v6#fs

    However it does not note this specifically, and it does not mean that there isn't still a problem or bug here.

  9. jasnell commented on Jun 6, 2016

    @jasnell
    Member

    Note, I'm not saying that glob's assumptions around error handling are right or wrong, just describing the situation. Previously fs.realpath did not throw, and now it does throw based on platform specific differences.

  10. jhamhader commented on Jun 6, 2016

    @jhamhader
    Contributor

    It is important to note that other fs functions also have this issue. I verified that about the following:

    • stat()
    • access()
    • chmod()
    • chown()
    • open()
    • readdir()
    • watch()
    • All other fs functions which uses the above functions (truncate(), writeFile(), etc)

    There is almost no fs function which is not affected.

  11. isaacs commented on Jun 10, 2016

    @isaacs
    ContributorAuthor

    @Fishrock123 Yes, glob's tests fail on Node 6. There's a PR from @thealphanerd which makes the test pass, but there are problems with the fix. It can't really be hacked around in node-glob without a very subtle and significant breaking change in node-glob that'll require people to update their code.

    Imo, citgm should have prevented this change from happening in the first place. It is a bug in the process that it got out into a node release. That underlying process bug should be fixed, along with this technical bug.

    @jhamhader Prior to node 6, one could use fs.realpath to avoid ELOOP errors in other fs operations, precisely because it would walk up the path from left to right resolving symbolic links.

  12. isaacs commented on Jun 10, 2016

    @isaacs
    ContributorAuthor
  13. saghul commented on Jun 11, 2016

    @saghul
    Member

    Prior to node 6, one could use fs.realpath to avoid ELOOP errors in other fs operations, precisely because it would walk up the path from left to right resolving symbolic links.

    Was this behavior documented? Generally speaking, all fs APIs are thing wrappers around their POSIX counterparts, and very thin wrappers at that. IMHO the general expectation is that they work just the man page says. In this line, I consider the current implementation works as expected.

    FTR, the work done here was to make fs.realpath faster, and keeping the JS fallback was met with resistance back then. We waited for Windows XP support to go away in order to land it, because uv_fs_realpath doesn't work there.

    The way I see it the only feasible solution here is to catch the error at the application level and handle it.

    If this is not desirable, then the changelog should be updated to reflect the reduction in functionality.

    I have no strong opinion about whether or not Node is capable of resolving the realpath of long looping symbolic links, but I would very much like a thing to point to when people complain about glob being broken. Thanks.

    I understand that a documentation change is acceptable to you then?

  14. ChALkeR commented on Jun 11, 2016

    @ChALkeR
    Member

    @isaacs

    It can't really be hacked around in node-glob without a very subtle and significant breaking change in node-glob that'll require people to update their code.

    Does this mean that other fs functions have also been affected, not only realpath()? Afaik only module uses fs.realpath in Node.js. What prevents from re-implementing the same functionality?

    Prior to node 6, one could use fs.realpath to avoid ELOOP errors in other fs operations, precisely because it would walk up the path from left to right resolving symbolic links.

    Perhaps what we need here need a separate method for just that, either in fs or in userland?

  15. isaacs commented on Jun 15, 2016

    @isaacs
    ContributorAuthor

    I understand that a documentation change is acceptable to you then?

    Adequately documenting breaking changes in a platform is the bare minimum requirement. Saying that something "raises new errors", without specifying what those errors are, when they are raised, or that they are platform-specific, is inadequate.

    It is not a "fix" though, and I'll be disappointed if that's the resolution.

    What prevents from re-implementing the same functionality?

    Time and typing.

    It looks like I'm going to have to fall back to the pre-node6 fs.realpath implementation anyway, I'd just prefer that it be in the platform rather than in my userland code, because I care a great deal about the stability of the Node.js platform and the community built on top of it. Stability is the point of a platform; it's what a platform is for, and breaking stability imposes a high cost. Stuff that was built on that platform falls over.

    Do not be flippant about that cost. Every time something like this comes up, it imposes many person-hours of work, and reduces confidence in Node.js. Ignoring that cost is user-hostile, especially if the justification is "Well, it was wrong of the user to rely on this thing not breaking anyway". I'm not saying that breaking changes are never justified, but they are expensive. As a platform, Node has a responsibility to make sure the benefit is valuable enough to be worth it.

    What is the point of making fs.realpath faster if most people don't use it directly, and modules that expose a realpath interface now all have to port the old (slower) implementation forward into their code? It seems like misguided priorities to me. If falling back to a JS implementation to preserve stability was discussed and rejected, then I'd argue that the discussion was with the wrong people or in the wrong context or based on the wrong set of values, because it is the wrong decision.

    The net impact of stuff like this is that module authors feel the need to scramble and do a bunch of busy-work to catch up to the ways that core has broken their programs. It causes resentment and reduces trust. I'm guessing that this is not the intent of the TSC or CTC, based on what I've been told every time something like this comes up and I complain about it. Citgm is a good step in the right direction. But there needs to be a clearer understanding of trade-offs much earlier in the design process. I don't see that happening. I see my code breaking, and then post-hoc justifications about why it was the right thing to do, based on values that are not clearly articulated and not shared by me or my users.

  16. 51 remaining items

  17. saghul commented on Jul 13, 2016

    @saghul
    Member

    Quick update: I've tried (without success) to get Windows to work properly. No luck. One clear case where our new realpath implementation fails is with ramdisks like the ones made with ImDisk. For context, we use GetFinalPathNameByHandle with VOLUME_NAME_DOS that is, with the drive letter. Turns out that some tools such as ImDisk bypass the Windows Volume Manager so GetFinalPathNameByHandle fails.

    If there is no way around this, I don't see other solution but to revert to the old code. I'll keep digging.

  18. colltoaction commented on Aug 12, 2016

    @colltoaction

    What version will include this patch? Next release on 6.X?

  19. jasnell commented on Aug 12, 2016

    @jasnell
    Member

    there's a new 6.x version coming out next week but it's not clear yet if this will be included in that. /cc @cjihrig @evanlucas

  20. cjihrig commented on Aug 15, 2016

    @cjihrig
    Contributor

    It's in the commit list at #8070.

  21. Shahjalal7311 commented on Jan 18, 2017

    @Shahjalal7311

    npm run build error in my existing project
    npm 3.10.8
    node -v 6.9.1

    i am getting this error

    ERROR in ./~/css-loader!./~/postcss-loader!./~/resolve-url-loader!./~/sass-loader?sourceMap!./~/bootstrap-loader/lib/boo
    tstrap.styles.loader.js!./~/bootstrap-loader/no-op.js
    Module build failed: Error: No PostCSS Config found in: D:\my_pos_system\node_modules\bootstrap-loader
        at Error (native)
        at D:\my_pos_system\node_modules\postcss-load-config\index.js:51:26
     @ ./~/style-loader!./~/css-loader!./~/postcss-loader!./~/resolve-url-loader!./~/sass-loader?sourceMap!./~/bootstrap-loa
    der/lib/bootstrap.styles.loader.js!./~/bootstrap-loader/no-op.js 4:14-193
     @ ./~/bootstrap-loader/lib/bootstrap.loader.js!./~/bootstrap-loader/no-op.js
     @ ./~/bootstrap-loader/loader.js
     @ ./src/vendor.browser.ts
    Child html-webpack-plugin for "index.html":
            + 4 hidden modules
    Child extract-text-webpack-plugin:
                                        Asset     Size  Chunks             Chunk Names
         19e65b89cee273a249fba4c09b951b74.eot   121 kB          [emitted]
         28df6ee7b407fd8a14b40bc01f4fd3ae.svg   334 kB          [emitted]
         dd4781d1acc57ba4c4808d1b44301201.ttf   189 kB          [emitted]
        2c159d0d05473040b53ec79df8797d32.woff  67.9 kB          [emitted]
            + 6 hidden modules
    
    npm ERR! Windows_NT 6.1.7601
    npm ERR! argv "C:\\Program Files\\nodejs\\node.exe" "C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js" "run
    " "webpack" "--" "--config" "config/webpack.prod.js"
    npm ERR! node v6.9.1
    npm ERR! npm  v3.10.8
    npm ERR! code ELIFECYCLE
    npm ERR! pos-system@0.0.1 webpack: `webpack --progress --profile --bail "--config" "config/webpack.prod.js"`
    npm ERR! Exit status 2
    npm ERR!
    npm ERR! Failed at the pos-system@0.0.1 webpack script 'webpack --progress --profile --bail "--config" "config/webpack.p
    rod.js"'.
    npm ERR! Make sure you have the latest version of node.js and npm installed.
    npm ERR! If you do, this is most likely a problem with the pos-system package,
    npm ERR! not with npm itself.
    npm ERR! Tell the author that this fails on your system:
    npm ERR!     webpack --progress --profile --bail "--config" "config/webpack.prod.js"
    npm ERR! You can get information on how to open an issue for this project with:
    npm ERR!     npm bugs pos-system
    npm ERR! Or if that isn't available, you can get their info via:
    npm ERR!     npm owner ls pos-system
    npm ERR! There is likely additional logging output above.
    
    npm ERR! Please include the following file with any support request:
    npm ERR!     D:\my_pos_system\npm-debug.log
    
    npm ERR! Windows_NT 6.1.7601
    npm ERR! argv "C:\\Program Files\\nodejs\\node.exe" "C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js" "run
    " "build:prod"
    npm ERR! node v6.9.1
    npm ERR! npm  v3.10.8
    npm ERR! code ELIFECYCLE
    npm ERR! pos-system@0.0.1 build:prod: `npm run webpack -- --config config/webpack.prod.js`
    npm ERR! Exit status 1
    npm ERR!
    npm ERR! Failed at the pos-system@0.0.1 build:prod script 'npm run webpack -- --config config/webpack.prod.js'.
    npm ERR! Make sure you have the latest version of node.js and npm installed.
    npm ERR! If you do, this is most likely a problem with the pos-system package,
    npm ERR! not with npm itself.
    npm ERR! Tell the author that this fails on your system:
    npm ERR!     npm run webpack -- --config config/webpack.prod.js
    npm ERR! You can get information on how to open an issue for this project with:
    npm ERR!     npm bugs pos-system
    npm ERR! Or if that isn't available, you can get their info via:
    npm ERR!     npm owner ls pos-system
    npm ERR! There is likely additional logging output above.
    

    debug log:

    0 info it worked if it ends with ok
    1 verbose cli [ 'C:\\Program Files\\nodejs\\node.exe',
    1 verbose cli   'C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js',
    1 verbose cli   'run',
    1 verbose cli   'build:prod' ]
    2 info using npm@3.10.8
    3 info using node@v6.9.1
    4 verbose run-script [ 'prebuild:prod', 'build:prod', 'postbuild:prod' ]
    5 info lifecycle pos-system@0.0.1~prebuild:prod: pos-system@0.0.1
    6 verbose lifecycle pos-system@0.0.1~prebuild:prod: unsafe-perm in lifecycle true
    7 verbose lifecycle pos-system@0.0.1~prebuild:prod: PATH: C:\Program Files\nodejs\node_modules\npm\bin\node-gyp-bin;D:\my_pos_system\node_modules\.bin;C:\ProgramData\Oracle\Java\javapath;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;D:\xampp\php;C:\Program Files (x86)\Skype\Phone\;C:\Program Files\Git\cmd;C:\ProgramData\ComposerSetup\bin;C:\Program Files (x86)\PuTTY\;C:\mongodb\bin;C:\Program Files\nodejs\;cmd;C:\Program Files\MongoDB\Server\3.0\bin;C:\Users\Arobil\AppData\Roaming\npm\node_modules\angular-cli\bin;C:\Users\Arobil\AppData\Roaming\npm;C:\Program Files (x86)\Microsoft VS Code\bin
    8 verbose lifecycle pos-system@0.0.1~prebuild:prod: CWD: D:\my_pos_system
    9 silly lifecycle pos-system@0.0.1~prebuild:prod: Args: [ '/d /s /c', 'npm run clean:dist' ]
    10 silly lifecycle pos-system@0.0.1~prebuild:prod: Returned: code: 0  signal: null
    11 info lifecycle pos-system@0.0.1~build:prod: pos-system@0.0.1
    12 verbose lifecycle pos-system@0.0.1~build:prod: unsafe-perm in lifecycle true
    13 verbose lifecycle pos-system@0.0.1~build:prod: PATH: C:\Program Files\nodejs\node_modules\npm\bin\node-gyp-bin;D:\my_pos_system\node_modules\.bin;C:\ProgramData\Oracle\Java\javapath;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;D:\xampp\php;C:\Program Files (x86)\Skype\Phone\;C:\Program Files\Git\cmd;C:\ProgramData\ComposerSetup\bin;C:\Program Files (x86)\PuTTY\;C:\mongodb\bin;C:\Program Files\nodejs\;cmd;C:\Program Files\MongoDB\Server\3.0\bin;C:\Users\Arobil\AppData\Roaming\npm\node_modules\angular-cli\bin;C:\Users\Arobil\AppData\Roaming\npm;C:\Program Files (x86)\Microsoft VS Code\bin
    14 verbose lifecycle pos-system@0.0.1~build:prod: CWD: D:\my_pos_system
    15 silly lifecycle pos-system@0.0.1~build:prod: Args: [ '/d /s /c',
    15 silly lifecycle   'npm run webpack -- --config config/webpack.prod.js' ]
    16 silly lifecycle pos-system@0.0.1~build:prod: Returned: code: 1  signal: null
    17 info lifecycle pos-system@0.0.1~build:prod: Failed to exec build:prod script
    18 verbose stack Error: pos-system@0.0.1 build:prod: `npm run webpack -- --config config/webpack.prod.js`
    18 verbose stack Exit status 1
    18 verbose stack     at EventEmitter.<anonymous> (C:\Program Files\nodejs\node_modules\npm\lib\utils\lifecycle.js:255:16)
    18 verbose stack     at emitTwo (events.js:106:13)
    18 verbose stack     at EventEmitter.emit (events.js:191:7)
    18 verbose stack     at ChildProcess.<anonymous> (C:\Program Files\nodejs\node_modules\npm\lib\utils\spawn.js:40:14)
    18 verbose stack     at emitTwo (events.js:106:13)
    18 verbose stack     at ChildProcess.emit (events.js:191:7)
    18 verbose stack     at maybeClose (internal/child_process.js:877:16)
    18 verbose stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:226:5)
    19 verbose pkgid pos-system@0.0.1
    20 verbose cwd D:\my_pos_system
    21 error Windows_NT 6.1.7601
    22 error argv "C:\\Program Files\\nodejs\\node.exe" "C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js" "run" "build:prod"
    23 error node v6.9.1
    24 error npm  v3.10.8
    25 error code ELIFECYCLE
    26 error pos-system@0.0.1 build:prod: `npm run webpack -- --config config/webpack.prod.js`
    26 error Exit status 1
    27 error Failed at the pos-system@0.0.1 build:prod script 'npm run webpack -- --config config/webpack.prod.js'.
    27 error Make sure you have the latest version of node.js and npm installed.
    27 error If you do, this is most likely a problem with the pos-system package,
    27 error not with npm itself.
    27 error Tell the author that this fails on your system:
    27 error     npm run webpack -- --config config/webpack.prod.js
    27 error You can get information on how to open an issue for this project with:
    27 error     npm bugs pos-system
    27 error Or if that isn't available, you can get their info via:
    27 error     npm owner ls pos-system
    27 error There is likely additional logging output above.
    28 verbose exit [ 1, true ]
    

    how can i fix the problem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    fsIssues and PRs related to file-system APIs and the fs module.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions