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

files without permission entries are not found on Windows #245

Closed
schnittstabil opened this issue Jan 26, 2016 · 16 comments
Closed

files without permission entries are not found on Windows #245

schnittstabil opened this issue Jan 26, 2016 · 16 comments

Comments

@schnittstabil
Copy link
Contributor

Steps to reproduce:

  1. create a file deleteMe.txt
  2. glob.sync('deleteMe.txt') returns [ 'deleteMe.txt' ]
  3. remove all permission entries (Windows Explorer)
  4. glob.sync('deleteMe.txt') returns [ ] w/o error

(same with async…)

@schnittstabil schnittstabil changed the title files with no permission entries are not found on Windows files without permission entries are not found on Windows Jan 26, 2016
@isaacs
Copy link
Owner

isaacs commented Jan 27, 2016

What do you get when you call fs.statSync('deleteMe.txt')? Or run the glob with glob.sync('deleteMe.txt', { stat: true })?

@schnittstabil
Copy link
Contributor Author

  1. fs.statSync('deleteMe.txt')

    Error: EPERM: operation not permitted, stat 'C:\Dokumente und Einstellungen\Administrator\Eigene Dateien\deleteMe.txt'
    at Error (native)
    at Object.fs.statSync (fs.js:892:18)
    at Object.<anonymous> (C:\Dokumente und Einstellungen\Administrator\Eigene Dateien\test.js:6:31)
    at Module._compile (module.js:413:34)
    at Object.Module._extensions..js (module.js:422:10)
    at Module.load (module.js:357:32)
    at Function.Module._load (module.js:314:12)
    at Function.Module.runMain (module.js:447:10)
    at startup (node.js:139:18)
    at node.js:999:3
    
  2. glob.sync('deleteMe.txt', { stat: true }) returns [] w/o error

  3. But fs.readdirSync('.') returns ['deleteMe.txt', …]

@isaacs
Copy link
Owner

isaacs commented Jan 27, 2016

Hm. Ok, that seems kinda like a bug to me. It should be handling EPERM from readdir calls as an indication that the thing is either not a directory or not traversable, but an EPERM on a file seems like it should be an error.

I'll have to review the code to be sure there isn't some method to this madness, but I agree that it seems like an oversight somewhere, and I'll try to figure out what bash does. On Unix, there's no combination of permissions that will prevent you from being able to stat a file, so this is probably just a windows-ism that I didn't think to account for.

@mspace
Copy link

mspace commented Sep 28, 2016

I came across the same issue: sindresorhus/del#50. Also @schnittstabil pointer that this is a glob issue.

isaacs added a commit that referenced this issue Sep 29, 2016
Fix #245

This works around cases in Windows systems where we can see that a file
exists on the directory listing, but the stat/lstat call fails for some
reason other than ENOENT or ENOTDIR.
@isaacs
Copy link
Owner

isaacs commented Sep 29, 2016

Anyone affected by this error, please try the following:

npm install isaacs/node-glob#fix-245

And let me know if the problem goes away.

@isaacs
Copy link
Owner

isaacs commented Sep 29, 2016

@schnittstabil Do you think it should return an error if { stat: true } is used?

isaacs added a commit that referenced this issue Sep 29, 2016
Fix #245

This works around cases in Windows systems where we can see that a file
exists on the directory listing, but the stat/lstat call fails for some
reason other than ENOENT or ENOTDIR.
@schnittstabil
Copy link
Contributor Author

Do you think it should return an error if { stat: true } is used?

Um, no, I think glob.sync('deleteMe.txt', { stat: true }) should return filenames, not errors!? 😉 – dunno if it should throw an error…
I've only mentioned it for the sake of completeness…

@isaacs
Copy link
Owner

isaacs commented Sep 29, 2016

Right, sorry, when I said "return an error", I meant throw from sync or first arg to cb from async.

It's an interesting edge case. I could see someone thinking that if they do stat: true, then that means that the stat call should be required to succeed. It'd be easy enough to make it work that way.

isaacs added a commit that referenced this issue Sep 29, 2016
Fix #245

This works around cases in Windows systems where we can see that a file
exists on the directory listing, but the stat/lstat call fails for some
reason other than ENOENT or ENOTDIR.
@schnittstabil
Copy link
Contributor Author

@isaacs Thanks a lot, works as expected on my VM.
Btw, executing glob('d/**') on a directory d without permission entries works too, it throws an error – similar to dir d.

if they do stat: true, then that means that the stat call should be required to succeed.

IMO it only means that stat is called on all results. However, one may expect appropriate statCache entries, thus glob(… {stat: true}) should set some error entries.

@isaacs
Copy link
Owner

isaacs commented Oct 3, 2016

@schnittstabil I can't reproduce that. What error is thrown? Can you provide a test program and the output of the crash? Thanks.

@schnittstabil
Copy link
Contributor Author

schnittstabil commented Oct 3, 2016

Btw, executing glob('d/**') on a directory d without permission entries works too, it throws an error – similar to dir d.

@isaacs Thus, no crash, works as expected.

@isaacs
Copy link
Owner

isaacs commented Oct 3, 2016

@schnittstabil When you say "it throws an error", what does that mean, exactly? Being a JavaScripter, I'd assume it means something like what happens when you do throw new Error('oops')

@schnittstabil
Copy link
Contributor Author

Sorry, for the misunderstanding. To be more clear:

  1. I've created a directory d
  2. and removed all permission entries (Windows Explorer) of d

As expected:

console.log(glob.sync('d/**'));
C:\Dokumente und Einstellungen\Administrator\Desktop\glob-test>node index.js
Error: EPERM: operation not permitted, scandir 'C:\Dokumente und Einstellungen\Administrator\Desktop\glob-test\d'
    at Error (native)
    at Object.fs.readdirSync (fs.js:808:18)
    at GlobSync._readdir (C:\Dokumente und Einstellungen\Administrator\Desktop\glob-test\node_modules\glob\sync.js:288:41)
    at GlobSync._processGlobStar (C:\Dokumente und Einstellungen\Administrator\Desktop\glob-test\node_modules\glob\sync.js:350:22)
    at GlobSync._process (C:\Dokumente und Einstellungen\Administrator\Desktop\glob-test\node_modules\glob\sync.js:130:10)
    at new GlobSync (C:\Dokumente und Einstellungen\Administrator\Desktop\glob-test\node_modules\glob\sync.js:48:10)
    at Function.globSync [as sync] (C:\Dokumente und Einstellungen\Administrator\Desktop\glob-test\node_modules\glob\sync.js:26:10)
    at Object.<anonymous> (C:\Dokumente und Einstellungen\Administrator\Desktop\glob-test\index.js:3:18)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)

However, I don't know if this is intended, but I don't understand why the following error gets printed:

try {
    glob('d/**', function (err, files) {});
} catch (err) {
}
C:\Dokumente und Einstellungen\Administrator\Desktop\glob-test>node index.js
glob error { [Error: EPERM: operation not permitted, scandir 'C:\Dokumente und Einstellungen\Administrator\Desktop\glob-test\d']
  errno: -4048,
  code: 'EPERM',
  syscall: 'scandir',
  path: 'C:\\Dokumente und Einstellungen\\Administrator\\Desktop\\glob-test\\d'
}

@isaacs
Copy link
Owner

isaacs commented Oct 4, 2016

@schnittstabil Oh, I see, so if the fs.readdir call gets an error, then that fails.

What it's saying is "this is a very strange error, and probably you want to know about it, but the glob process can continue." If you pass silent: true in as an option, then it is quiet about it.

I don't know why it's warning in async and throwing in sync, though, that's strange. It looks like readdir failures need to be cleaned up a little bit as well. That's definitely a separate issue (though related cause.)

@schnittstabil
Copy link
Contributor Author

Just for the sake of completeness, errSync and errAsync are essential the same 'EPERM' error:

try {
    glob.sync('d/**');
} catch (errSync) {}

glob('d/**', errAsync => {});

The additional glob error {…}, which only happens in async, has puzzled me a bit…

@schnittstabil
Copy link
Contributor Author

I don't know why it's warning in async and throwing in sync, though, that's strange.

Found it:
sync.js#L336-L342:

    default: // some unusual error.  Treat as failure.
      this.cache[this._makeAbs(f)] = false
      if (this.strict)
        throw er
      if (!this.silent)
        console.error('glob error', er)
      break

In glob.js#L609-L619:

    default: // some unusual error.  Treat as failure.
      this.cache[this._makeAbs(f)] = false
      if (this.strict) {
        this.emit('error', er)
        // If the error is handled, then we abort
        // if not, we threw out of here
        this.abort()
      }
      if (!this.silent)
        console.error('glob error', er)
      break

isaacs added a commit that referenced this issue Oct 7, 2016
Fix #245

This works around cases in Windows systems where we can see that a file
exists on the directory listing, but the stat/lstat call fails for some
reason other than ENOENT or ENOTDIR.
othiym23 added a commit to npm/npm that referenced this issue Nov 4, 2016
- Handle files without associated perms on Windows.
- Fix failing case with `absolute` option.

Credit: @isaacs
Credit: @phated
Fixes: isaacs/node-glob#245
Fixes: isaacs/node-glob#249
Reviewed-By: @othiym23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants