expressjs / serve-static

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

Add staticPath option #72

Closed purplestone closed 8 years ago

purplestone commented 8 years ago

Add allowPost option Add staticPath option

dougwilson commented 8 years ago

Hi! Thanks for your pull request!

This feature is reintroducing the "root path disclosure" bug, and unfortunately I cannot accept the PR in the current state :(

dougwilson commented 8 years ago

If you can please open two different pull requests for each feature you are introducing, that will be the easiest way to at least get one in and makes the discussion a lot easier :)!

purplestone commented 8 years ago

Is there test case for the "root path disclosure" bug?

dougwilson commented 8 years ago

Is there test case for the "root path disclosure" bug?

Yep, there are multiple tests,but of course they don't cover your new feature since it didn't exist at the time.

The issue is that you cannot let people use ".." characters to discover what root paths are in use. In this PR, the value of staticPath can be uncovered by specially-crafted requests.

In addition, I don't even understand what the purpose of your staticPath feature is. Your docs for it in the README describe something that already exists, so it sounds like you're just adding a 2nd way to do something, rather than anything new to me. If you can provide full explanations for the features you are adding that explain why they need to be adds and their use cases, that would help a lot. Please do that in the new PRs, not this closed one, so other people can follow along :)

purplestone commented 8 years ago

I did not find a way that get the path from a url prefix . for example I visit url : /static/(**/*) should to get files : public/resource/(**/*) How to write this code?

dougwilson commented 8 years ago

Assuming I am understanding you correctly:

app.use('/static', serveStatic('public/resource/'))

Because this middleware, like all middleware, operate off the mounted path of the middleware.

purplestone commented 8 years ago

Oh,yeah, It Works if I use Expressjs, thank you. How to do by require('http')?

dougwilson commented 8 years ago

So this module is a middleware, and that really means any feature you want in a middleware you would have to re-create yourself if using require("http") directly without other module like the "router" module.

Based on your comment on your PR, I think you would be a lot better off simply using the "send" module directly, as this module's purpose is simply to wrap the "send" module into a middleware, and if you are using require("http"), then a middleware is probably not what you want to use.

I'm not at a computer right now, so cannot write any code example, but I'm going to reply on your PR pretty much that your use-case can very easily be solved using the "send" module, and by using this module in the way you are trying is probably why you are having issues :) I'll try to reply in the next 5 hours or such :)

purplestone commented 8 years ago

Yes, I had used Expressjs, It works good. Thank you.