Skip to content

Accessing stdin with file descriptor 0 on Windows causes exception if there's no input / if stdin is closed #19831

Closed

Description

@mklement0
  • Version: v9.11.1
  • Platform: Windows 10 Pro (64-bit; v10.0.15063)
  • Subsystem: fs module

On Windows, run the following from a cmd.exe console ("Command Prompt"):

:: Echo stdin input

::  OK: Nonempty stdin input.
echo hi | node -pe "require('fs').readFileSync(0).toString()"

:: BREAKS ON WINDOWS: no stdin input, which should *prompt* for it when run in a terminal.
node -pe "require('fs').readFileSync(0).toString()"

:: BREAKS ON WINDOWS: stdin input closed; should return an empty line.
node -pe "require('fs').readFileSync(0).toString()" < NUL

The last 2 commands break as follows:

fs.js:531
    binding.fstat(fd);
            ^

Error: EISDIR: illegal operation on a directory, fstat
    at tryStatSync (fs.js:531:13)
    at Object.fs.readFileSync (fs.js:567:3)
...

Using .readFile() and the readline module is equally affected.

On Unix platforms, the behavior is as expected in all cases.

Activity

  1. jens1o commented on Apr 5, 2018

    @jens1o

    Can reproduce this on Win.

  2. ryzokuken commented on Apr 5, 2018

    @ryzokuken
    Contributor

    Just confirmed, works on both Linux and MacOS. @mklement0 @jens1o could you try a few other versions of Node? Does this also happen on LTS?

  3. vsemozhetbyt commented on Apr 5, 2018

    @vsemozhetbyt
    Contributor

    Window 7 x64.
    Throws with Node.js 06.14.0, 08.11.0, 09.11.1.
    OK with the master (Node.js 10.0.0. 2018-04-03 nightly).

  4. added
    fsIssues and PRs related to file-system APIs and the fs module.
    windowsIssues and PRs related to the Windows platform.
    on Apr 5, 2018
  5. ryzokuken commented on Apr 5, 2018

    @ryzokuken
    Contributor

    @vsemozhetbyt if it's okay with nightly, can't we just find out which commit "accidentally" fixed it and backport it?

  6. vsemozhetbyt commented on Apr 5, 2018

    @vsemozhetbyt
    Contributor

    I can try to brute-force-bisect)

  7. changed the title [-]Accessing stdin with file descriptor 0 on windows causes exception if there's no input / if stdin is closed[/-] [+]Accessing stdin with file descriptor 0 on Windows causes exception if there's no input / if stdin is closed[/+] on Apr 5, 2018
  8. vsemozhetbyt commented on Apr 5, 2018

    @vsemozhetbyt
    Contributor

    Unbreak point:

    > node.v10.0.0-nightly20180116f75bc2c1a5.exe -pe "require('fs').readFileSync(0).toString()"
    fs.js:634
        binding.fstat(fd);
                ^
    
    Error: EBADF: bad file descriptor, fstat
        at tryStatSync (fs.js:634:13)
        at Object.fs.readFileSync (fs.js:670:3)
        at [eval]:1:15
        at Script.runInThisContext (vm.js:64:33)
        at Object.runInThisContext (vm.js:185:38)
        at Object.<anonymous> ([eval]-wrapper:6:22)
        at Module._compile (module.js:660:30)
        at evalScript (bootstrap_node.js:484:27)
        at startup (bootstrap_node.js:170:9)
        at bootstrap_node.js:640:3
    
    > node.v10.0.0-nightly20180116f75bc2c1a5.exe -pe "require('fs').readFileSync(0).toString()" < NUL
    fs.js:634
        binding.fstat(fd);
                ^
    
    Error: EINVAL: invalid argument, fstat
        at tryStatSync (fs.js:634:13)
        at Object.fs.readFileSync (fs.js:670:3)
        at [eval]:1:15
        at Script.runInThisContext (vm.js:64:33)
        at Object.runInThisContext (vm.js:185:38)
        at Object.<anonymous> ([eval]-wrapper:6:22)
        at Module._compile (module.js:660:30)
        at evalScript (bootstrap_node.js:484:27)
        at startup (bootstrap_node.js:170:9)
        at bootstrap_node.js:640:3
    
    > node.v10.0.0-nightly201801171b0d9795b6.exe -pe "require('fs').readFileSync(0).toString()"
    123
    ^Z
    123
    
    > node.v10.0.0-nightly201801171b0d9795b6.exe -pe "require('fs').readFileSync(0).toString()" < NUL
    
    >
  9. vsemozhetbyt commented on Apr 5, 2018

    @vsemozhetbyt
    Contributor

    So it seems some of these commits may have the fix:

    f75bc2c...1b0d979

  10. ryzokuken commented on Apr 5, 2018

    @ryzokuken
    Contributor
  11. vsemozhetbyt commented on Apr 5, 2018

    @vsemozhetbyt
    Contributor

    Let's see what @nodejs/fs think)

  12. bzoz commented on May 16, 2018

    @bzoz
    Contributor

    This was fixed in 8c00a80 but then it was broken again in #20167. Now we get:

    >node -pe "require('fs').readFileSync(0).toString()"
    fs.js:163
      return (stats[1/* mode */] & S_IFMT) === fileType;
                   ^
    
    TypeError: Cannot read property '1' of undefined
        at isFileType (fs.js:163:16)
        at Object.fs.readFileSync (fs.js:455:7)
        at [eval]:1:15
        at Script.runInThisContext (vm.js:91:20)
        at Object.runInThisContext (vm.js:298:38)
        at Object.<anonymous> ([eval]-wrapper:6:22)
        at Module._compile (internal/modules/cjs/loader.js:678:30)
        at evalScript (internal/bootstrap/node.js:542:27)
        at startup (internal/bootstrap/node.js:204:9)
        at bootstrapNodeJSCore (internal/bootstrap/node.js:575:3)

    /cc @joyeecheung

    EDIT: it only reproduces on Windows, on Linux it works fine

  13. joyeecheung commented on May 16, 2018

    @joyeecheung
    Member

    @bzoz Thanks for the ping. Looks like it's a corner case of tryStatSync, I can think of a few ways to fix this:

    1. Special case for fd 0, 1, 2 and do not check S_IFREG with fstat
    2. Fix fstat for fd 0, it currently returns EISDIR (looks like a libuv bug? Or is it a wont-fix? cc @bnoordhuis @cjihrig), the difference relies in how we handle the error. 8c00a80 accidentally fixed this when it ignored the error if the fd was supplied by users. At that commit the S_IFREG was applied on the global stats array that contained the result of last (possibly irrelevant) successful operation.
  14. joyeecheung commented on May 16, 2018

    @joyeecheung
    Member

    Also this is where the root cause is:

    > fs.fstatSync(0)
    Error: EISDIR: illegal operation on a directory, fstat
        at Object.fs.fstatSync (fs.js:894:3)
    > fs.fstatSync(1)
    Error: EISDIR: illegal operation on a directory, fstat
        at Object.fs.fstatSync (fs.js:894:3)
    > fs.fstatSync(2)
    Error: EISDIR: illegal operation on a directory, fstat
        at Object.fs.fstatSync (fs.js:894:3)
    
  15. 6 remaining items

  16. GiGurra commented on Nov 3, 2019

    @GiGurra

    @addaleax I have been looking into this now for a couple of hours and have found what I believe could be the source of my issue.

    • my case works in linux/bash/fish (cat somefile.txt | myscript)
    • my case works in windows cmd (TYPE somefile.txt | myscript)
    • my case does NOT work in powershell (cat somefile.txt | myscript), gives the error listed in this issue

    My test script is as simple as it gets :)

    #!/usr/bin/env node
    console.log("printer running")
    console.log(require('fs').readFileSync(process.stdin.fd).toString());
    

    npm link/install on the above and then doing cat somefile.txt | myscript ==> fails with the error mentioned in this github issue.

    I believe it has to do with how powershell works together with the startup script that npm install creates. (npm install creates a powershell startup script, cmd startup script and cygwin startup script)

    I dont know much/anything about powershell tbh, but it seems like it does some funky stuff to stdin -not just forwarding it to any child processes spawned by the powershell script, and in the process the generated powershell script for any cli application installed with npm install will lose access to stdin.

    Conclusion: The powershell startup script that npm install/link creates does not forward stdin properly/at all.

    Update: Confirmed. If explicitly tell powershell to use the generated cmd script, it works fine :)

    • powershell cat src.txt | myscript.cmd works
    • powershell cat src.txt | myscript gives the error
    • powershell cat src.txt | myscript.ps1 gives the error
  17. mklement0 commented on Nov 3, 2019

    @mklement0
    Author

    Note that the commands in the OP still fail, irrespective of whether you run them from cmd.exe or PowerShell, as of v13.01.

    However, the error message for closed stdin / empty input has changed and is now:

    fs.js:168
      return (stats[1/* mode */] & S_IFMT) === fileType;
                   ^
    
    TypeError: Cannot read property '1' of undefined
    ...
    

    @GiGurra: I suspect the PowerShell issue you're describing is a separate issue: it may be that the .ps1 file you're running is not designed to accept stdin input. To do so, it would have to check the automatic $Input variable explicitly.

    I'm not familiar with the mechanism that auto-generates .ps1 files in the context of npm install (can you provide a link?), but it sounds like that script-generation process should be revisited in order to provide stdin support.

    Here's an example that shows that nonempty stdin input is received in a PowerShell Script:

    Create test file t.ps1 with the following content:

    $Input | node -pe "require('fs').readFileSync(0).toString()"

    Then invoke it from PowerShell as follows, and you'll see that the stdin input is correctly received and echoed:

    # Provide stdin input and echo it.
    PS> 'echo me' | .\t.ps1
    echo me

    Again, not providing stdin input surfaces the error, albeit with a different error message - whether that points to yet another problem is unclear to me:

    # Provide NO stdin input
    PS> .\t.ps1
    internal/fs/utils.js:220
        throw err;
        ^
    
    Error: EOF: end of file, read
    ...
  18. saghul commented on Nov 5, 2019

    @saghul
    Member

    Do things work if you use process.stdin.read instead of the fs APIs?

  19. mklement0 commented on Nov 7, 2019

    @mklement0
    Author

    @saghul

    The following work correctly:

    node -e "process.stdin.pipe(process.stdout)"
    
    node -e "process.stdin.on('readable', function() { try { process.stdout.write(process.stdin.read()) } catch {} })"
    

    Without the try / catch, an exception about writing a null to the stream is thrown. Is that expected?

  20. addaleax commented on Nov 7, 2019

    @addaleax
    Member

    Without the try / catch, an exception about writing a null to the stream is thrown. Is that expected?

    It’s intentional that stdin.read() returns null when no more data is available or end-of-stream is reached, and it’s intentional that passing data that is not string or buffer to stdout.write() leads to an exception.

    I’m actually personally a bit torn on whether this is the right behaviour, i.e. whether .write(null) shouldn’t maybe work like .end() for consistency, but that goes a bit off-topic here… :)

  21. saghul commented on Nov 8, 2019

    @saghul
    Member

    This looks a bit like a programing error to me. On Windows you can't just write to fd 0 on all shells. You may need to do it to CONOUT$.

  22. mklement0 commented on Nov 8, 2019

    @mklement0
    Author

    @saghul: Given that we're talking about standard input, you probably meant CONIN$, but note that:

    • process.stdin.fd is 0 on Windows too (from the docs: "The process.stdin property returns a stream connected to stdin (fd 0)"; try node -pe process.stdin.fd on Windows).
    • non-empty stdin input works just, fine as noted in the OP (echo hi | node -pe "require('fs').readFileSync(0).toString()")

    on all shells.

    The standard streams are not shell features, they're system features. Shells may choose to surface them differently, though, as PowerShell does via $Input, for instance.

    Whatever the calling shell, readFileSync(0) shouldn't fail if there's either no or empty stdin input - it works properly on Unix-like platforms, but it fails on Windows.

  23. added
    stdioIssues and PRs related to standard input, output, and error streams.
    on Jun 26, 2020
  24. huseyinacacak-janea commented on Sep 2, 2024

    @huseyinacacak-janea
    Contributor

    I've investigated this issue with Node v18.20.2, v20.16.0 and v22.7.0. Both v20 and v22 worked as expected. I guess v18 doesn't have the latest libuv release because it still has this issue.
    @mklement0 Do you think this can be closed as this issue is now fixed in the current version of Node?

  25. added
    libuvIssues and PRs related to the libuv dependency or the uv binding.
    and removed
    help wantedIssues that need assistance from volunteers or PRs that need help to proceed.
    on Sep 2, 2024
  26. mklement0 commented on Sep 2, 2024

    @mklement0
    Author

    Thanks, @huseyinacacak-janea.

    Indeed, the problem seems to be fixed in the LTS version that is current as of this writing (v20.x), so I'm closing this.

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.libuvIssues and PRs related to the libuv dependency or the uv binding.stdioIssues and PRs related to standard input, output, and error streams.windowsIssues and PRs related to the Windows platform.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions