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

glob does not return the path of a broken directory symlink, even when 'nonull:true' is set #170

Closed
fourcube opened this issue Mar 4, 2015 · 5 comments

Comments

@fourcube
Copy link

fourcube commented Mar 4, 2015

Version: glob 4.4.1
OS: Linux devbox 3.16.0-30-generic #40-Ubuntu SMP Mon Jan 12 22:06:37 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux

Steps to reproduce:

cd /tmp
mkdir foo
ln -s foo bar
rm -r foo

npm install glob

Run the following code:

glob('bar', {nonull:true}, function (er, res) { 
  console.log(er, res);
});

Result: null []

fourcube@devbox:/tmp$ stat bar
  File: ‘bar’ -> ‘foo’
  Size: 3           Blocks: 0          IO Block: 4096   symbolic link
Device: 801h/2049d  Inode: 65333       Links: 1
Access: (0777/lrwxrwxrwx)  Uid: ( 1000/fourcube)   Gid: ( 1000/fourcube)
Access: 2015-03-04 17:29:34.964707559 +0100
Modify: 2015-03-04 17:29:32.220707646 +0100
Change: 2015-03-04 17:29:32.220707646 +0100
 Birth: -
@fourcube
Copy link
Author

fourcube commented Mar 4, 2015

https://github.com/isaacs/node-glob/blob/master/glob.js#L642

Glob.prototype._stat2 = function (f, abs, er, stat, cb) {
  if (er) {
    this.statCache[abs] = false
    return cb()
  }

  this.statCache[abs] = stat

  if (abs.slice(-1) === '/' && !stat.isDirectory())
    return cb(null, false, stat)

  var c = stat.isDirectory() ? 'DIR' : 'FILE'
  this.cache[f] = this.cache[f] || c
  return cb(null, c, stat)
}

When a broken symlink is encountered, er will be ENOENT. Should the error and path be returned when nonull:true?

@isaacs
Copy link
Owner

isaacs commented Mar 4, 2015

Yeah, it should probably be doing an lstat to figure out existence, and then just try to readdir to see if it's a directory if it shows up as either a dir or symlink.

Good find! This is indeed a bug.

@isaacs
Copy link
Owner

isaacs commented Mar 4, 2015

I suspect that this is causing isaacs/rimraf#65 and ember-cli/ember-cli#3413.

@fourcube
Copy link
Author

fourcube commented Mar 4, 2015

@isaacs I stumbled over this while running broccoli serve with a dependency tree like plugin->less-processor->rimraf->glob. Since ember-cli's build pipeline is also based on broccoli, ember-cli/ember-cli#3413 might also be fixed by solving this one.

What happens during broccoli serve is this:

  1. A symlink is set on a temporary directory by X
  2. Processing works as normal
  3. ...
  4. Temporay directory is deleted
  5. The symlink still exists, but is reported as absent by glob
  6. rimraf does not delete the link
  7. X tries to create the link again -> EEXIST

@isaacs isaacs closed this as completed in 4fd4a48 Mar 5, 2015
isaacs added a commit to isaacs/rimraf that referenced this issue Mar 5, 2015
Regression introduced by pulling in glob as a dep, due to root cause
isaacs/node-glob#170.

Added good and bad symlinks to test, which would have caught this.

Fixes #65
Fixes ember-cli/ember-cli#3413
@fourcube
Copy link
Author

fourcube commented Mar 5, 2015

Thank you for fixing this issue so quickly!

cdelahousse added a commit to cdelahousse/repolinter that referenced this issue Sep 27, 2018
isaacs/node-glob#170 made it so that globs that match
broken symlinks are returned as file paths. This patch works around that issue
by catching exceptions thrown when opening files.

I opted to handle exceptions instead of filtering out broken symlinks in
fs.getAllFiles() because of the comment here: https://nodejs.org/api/fs.html#fs_fs_stat_path_options_callback

I changed the file content getters to return undefined explicitly when no file
content is returned. This is to be consistent with what's already there
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

2 participants