expressjs / serve-static

Serve static files
MIT License
1.39k stars 228 forks source link

Avoid redirect loop if no index is provided #18

Closed jjmerino closed 9 years ago

jjmerino commented 9 years ago

Problem The library falls into an infinite redirect loop when index option is set to false and the url matches a directory. How to reproduce Serve a folder using serve static and disable index: {index:false} , try to access any subdirectory. You will get an infinite redirect.Example: localhost/public/folder will turn into localhost/public/folder////////////////////.

Explanation When index option is set to false, the send library will, by default, send a 403 response if the requested resource is a directory. This will not happen if there are any on('redirect',...) listeners, as the send library will delegate to the listeners.

Unfortunately, the serve-static library handles the redirection but does not check to see if the resource ends with /. This results in the library redirecting to originalUrl+='/' over and over again. Producing an infinite redirect loop when index is set to false and the user requests a directory.

Proposed Solution The problem has been fixed by assuming a desired behaviour similar to that of the send library and the rest of serve-static, i.e, 403 for root directory, and 404 (ignore and forward) for subdirectories, to ensure non-intrusiveness when using as a middleware. 2 passing tests have been added to the specs for both cases.

dougwilson commented 9 years ago

Thank you so much for such a detailed bug report!!

dougwilson commented 9 years ago

I'm going to make a slight modification in that the root case should also next() instead of 403 to allow people to add directory handling afterwards; send does the 403 because it's the end-of-the-line type of deal, while this is simply a middleware :)

jjmerino commented 9 years ago

Thanks for the positive feedback! I agree 404 makes more sense. I didn't see that at the time. Cheers

dougwilson commented 9 years ago

So usually after the amount of code changes I made to your PR I would normally not keep your name as the commit author, but I'm just so thankful for the thoroughness of your report I'm keeping your name on the new commit I made :)

jjmerino commented 9 years ago

Thanks, I really appreciate that! :)

dougwilson commented 9 years ago

Published as 1.6.4 on npm :)