search

edgecase / dieter

Asset pipeline ring middleware
134 stars 22 forks source link

alter :path-info as well as :uri #59

Closed kbaribeau closed 10 years ago

kbaribeau commented 10 years ago

Hey all!

I'm just spinning up a new compojure app and dieter seems like the right lib for me to handle less compilation. However, I noticed that when deploying to immutant dieter wasn't playing nice. So after lots of digging I discovered that the :path-info key in a request is actually preferred over :uri by ring when serving static content. So I wrote a quick patch.

As an aside, issue #48 has also been a problem for me. If you guys are accepting contributions I might be tempted to try a fix for that as well.

I threw some more detail in the commit message, see below.

When changing the path of a request, be sure to change :path-info as well as :uri.

Ring actually looks at :path-info first. See https://github.com/ring-clojure/ring/blob/1.2/ring-core/src/ring/middleware/file.clj#L21 and https://github.com/ring-clojure/ring/blob/1.2/ring-core/src/ring/util/request.clj#L32-L36

So, when we're running on a server that supplies :path-info, we have to be sure it is correct, or else we'll end up either 404'ing (bad) or serving the unaltered content (possibly worse).

jxa commented 10 years ago

Need to revert this one until #60 is fixed. Or you can rebase 3879f91 onto master so it doesn't need 309912e and I'll merge it.

kbaribeau commented 10 years ago

Hey @jxa, I don't think I can edit the branch inside a PR, but I've pushed up a branch on my work called path-fix (https://github.com/kbaribeau/dieter/tree/path-fix) -- it should merge cleanly onto master here and pass all tests.