search

krakenjs / express-enrouten

An express route initialization and configuration module.
Other
172 stars 38 forks source link

Files with no extension are being processed by the directory traversal #73

Open lmarkus opened 8 years ago

lmarkus commented 8 years ago

Referenced from https://github.com/krakenjs/kraken-js/issues/440

The problem is on the express-enrouten module, specifically here: https://github.com/krakenjs/express-enrouten/blob/v1.2.x-release/lib/directory.js#L101-L102

In order to support requireable non-traditional file extensions, (eg: thing.coffee), the file extension is thrown away, and the file is checked using require.resolve().

There are three scenarios here:

  1. controllerFile.js: Extension thrown out, /path/to/controllerFile resolves as a module, since it can be matched to controllerFile.js. It will be processed.
  2. textFile.txt: Extension thrown out, path/to/textFile This does not resolve as a module, since require() can't match it to textFile, textFile.js, textFile.json nor a directory with an index.js file. It will be ignored.
  3. fileWithNoExtension: /path/to/fileWithNoExtension resolves as a module, because it can be matched to a valid existing file (just like the first case). It will be processed.

Case 3 causes issues with SVN-like version control systems, since CVS stores extensionless files in its hidden directories (eg: Entries, Repository, Root).

jasisk commented 8 years ago

Hmm. I'm not so sure we should be taking on this responsibility—I think it should land on the app developer.

We'll be suppressing the error case of a documented behavior. "If I can resolve the file, I load it."

Imagine the common case: a bug in a controller. With this PR, it just won't be loaded. The dev will be none the wiser.

Rather than take on that responsibility, perhaps we should add blacklisting?

lmarkus commented 8 years ago

Good call on the buggy controller case. I just added debug statements, but I can see how that makes for a less enjoyable dev experience.

I started going down the configuration path, but this seemed like a simpler choice. I have a half-baked branch with configurable ignore lists that I can wrap up on Monday.