Skip to content
This repository has been archived by the owner on Dec 1, 2017. It is now read-only.

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

Open
KevinRausch opened this issue Jul 17, 2013 · 2 comments

Comments

@KevinRausch
Copy link

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
Copy link
Contributor

ozten commented Jul 17, 2013

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
Copy link
Contributor

ozten commented Jul 23, 2013

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants