Decode %-encoded URL path params

Also adds utility fn map-kv which is convenient for en/decoding
both maps (like path-params) and vectors (like path parts)

Converts path-params fn to use map-kv

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)

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
+  (map-kv #(url-encode (into-string %)) params))
-    (fn [m k v]
-      (assoc m k (url-encode (into-string v))))
-    {}
-    params))
 
 (defn query-string
   "shallow transform of query parameters into query string"

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))
+           (when 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)))))))))
+               (-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})))

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"
+        (testing "decode %-encoded path params"
-          (let [router (r/router [["/one-param-path/:param"]
+          (let [router (r/router [["/one-param-path/:param1"]
                                   ["/two-param-path/:param1/:param2"]
-                                  ["/catchall/*remaining-path"]
+                                  ["/catchall/*remaining-path"]] {:router r})
-                                  ["/space in path"]] {:router r})
                 decoded-params #(-> router (r/match-by-path %) :path-params)
-                decoded-param #(-> (decoded-params %) :param)
+                decoded-param1 #(-> (decoded-params %) :param1)
-                decoded-catchall #(-> (decoded-params %) :remaining-path)]
+                decoded-remaining-path #(-> (decoded-params %) :remaining-path)]
-            (println "Testing" (r/router-name router))
+            (is (= "foo bar" (decoded-param1 "/one-param-path/foo%20bar")))
-            (is (= "foo bar" (decoded-param "/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-remaining-path "/catchall/foo%20bar")))
-            (is (= "foo bar" (decoded-catchall "/catchall/foo%20bar")))
+            (is (= "!#$&'()*+,/:;=?@[]"
-            (is (= "!#$&'()*+,/:;=?@[]" (decoded-param "/one-param-path/%21%23%24%26%27%28%29%2A%2B%2C%2F%3A%3B%3D%3F%40%5B%5D")))
+                   (decoded-param1 "/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)))))
 
         (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"] {})))))
+                    ["/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")))))))
 
       r/lookup-router :lookup-router
       r/single-static-path-router :single-static-path-router