From f6054a7c408f36e955254c2e246a264ddf4b7d4c Mon Sep 17 00:00:00 2001 From: Matthew Davidson Date: Mon, 23 Jul 2018 00:45:50 -0400 Subject: [PATCH] Add URL-decoding support Enables matching URLs with %-encoded chars, and decodes path params. Currently Reitit cannot handle %-encoded paths, even though they can be quite common (like `/search/my random search string`) E.g.: Successfully match `/space in path` to `/space%20in%20path` Match `/path/:param1` to `/path/foo%20bar` and see `:param1` => `"foo bar"` in path params Does not apply %-decoding to routes, only URLs Does not guarantee decoding '+' into a space. (Java's URLDecoder supports it, but it shouldn't for URL path part.) --- modules/reitit-core/src/reitit/core.cljc | 4 +-- modules/reitit-core/src/reitit/impl.cljc | 25 +++++++++--------- modules/reitit-core/src/reitit/segment.cljc | 7 ++--- test/cljc/reitit/core_test.cljc | 29 ++++++++++++++++++++- 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/modules/reitit-core/src/reitit/core.cljc b/modules/reitit-core/src/reitit/core.cljc index 62a77c6d..0249a489 100644 --- a/modules/reitit-core/src/reitit/core.cljc +++ b/modules/reitit-core/src/reitit/core.cljc @@ -241,7 +241,7 @@ (route-names [_] names) (match-by-path [_ path] - (impl/fast-get data path)) + (impl/fast-get data (impl/url-decode path))) (match-by-name [_ name] (if-let [match (impl/fast-get lookup name)] (match nil))) @@ -321,7 +321,7 @@ (route-names [_] names) (match-by-path [_ path] - (if (#?(:clj .equals :cljs =) p path) + (if (#?(:clj .equals :cljs =) p (impl/url-decode 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 d5a41969..064b3a34 100644 --- a/modules/reitit-core/src/reitit/impl.cljc +++ b/modules/reitit-core/src/reitit/impl.cljc @@ -45,6 +45,17 @@ (defn contains-wilds? [path] (boolean (some wild-or-catch-all-param? (segments path)))) +(defn url-encode [s] + (some-> s + #?(:clj (URLEncoder/encode "UTF-8") + :cljs (js/encodeURIComponent)) + #?(:clj (.replace "+" "%20")))) + +(defn url-decode [s] + (some-> s #?(:clj (URLDecoder/decode "UTF-8") + :cljs (js/decodeURIComponent)))) + + ;; ;; https://github.com/pedestal/pedestal/blob/master/route/src/io/pedestal/http/route/path.clj ;; @@ -93,7 +104,7 @@ (let [{:keys [path-re path-params]} route] (fn [path] (when-let [m (re-matches path-re path)] - (zipmap path-params (rest m)))))) + (zipmap path-params (map url-decode (rest m))))))) ;; ;; Routing (c) Metosin @@ -108,7 +119,7 @@ (merge $ {:path path :matcher (if (contains-wilds? path) (path-matcher $) - #(if (#?(:clj .equals, :cljs =) path %) {})) + #(if (#?(:clj .equals, :cljs =) path (url-decode %)) {})) :result result :data data}) (dissoc $ :path-re :path-constraints) @@ -166,16 +177,6 @@ ;; Path-parameters, see https://github.com/metosin/reitit/issues/75 ;; -(defn url-encode [s] - (some-> s - #?(:clj (URLEncoder/encode "UTF-8") - :cljs (js/encodeURIComponent)) - #?(:clj (.replace "+" "%20")))) - -(defn url-decode [s] - (some-> s #?(:clj (URLDecoder/decode "UTF-8") - :cljs (js/decodeURIComponent)))) - (defprotocol IntoString (into-string [_])) diff --git a/modules/reitit-core/src/reitit/segment.cljc b/modules/reitit-core/src/reitit/segment.cljc index d1a1dda2..35cd9cad 100644 --- a/modules/reitit-core/src/reitit/segment.cljc +++ b/modules/reitit-core/src/reitit/segment.cljc @@ -39,9 +39,10 @@ (-lookup [_ [p & ps] path-params] (if (nil? p) (if 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)))))))) + (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))))))))) (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 19091509..7fc68cb8 100644 --- a/test/cljc/reitit/core_test.cljc +++ b/test/cljc/reitit/core_test.cljc @@ -51,6 +51,22 @@ #"^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"] + ["/two-param-path/:param1/:param2"] + ["/catchall/*remaining-path"] + ["/space in 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"))) + (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))))) + (testing "complex" (let [router (r/router [["/:abba" ::abba] @@ -100,7 +116,18 @@ #"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