Merge f1ade4b089 into 248200aad3

doc/ring/coercion.md

@@ -63,6 +63,8 @@ Handlers can access the coerced parameters via the `:parameters` key in the requ
                 {:status 200
                  :body {:total total}}))})
 ```
+
+
 ### Nested parameter definitions
 
 Parameters are accumulated recursively along the route tree, just like
@@ -92,26 +94,6 @@ handling for merging eg. malli `:map` schemas.
 ;         [:task-id :int]]}
 ```
 
-### Differences in behaviour for different parameters
-
-All parameter coercions *except* `:body`:
-
-1. Allow keys outside the schema (by opening up the schema using eg. `malli.util/open-schema`)
-2. Keywordize the keys (ie. header & query parameter names) of the input before coercing
-
-In contrast, the `:body` coercion:
-
-1. Uses the specified schema
-   * depending on the coercion, it can be configured as open or closed, see specific coercion docs for details
-2. Does not keywordize the keys of the input before coercion
-   * however, coercions like malli might do the keywordization when coercing json bodies, depending on configuration
-
-This admittedly confusing behaviour is retained currently due to
-backwards compatibility reasons. It can be configured by passing
-option `:reitit.coercion/parameter-coercion` to `reitit.ring/router`
-or `reitit.coercion/compile-request-coercers`. See also:
-`reitit.coercion/default-parameter-coercion`.
-
 ## Coercion Middleware
 
 Defining a coercion for a route data doesn't do anything, as it's just data. We have to attach some code to apply the actual coercion. We can use the middleware from `reitit.ring.coercion`:

modules/reitit-ring/src/reitit/ring.cljc

@@ -173,8 +173,7 @@
    (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" (let [path (str/replace-first path #"^/+" "/")] ; Locations starting with // redirect to another hostname. Avoid these due to security implications.
+                :headers {"Location" (if query-string (str path "?" query-string) path)}
-                                       (if query-string (str path "?" query-string) path))}
                 :body ""}))
            (redirect-handler [request]
              (let [uri (:uri request)]

test/cljc/reitit/ring_test.cljc

@@ -416,31 +416,7 @@
             :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)]