luciano/reitit
1
0
Fork 0
mirror of https://github.com/metosin/reitit.git synced 2026-01-24 15:19:01 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Merge pull request #776 from metosin/feat/337-fix-external-redirect

Browse source 
fix: redirect-trailing-slash-handler won't make external redirects
This commit is contained in:
Joel Kaasinen 2026-01-23 14:16:11 +02:00 committed by GitHub
commit 8391fafbe2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 27 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
modules/reitit-ring/src/reitit/ring.cljc
View file

@ -173,7 +173,8 @@
(letfn [(maybe-redirect [{:keys [query-string] :as request} path]
(if (and (seq path) (r/match-by-path (::r/router request) path))
{:status (if (= (:request-method request) :get) 301 308)
:headers {"Location" (if query-string (str path "?" query-string) path)}
:headers {"Location" (let [path (str/replace-first path #"^/+" "/")] ; Locations starting with // redirect to another hostname. Avoid these due to security implications.
(if query-string (str path "?" query-string) path))}
:body ""}))
(redirect-handler [request]
(let [uri (:uri request)]

26
test/cljc/reitit/ring_test.cljc
View file

@ -416,7 +416,31 @@
:get "/slash-less//" "/slash-less?kikka=kukka"
:post "/with-slash" "/with-slash/?kikka=kukka"
:post "/slash-less/" "/slash-less?kikka=kukka"
:post "/slash-less//" "/slash-less?kikka=kukka"))))))
:post "/slash-less//" "/slash-less?kikka=kukka"))))
;; See issue #337
(testing "Avoid external redirects"
(let [app (ring/ring-handler
(ring/router [["*" {:get (constantly nil)}]])
(ring/redirect-trailing-slash-handler))
resp (fn [uri & [query-string]]
(let [r (app {:request-method :get :uri uri :query-string query-string})]
{:status (:status r)
:Location (get-in r [:headers "Location"])}))]
(testing "without query params"
(is (= {:status 301 :Location "/malicious.com/foo/"} (resp "//malicious.com/foo")))
(is (= {:status 301 :Location "/malicious.com/foo"} (resp "//malicious.com/foo/")))
(is (= {:status 301 :Location "/malicious.com/foo"} (resp "//malicious.com/foo//")))
(is (= {:status 301 :Location "/malicious.com/foo/"} (resp "///malicious.com/foo")))
(is (= {:status 301 :Location "/malicious.com/foo"} (resp "///malicious.com/foo/")))
(is (= {:status 301 :Location "/malicious.com/foo"} (resp "///malicious.com/foo//"))))
(testing "with query params"
(is (= {:status 301 :Location "/malicious.com/foo/?bar=quux"} (resp "//malicious.com/foo" "bar=quux")))
(is (= {:status 301 :Location "/malicious.com/foo?bar=quux"} (resp "//malicious.com/foo/" "bar=quux")))
(is (= {:status 301 :Location "/malicious.com/foo?bar=quux"} (resp "//malicious.com/foo//" "bar=quux")))
(is (= {:status 301 :Location "/malicious.com/foo/?bar=quux"} (resp "///malicious.com/foo" "bar=quux")))
(is (= {:status 301 :Location "/malicious.com/foo?bar=quux"} (resp "///malicious.com/foo/" "bar=quux")))
(is (= {:status 301 :Location "/malicious.com/foo?bar=quux"} (resp "///malicious.com/foo//" "bar=quux"))))))))
(deftest async-ring-test
(let [promise #(let [value (atom ::nil)]