luciano/reitit
1
0
Fork 0
mirror of https://github.com/metosin/reitit.git synced 2026-02-10 21:35:17 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: redirect-trailing-slash-handler won't make external redirects

Browse source 
A redirect header like

Location: //malicious.com/foo

Would result in a redirect to https://malicious.com/foo . We never
want Locations like that out of redirect-trailing-slash-handler.

Fixes #337
This commit is contained in:
Joel Kaasinen 2026-01-23 09:54:26 +02:00
parent 248200aad3
commit 71a777b4fa
No known key found for this signature in database
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] (letfn [(maybe-redirect [{:keys [query-string] :as request} path]
(if (and (seq path) (r/match-by-path (::r/router request) path)) (if (and (seq path) (r/match-by-path (::r/router request) path))
{:status (if (= (:request-method request) :get) 301 308) {: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 ""})) :body ""}))
(redirect-handler [request] (redirect-handler [request]
(let [uri (:uri 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" :get "/slash-less//" "/slash-less?kikka=kukka"
:post "/with-slash" "/with-slash/?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")))))) :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 (deftest async-ring-test
(let [promise #(let [value (atom ::nil)] (let [promise #(let [value (atom ::nil)]