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

IndexFromEmptyFile is a dangerous option #17

Closed
MartinLoeper opened this issue Dec 8, 2017 · 3 comments
Closed

IndexFromEmptyFile is a dangerous option #17

MartinLoeper opened this issue Dec 8, 2017 · 3 comments

Comments

@MartinLoeper
Copy link

I just started to use this nice library and integrated it into our nodejs backend which is intended to serve both - static and dynamic content.

There is one behaviour which I did not expect: API requests to our REST endpoint stopped working if their path was ending with "/".

The issue is, that indexFromEmptyFile is enabled by default and thus every request is manipulated if it is ending with "/". - Even if the serve-static middleware does not handle the request.
In our scenario, the next middleware - nestjs - cannot handle the request anymore as the path does not match.

@tkoenig89
Copy link
Owner

The reason this is enabled by default is, that you might have this serving a website staticly. You might not want the users to type the "index.html" at the end of your website's url. Which seemed to me like the most common case at the time. For all other cases it can be switched off.

Back to your issue:
I guess you might have a setup where static files might lie somewhere in "/**/*" and your dynamic request are served from "/api/**/*". In this case the simplest solution and the one i always use in this case is to put the static fileserver at the end of the url routing. This way your API endoints ending with "/" will not be effected by express-static-gzip.

Should be something like this:

let app = express();
app.use("/api", nestjsInstance);
//...
app.use("/", expressStaticGzip("static/content"));

If this does not solve your issue, you might want to just disable indexFromEmptyFile. If there are more voices to say, it should be turned off by default, this can be done in a future version.

@MartinLoeper
Copy link
Author

MartinLoeper commented Dec 8, 2017

Thank you for the detailed answer!
I already chose the option to disable indexFromEmptyFile which you mentioned because this is in fact easily done. It solves my issue.

However, I just wanted to make others know the issue which I encountered in case they have a similar setup. Maybe you could just add a warning somewhere in the first paragraph in the README, that expressStaticGzip does not only extend serveStatic by adding gzip compression (as I exprected), but might also cause regression if the api requests are not handled in a middleware on top (as your snippet above illustrates).

I think, the module would be even better if a developer could easily replace serveStatic calls with expressStaticGzip calls without breaking functionality :)

All in all, thank you for the fast and concise answer!

@tkoenig89
Copy link
Owner

Just did some modifications on the readme and made it more prominent what can happen with this option. Maybe this helps others :)

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