Skip to content

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

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

2 participants

@KevinRausch

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
Mozilla member
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
Mozilla member
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 join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.