express-enrouten icon indicating copy to clipboard operation
express-enrouten copied to clipboard

Files with no extension are being processed by the directory traversal

Open lmarkus opened this issue 9 years ago • 2 comments

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).

lmarkus avatar Dec 29 '15 10:12 lmarkus

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?

jasisk avatar Jan 01 '16 19:01 jasisk

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.

lmarkus avatar Jan 02 '16 00:01 lmarkus