expressjs / serve-static

Serve static files
MIT License
1.38k stars 227 forks source link

Add option to remove trailing slashes, as opposed to adding them #132

Closed fabiosantoscode closed 4 years ago

fabiosantoscode commented 4 years ago

Hello!

We're serving a website through Gatsby, which is having some trouble removing trailing slashes when serving an index.html file.

As you can see from the PR and discussion, it seems to be impossible through configuration options to remove the trailing slash, even if it's possible to stop from redirecting to a trailing slash.

Would you be interested in a PR which adds (or changes) an option to remove trailing slashes when serving index files?

Thank you very much :)

dougwilson commented 4 years ago

Hi @fabiosantoscode . I wonder what the problem is, first. Can you provide a reproduction case that we can debug through to see the issue you are having? Just modify one of the examples in https://github.com/expressjs/serve-static#examples and provide the modified code here and the steps to see what is happening and then what you expect to happen instead. Thanks!

fabiosantoscode commented 4 years ago

@dougwilson certainly!

I've just noticed, by trying to reproduce this myself, that this statement was incorrect:

even if it's possible to stop from redirecting to a trailing slash

However, the main issue still holds.

I'm going to paste my code and describe how I built the "public" directory, but let me know if you'd like me to create a gist or a github repository.

var express = require('express')           
var serveStatic = require('serve-static')  

var app = express()                        

app.use(serveStatic('public', { redirect: false }))             
app.listen(4444)                           

This is what's in the public folder (the index.html file is just an empty file, since it doesn't matter what it contains)

public/
└── foo
    └── index.html

To reproduce the issue I'm describing, I've used curl:

(venv) fabio@fabio-thinkpad ♥  curl -I localhost:4444/foo
HTTP/1.1 404 Not Found
[...other headers]

(curl -I localhost:4444/foo/ yields a 200 OK response)

I would like a way to serve /foo here by reading whatever is in its index.html file. And also for /foo/ to be redirected to just /foo. This might be two options, or one option. Either way I'm happy to send a PR.

Thanks for the speedy reply!

dougwilson commented 4 years ago

Ah, I see. I don't think that's going to work, though, as all the relative links on the index.html would not work in that case, right?

dougwilson commented 4 years ago

Any reason why not to just do public/foo.html to achieve the same thing? Then the file would be logically in the proper relative directly to which it would be served out it. The https://github.com/expressjs/serve-static#extensions option would make that work for the url /foo

fabiosantoscode commented 4 years ago

public/foo.html would certainly achieve the same thing. I didn't think of that at all. I'm not sure if that's a good option for the Gatsby project, but I'll check.

The problem with links is mostly benign, I think. I haven't seen a relative link in the wild in years. Either way, if you're setting an option to remove trailing slashes you should understand the consequences.

dougwilson commented 4 years ago

The problem with links is mostly benign, I think. I haven't seen a relative link in the wild in years. Either way, if you're setting an option to remove trailing slashes you should understand the consequences.

You'd be surprised :) I am constantly helping new web programmers with little to no understanding of any of this, and would expect it to look the same way as when they double-click on the file on their file system.

I don't think having the option you propose would be valid for this project now that I better understand it, I'm sorry. You're welcome to use a different project or similar if that is really what you want to do without having a file system layout that matches the files served. But in reality, files should never be served up by Node.js unless as a last resort, so you should be using Apache/NGINX/etc. I'm not sure if they have those options you're looking for, either, but if they do, that would probably be the best overall solution.

fabiosantoscode commented 4 years ago

Thank you!

formulahunter commented 3 years ago

Hi @dougwilson, random question for you... I've recently started working on a Nginx/Nodejs stack and read part of your comment above:

...files should never be served up by Node.js unless as a last resort...

I've suspected this to be true for a while but don't yet fully understand the rationale. Do you mind explaining the reasoning behind this?

dougwilson commented 3 years ago

Hi @formulahunter this is a general Node.js question I would suggest asking on Node.js itself to get the full answer. The short answer is that serving files will consume your worker pool (https://nodejs.org/en/docs/guides/dont-block-the-event-loop/).