diff --git a/modules/reitit-core/src/reitit/core.cljc b/modules/reitit-core/src/reitit/core.cljc index d3f0d8e7..70a5680a 100644 --- a/modules/reitit-core/src/reitit/core.cljc +++ b/modules/reitit-core/src/reitit/core.cljc @@ -209,7 +209,7 @@ (reduce (fn [_ ^Route route] (if-let [path-params ((:matcher route) path)] - (reduced (->Match (:path route) (:data route) (:result route) path-params path)))) + (reduced (->Match (:path route) (:data route) (:result route) (impl/url-decode-coll path-params) path)))) nil pl)) (match-by-name [_ name] (if-let [match (impl/fast-get lookup name)] @@ -263,7 +263,7 @@ (route-names [_] names) (match-by-path [_ path] - (impl/fast-get data (impl/url-decode path))) + (impl/fast-get data path)) (match-by-name [_ name] (if-let [match (impl/fast-get lookup name)] (match nil))) @@ -314,7 +314,7 @@ (match-by-path [_ path] (if-let [match (segment/lookup pl path)] (-> (:data match) - (assoc :path-params (:path-params match)) + (assoc :path-params (impl/url-decode-coll (:path-params match))) (assoc :path path)))) (match-by-name [_ name] (if-let [match (impl/fast-get lookup name)] @@ -352,7 +352,7 @@ (route-names [_] names) (match-by-path [_ path] - (if (#?(:clj .equals :cljs =) p (impl/url-decode path)) + (if (#?(:clj .equals :cljs =) p path) match)) (match-by-name [_ name] (if (= n name) diff --git a/modules/reitit-core/src/reitit/impl.cljc b/modules/reitit-core/src/reitit/impl.cljc index ea29670f..6f954807 100644 --- a/modules/reitit-core/src/reitit/impl.cljc +++ b/modules/reitit-core/src/reitit/impl.cljc @@ -19,6 +19,17 @@ (java.util HashMap Map) (java.net URLEncoder URLDecoder)))) +(defn map-kv + "Applies a function to every value of a map. + + Also works on vectors. Maintains key for maps, order for vectors." + [f coll] + (reduce-kv + (fn [m k v] + (assoc m k (f v))) + (empty coll) + coll)) + (defn wild? [s] (contains? #{\: \*} (first (str s)))) @@ -93,7 +104,7 @@ (let [{:keys [path-re path-params]} route] (fn [path] (when-let [m (re-matches path-re path)] - (zipmap path-params (map url-decode (rest m))))))) + (zipmap path-params (rest m)))))) ;; ;; Routing (c) Metosin @@ -108,7 +119,7 @@ (merge $ {:path path :matcher (if (contains-wilds? path) (path-matcher $) - #(if (#?(:clj .equals, :cljs =) path (url-decode %)) {})) + #(if (#?(:clj .equals, :cljs =) path %) {})) :result result :data data}) (dissoc $ :path-re :path-constraints) @@ -203,6 +214,11 @@ s) :cljs (js/decodeURIComponent (str/replace s "+" " "))))) +(defn url-decode-coll + "URL-decodes maps and vectors" + [coll] + (map-kv url-decode coll)) + (defprotocol IntoString (into-string [_])) @@ -233,13 +249,9 @@ (into-string [_])) (defn path-params - "shallow transform of the path parameters values into strings" + "Convert parameters' values into URL-encoded strings, suitable for URL paths" [params] - (reduce-kv - (fn [m k v] - (assoc m k (url-encode (into-string v)))) - {} - params)) + (map-kv #(url-encode (into-string %)) params)) (defn query-string "shallow transform of query parameters into query string" diff --git a/modules/reitit-core/src/reitit/segment.cljc b/modules/reitit-core/src/reitit/segment.cljc index 35cd9cad..929cf28b 100644 --- a/modules/reitit-core/src/reitit/segment.cljc +++ b/modules/reitit-core/src/reitit/segment.cljc @@ -38,11 +38,10 @@ (segment children wilds catch-all match)))) (-lookup [_ [p & ps] path-params] (if (nil? p) - (if match (assoc match :path-params path-params)) - (let [p (impl/url-decode p)] - (or (-lookup (impl/fast-get children' p) ps path-params) - (some #(-lookup (impl/fast-get children' %) ps (assoc path-params % p)) wilds) - (-catch-all children' catch-all path-params p ps))))))))) + (when match (assoc match :path-params path-params)) + (or (-lookup (impl/fast-get children' p) ps path-params) + (some #(-lookup (impl/fast-get children' %) ps (assoc path-params % p)) wilds) + (-catch-all children' catch-all path-params p ps)))))))) (defn insert [root path data] (-insert (or root (segment)) (impl/segments path) (map->Match {:data data}))) diff --git a/test/cljc/reitit/core_test.cljc b/test/cljc/reitit/core_test.cljc index 7817d365..8a7e3482 100644 --- a/test/cljc/reitit/core_test.cljc +++ b/test/cljc/reitit/core_test.cljc @@ -51,21 +51,18 @@ #"^missing path-params for route /api/ipa/:size -> \#\{:size\}$" (r/match-by-name! router ::beer)))))) - (testing "URL-encoded" - (let [router (r/router [["/one-param-path/:param"] + (testing "decode %-encoded path params" + (let [router (r/router [["/one-param-path/:param1"] ["/two-param-path/:param1/:param2"] - ["/catchall/*remaining-path"] - ["/space in path"]] {:router r}) + ["/catchall/*remaining-path"]] {:router r}) decoded-params #(-> router (r/match-by-path %) :path-params) - decoded-param #(-> (decoded-params %) :param) - decoded-catchall #(-> (decoded-params %) :remaining-path)] - (println "Testing" (r/router-name router)) - (is (= "foo bar" (decoded-param "/one-param-path/foo%20bar"))) + decoded-param1 #(-> (decoded-params %) :param1) + decoded-remaining-path #(-> (decoded-params %) :remaining-path)] + (is (= "foo bar" (decoded-param1 "/one-param-path/foo%20bar"))) (is (= {:param1 "foo bar" :param2 "baz qux"} (decoded-params "/two-param-path/foo%20bar/baz%20qux"))) - (is (= "foo bar" (decoded-catchall "/catchall/foo%20bar"))) - (is (= "foo bar" (decoded-catchall "/catchall/foo%20bar"))) - (is (= "!#$&'()*+,/:;=?@[]" (decoded-param "/one-param-path/%21%23%24%26%27%28%29%2A%2B%2C%2F%3A%3B%3D%3F%40%5B%5D"))) - (is (= "/space in path" (-> router (r/match-by-path "/space%20in%20path") :template))))) + (is (= "foo bar" (decoded-remaining-path "/catchall/foo%20bar"))) + (is (= "!#$&'()*+,/:;=?@[]" + (decoded-param1 "/one-param-path/%21%23%24%26%27%28%29%2A%2B%2C%2F%3A%3B%3D%3F%40%5B%5D"))))) (testing "complex" (let [router (r/router @@ -116,18 +113,7 @@ #"can't create :lookup-router with wildcard routes" (r/lookup-router (r/resolve-routes - ["/api/:version/ping"] {}))))) - - (testing "URL-decoding" - (let [router (r/router [["/space in path"]] {:router r}) - matched-template #(-> router (r/match-by-path %) :template)] - (is (= "/space in path" (matched-template "/space%20in%20path")))) - - (testing "should only apply to real URLs, not configured routes" - (let [router (r/router [["/percent%20in%20path"]] {:router r}) - matched-template #(-> router (r/match-by-path %) :template)] - (is (= "/percent%20in%20path" (matched-template "/percent%2520in%2520path"))) - (is (not= "/percent%20in%20path" (matched-template "/percent%20in%20path"))))))) + ["/api/:version/ping"] {})))))) r/lookup-router :lookup-router r/single-static-path-router :single-static-path-router