This repository has been archived by the owner on Jan 25, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 38
Files with no extension are being processed by the directory traversal #73
Comments
lmarkus
pushed a commit
to lmarkus/express-enrouten
that referenced
this issue
Dec 31, 2015
Instead of just trying to resolve the module, try to load it. Since this block is already surrounded by a try/catch statement it's a safe place to attempt requiring a module. I don't think it's wasted effort, since the whole point of validating the modules is to try to load them later. Also, some very minor refactoring of the extension removal code to make it more readable.
lmarkus
pushed a commit
to lmarkus/express-enrouten
that referenced
this issue
Dec 31, 2015
Instead of just trying to resolve the module, try to load it. Since this block is already surrounded by a try/catch statement it's a safe place to attempt requiring a module. I don't think it's wasted effort, since the whole point of validating the modules is to try to load them later. Also, some very minor refactoring of the extension removal code to make it more readable.
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? |
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
pushed a commit
to lmarkus/express-enrouten
that referenced
this issue
Jan 2, 2016
Fixes krakenjs#73 This introduces a change to the directory configuration (It's now an {path:"path/to/controllers", ignore:['pattern1',...]} object instead of a path string. However, I made it backwards compatible. It shows a deprecation message, but we could support both methods if desired
lmarkus
pushed a commit
to lmarkus/express-enrouten
that referenced
this issue
Jan 2, 2016
Fixes krakenjs#73 This introduces a change to the directory configuration (It's now an {path:"path/to/controllers", ignore:['pattern1',...]} object instead of a path string. However, I made it backwards compatible. It shows a deprecation message, but we could support both methods if desired
lmarkus
pushed a commit
to lmarkus/express-enrouten
that referenced
this issue
Jan 2, 2016
Fixes krakenjs#73 This introduces a change to the directory configuration (It's now an {path:"path/to/controllers", ignore:['pattern1',...]} object instead of a path string. However, I made it backwards compatible. It shows a deprecation message, but we could support both methods if desired
lmarkus
pushed a commit
to lmarkus/express-enrouten
that referenced
this issue
Jan 2, 2016
Fixes krakenjs#73 This introduces a change to the directory configuration (It's now an {path:"path/to/controllers", ignore:['pattern1',...]} object instead of a path string. However, I made it backwards compatible. It shows a deprecation message, but we could support both methods if desired
Closed
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Referenced from krakenjs/kraken-js#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
require
able non-traditional file extensions, (eg:thing.coffee
), the file extension is thrown away, and the file is checked usingrequire.resolve()
.There are three scenarios here:
controllerFile.js
: Extension thrown out,/path/to/controllerFile
resolves as a module, since it can be matched tocontrollerFile.js
. It will be processed.textFile.txt
: Extension thrown out,path/to/textFile
This does not resolve as a module, sincerequire()
can't match it totextFile
,textFile.js
,textFile.json
nor a directory with anindex.js file
. It will be ignored.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
).The text was updated successfully, but these errors were encountered: