Skip to content
This repository has been archived by the owner on Jan 25, 2020. It is now read-only.

Ignore invalid files with no extension #74

Closed
wants to merge 3 commits into from

Conversation

lmarkus
Copy link
Contributor

@lmarkus lmarkus commented Dec 31, 2015

Proposed fix for #73

Instead of just trying to resolve a module during the isFileModule verification step, 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 added 3 commits December 30, 2015 16:25
This commit simply adds a file with no extension and some content to the "Superfluous" fixtures directory.
Such a file is a perfectly valid use case. For example, SVN-like versioning systems populate a project with
hidden folders that include such files. The pattern is: A non-empty file without an extension.

These types of files will currently cause express enrouten to fail, as it will incorrectly identify them as
a valid node module, and will try to load them.
This passes today, but I feel it's good to have it around as a safety net.
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.
@@ -0,0 +1 @@
Klaatu barada nikto
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gort would approve!

@grawk
Copy link
Member

grawk commented Dec 31, 2015

LGTM 👍

@jasisk jasisk closed this Jan 29, 2016
@jasisk
Copy link
Contributor

jasisk commented Jan 29, 2016

Closed due to branch renaming. Will reconstruct, reopen, and notify you when complete.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants