mozilla / connect-cachify

Express connect middleware to provide easy frontend caching for Node.js
Mozilla Public License 2.0
131 stars 21 forks source link

output prefix should only be used in hashify function, not in middleware #35

Open KevinRausch opened 11 years ago

KevinRausch commented 11 years ago

The prefix option should only be used in creating output strings in the hashify function. The job of 'path' parsing should be left to routing logic outside the module or to app.use([path],middleware()). Inside the middleware function, there should be no requirement that the path be present in the request url.

For example, my main app handles the path '/' and I have a subapp mounted at '/tools'. In this subapp, urls are cachified at '/tools/[hash]/file.ext'. My routing logic, due to the mounted subapp, strips the /tools portion of the url from incoming requests, so the cachify module only sees '/[hash]/file.ext'. At this point, the module will not see the specified prefix in the url and will not process the request.

ozten commented 11 years ago

Good point.

I wonder if we could do something like this:

app.get('/tool/:cachify/file.ext', function(req, res) {
    ...

The middleware would be updated to look for req.params.cachify. If the middleware sees this path parameter, it will use that, otherwise it defaults to the current behavior.

Another improvement would be to validate that the hash is actually hash like before updating the path.

I haven't had coffee or tested this, but it's an idea.

ozten commented 11 years ago

Okay, I added a unit test ba6275d0101c6d559c670e7e5d6a2b98892b7574 to make sure /tool wouldn't be stripped in the current codebase.