SendStream.pipe with empty path resolves to path with slash on end about send HOT 10 CLOSED

There is an open issues asking for using a file in the express.static repo: expressjs/serve-static#89 but since I had to rush out the Express 4.16 release due to a security issue, I was not able to get that into the 4.16 release. If you are using Express and just want to send a specific file to whatever route, you would use res.sendFile:

// This should be equivalent to what you're doing above
var path = require('path')
app.use(function (req, res) {
  res.sendFile(path.resolve('client.html'))
})

from send.

Yea, that is how we are working around https://nodejs.org/en/blog/vulnerability/september-2017-path-validation/

Can provide some information around what is breaking / your use-case? In the end, the underlying path is slightly different but should have the same meaning. We can figure out a different method of protection if necessary.

from send.

From Express, we wire up several speciality routes, and then use a wildcard match for the last one to return a static html file that behaves like a single page app. Here's a small snippet extended from the Express guide which highlights the issue.

const express = require('express')
const app = express()

app.get('/', function (req, res) {
    res.send('Hello World!')
})

app.use('/*', express.static('client.html'))

app.listen(3000, function () {
    console.log('Example app listening on port 3000!')
})

On prior versions, you could navigate to http://localhost:3000/my/path and have it route appropriately, but on the most recent version of Express/send-static/send, this results in an error. If I log out the resolved path, it is "[project directory]/client.html/".

from send.

Thanks, @adamldoyle . Is client.html a directory or a filename? The express.static takes a directory as it's argument.

from send.

Looks like we've been passing in a filename and for awhile now it has just kind of worked, interesting. I suppose that explains the slash getting added to the end. Is there a better way to route all routes to a single file like this?

from send.

We'll make the change and keep and eye on that issue. Thanks!

from send.

Even though it's not supported, I'm going to wait a bit and if there are others in the same boat as you, we'll probably fix it (though fix it enough to barely work -- I would suggest changing to use res.sendFile at least until the static-static feature is realized). I searched on Google the express.static usage you showed and it seems like people may be doing that, probably I guess someone found that it just happens to work and now it's made it's way around I guess.

It is pretty exhausting to test changes to the path protection, since it involves rigorously testing the new change on every single minor version of Node.js since 0.8.0, to make sure that nothing is wrong (because the alternative is an attacked can read any file accessible to Node.js, which is what happened with Node.js 8.5.0 and caused the slight behavior change).

I will poke at what you provided as well to see what may be a better way to resolve, but it took 3 days to come up with that change we have now, so another change may be days away :(

from send.

When I was messing around to see if I could get something to work, I was able to get around the issue by only normalizing the path with './' if the path wasn't the empty string. I don't know if this re-exposes the issue or only solves my very specific use case, but it worked.

from send.

Cool, I will take a look to see how that changes things. I can potentially see skipping all the normalization if (!path) since it's kind of pointless any ways. I just wrote up expressjs/express#3436 regarding this if you want to take a read through. Since you already changed the API usage, it doesn't quite matter now, but just wanted to let you know this wasn't intentional and we didn't know this was even a possible usage, and apologize for any time this consumed on your end.

from send.

Since I am fixing this, I'm going to re-open to track :)

from send.