Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ring/create-file-handler doesn't accept URL-encoded filenames #484

Closed
aphyr opened this issue Mar 25, 2021 · 1 comment · Fixed by #489
Closed

ring/create-file-handler doesn't accept URL-encoded filenames #484

aphyr opened this issue Mar 25, 2021 · 1 comment · Fixed by #489
Labels

Comments

@aphyr
Copy link

aphyr commented Mar 25, 2021

Hi there! It looks like create-resource-handler and create-file-handler don't understand URL-escaped characters, so requests for files like foo%20bar.png will return a 404. Is it possible that I'm doing something wrong, or this is an intentional design choice? If not, would it be appropriate to URL-decode (:uri req) in the resource handler?

@miikka miikka added the bug label Apr 23, 2021
@miikka
Copy link
Contributor

miikka commented Apr 23, 2021

This seems to be a problem specifically when the handlers are mounted outside of the router. I tried this with the following code:

(ns example.server
  (:require [reitit.ring :as ring]
            [ring.adapter.jetty :as jetty]))

(def app
  (ring/ring-handler
    (ring/router
      [["/ping" (constantly {:status 200, :body "pong"})]
       ["/internal/*" (ring/create-file-handler)]])
    (ring/routes
     (ring/create-file-handler {:path "/external"})
     (ring/create-default-handler))))

(defn start []
  (jetty/run-jetty #'app {:port 3000, :join? false})
  (println "server running in port 3000"))

I created a file public/foo bar.txt. Now http://localhost:3000/internal/foo%20bar.txt works as expected but http://localhost:3000/external/foo%20bar.txt returns 404. I'd say that they both should work and URL-decoding (:uri req) sounds about right.

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

Successfully merging a pull request may close this issue.

2 participants