From 4933927fa9c99571813210802842b13bd4c67971 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Wed, 22 May 2019 16:26:06 +0300 Subject: [PATCH 1/4] Add a failing test --- test/cljc/reitit/ring_test.cljc | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/test/cljc/reitit/ring_test.cljc b/test/cljc/reitit/ring_test.cljc index b23d41b7..282bbc3b 100644 --- a/test/cljc/reitit/ring_test.cljc +++ b/test/cljc/reitit/ring_test.cljc @@ -3,7 +3,8 @@ [clojure.set :as set] [reitit.middleware :as middleware] [reitit.ring :as ring] - [reitit.core :as r]) + [reitit.core :as r] + [reitit.trie :as trie]) #?(:clj (:import (clojure.lang ExceptionInfo)))) @@ -577,3 +578,19 @@ (fn [{:keys [::r/router]} _ _] (is router))) {} ::respond ::raise))) + +#?(:clj + (deftest invalid-path-parameters-parsing-concurrent-requests-277-test + (testing "in enought concurrent system, path-parameters can bleed" + (doseq [compiler [trie/java-trie-compiler trie/clojure-trie-compiler]] + (let [app (ring/ring-handler + (ring/router + ["/:id" (fn [request] + {:status 200 + :body (-> request :path-params :id)})]) + {::trie/trie-compiler compiler})] + (dotimes [_ 10] + (future + (dotimes [n 100000] + (let [body (:body (app {:request-method :get, :uri (str "/" n)}))] + (is (= body (str n)))))))))))) From 60ee39bd53e21842b8d68fab2f1e88251f13bf3a Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Wed, 22 May 2019 16:39:37 +0300 Subject: [PATCH 2/4] Trie$Match is mutable, fixes #277 --- modules/reitit-core/java-src/reitit/Trie.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/modules/reitit-core/java-src/reitit/Trie.java b/modules/reitit-core/java-src/reitit/Trie.java index b7af4c1f..6a7b6881 100644 --- a/modules/reitit-core/java-src/reitit/Trie.java +++ b/modules/reitit-core/java-src/reitit/Trie.java @@ -113,16 +113,19 @@ public class Trie { } static final class DataMatcher implements Matcher { - private final Match match; + + private final IPersistentMap params; + private final Object data; DataMatcher(IPersistentMap params, Object data) { - this.match = new Match(params, data); + this.params = params; + this.data = data; } @Override public Match match(int i, int max, char[] path) { if (i == max) { - return match; + return new Match(params, data); } return null; } @@ -139,7 +142,7 @@ public class Trie { @Override public String toString() { - return (match.data != null ? match.data.toString() : "nil"); + return (data != null ? data.toString() : "nil"); } } From 4178acde5f1728a6bfba3be1281970e2808e434e Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Wed, 22 May 2019 21:10:51 +0300 Subject: [PATCH 3/4] Make Trie$Match immutable --- modules/reitit-core/java-src/reitit/Trie.java | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/modules/reitit-core/java-src/reitit/Trie.java b/modules/reitit-core/java-src/reitit/Trie.java index 6a7b6881..dcfecd0e 100644 --- a/modules/reitit-core/java-src/reitit/Trie.java +++ b/modules/reitit-core/java-src/reitit/Trie.java @@ -38,8 +38,8 @@ public class Trie { return decode(new String(chars, begin, end - begin), hasPercent, hasPlus); } - public static class Match { - public IPersistentMap params; + public final static class Match { + public final IPersistentMap params; public final Object data; public Match(IPersistentMap params, Object data) { @@ -47,6 +47,10 @@ public class Trie { this.data = data; } + Match assoc(Object key, Object value) { + return new Match(params.assoc(key, value), data); + } + @Override public String toString() { Map m = new HashMap<>(); @@ -113,19 +117,16 @@ public class Trie { } static final class DataMatcher implements Matcher { - - private final IPersistentMap params; - private final Object data; + private final Match match; DataMatcher(IPersistentMap params, Object data) { - this.params = params; - this.data = data; + this.match = new Match(params, data); } @Override public Match match(int i, int max, char[] path) { if (i == max) { - return new Match(params, data); + return match; } return null; } @@ -142,7 +143,7 @@ public class Trie { @Override public String toString() { - return (data != null ? data.toString() : "nil"); + return (match.data != null ? match.data.toString() : "nil"); } } @@ -177,10 +178,7 @@ public class Trie { } } final Match m = child.match(stop, max, path); - if (m != null) { - m.params = m.params.assoc(key, decode(new String(path, i, stop - i), hasPercent, hasPlus)); - } - return m; + return m != null ? m.assoc(key, decode(new String(path, i, stop - i), hasPercent, hasPlus)) : null; } return null; } @@ -219,7 +217,7 @@ public class Trie { @Override public Match match(int i, int max, char[] path) { if (i <= max) { - return new Match(params.assoc(parameter, decode(path, i, max)), data); + return new Match(params, data).assoc(parameter, decode(path, i, max)); } return null; } From 38d419a82bbc2a73c145f46698d6511315064be4 Mon Sep 17 00:00:00 2001 From: Tommi Reiman Date: Wed, 22 May 2019 21:12:13 +0300 Subject: [PATCH 4/4] Fix typo --- test/cljc/reitit/ring_test.cljc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cljc/reitit/ring_test.cljc b/test/cljc/reitit/ring_test.cljc index 282bbc3b..a907fcc4 100644 --- a/test/cljc/reitit/ring_test.cljc +++ b/test/cljc/reitit/ring_test.cljc @@ -581,7 +581,7 @@ #?(:clj (deftest invalid-path-parameters-parsing-concurrent-requests-277-test - (testing "in enought concurrent system, path-parameters can bleed" + (testing "in enough concurrent system, path-parameters can bleed" (doseq [compiler [trie/java-trie-compiler trie/clojure-trie-compiler]] (let [app (ring/ring-handler (ring/router