Skip to content
This repository was archived by the owner on Mar 3, 2026. It is now read-only.

Commit cf57035

Browse files
jknackjknack
committed
endpoint path with regex defined params problems after jooby upgrade 1.0.0.CR6 → 1.0.0.CR8 fix jooby-project#526 
1 parent 79f5c52 commit cf57035

6 files changed

Lines changed: 150 additions & 24 deletions

File tree

‎coverage-report/src/test/java/org/jooby/issues/Issue526.java‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package org.jooby.issues;
2+
3+
import org.jooby.test.ServerFeature;
4+
import org.junit.Test;
5+
6+
public class Issue526 extends ServerFeature {
7+
8+
{
9+
get("/526/V{var:\\d{4,7}}", req -> req.param("var").value());
10+
11+
get("/526/var/:var", req -> req.param("var").value());
12+
13+
err((req, rsp, x) -> {
14+
rsp.send(x.getMessage());
15+
});
16+
}
17+
18+
public void shouldAcceptAdvancedRegexPathExpression() throws Exception {
19+
request()
20+
.get("/526/V1234")
21+
.expect("1234");
22+
23+
request()
24+
.get("/526/V12")
25+
.expect(404);
26+
27+
request()
28+
.get("/526/V1234567")
29+
.expect("1234567");
30+
31+
request()
32+
.get("/526/V12345678")
33+
.expect(404);
34+
}
35+
36+
@Test
37+
public void shouldAcceptSpecialChars() throws Exception {
38+
request()
39+
.get("/526/var/x%252Fy%252Fz")
40+
.expect("Not Found(404): /526/var/x/y/z");
41+
}
42+
43+
}

‎jooby-jetty/src/test/java/org/jooby/internal/jetty/JettyHandlerTest.java‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public void handleShouldSetMultipartConfig() throws Exception {
4545
.expect(unit -> {
4646
HttpServletRequest request = unit.get(HttpServletRequest.class);
4747

48-
expect(request.getRequestURI()).andReturn("/");
48+
expect(request.getPathInfo()).andReturn("/");
4949
})
5050
.expect(unit -> {
5151
HttpHandler dispatcher = unit.get(HttpHandler.class);
@@ -75,7 +75,7 @@ public void handleShouldIgnoreMultipartConfig() throws Exception {
7575
.expect(unit -> {
7676
HttpServletRequest request = unit.get(HttpServletRequest.class);
7777

78-
expect(request.getRequestURI()).andReturn("/");
78+
expect(request.getPathInfo()).andReturn("/");
7979
})
8080
.expect(unit -> {
8181
HttpHandler dispatcher = unit.get(HttpHandler.class);
@@ -105,7 +105,7 @@ public void handleWsUpgrade() throws Exception {
105105
.expect(unit -> {
106106
HttpServletRequest request = unit.get(HttpServletRequest.class);
107107

108-
expect(request.getRequestURI()).andReturn("/");
108+
expect(request.getPathInfo()).andReturn("/");
109109
})
110110
.expect(unit -> {
111111
HttpServletRequest req = unit.get(HttpServletRequest.class);
@@ -153,7 +153,7 @@ public void handleThrowUnsupportedOperationExceptionWhenWsIsMissing() throws Exc
153153
.expect(unit -> {
154154
HttpServletRequest request = unit.get(HttpServletRequest.class);
155155

156-
expect(request.getRequestURI()).andReturn("/");
156+
expect(request.getPathInfo()).andReturn("/");
157157
})
158158
.expect(unit -> {
159159
HttpServletRequest req = unit.get(HttpServletRequest.class);
@@ -199,7 +199,7 @@ public void handleThrowUnsupportedOperationExceptionOnNoWebSocketRequest() throw
199199
.expect(unit -> {
200200
HttpServletRequest request = unit.get(HttpServletRequest.class);
201201

202-
expect(request.getRequestURI()).andReturn("/");
202+
expect(request.getPathInfo()).andReturn("/");
203203
})
204204
.expect(unit -> {
205205
HttpServletRequest req = unit.get(HttpServletRequest.class);
@@ -241,7 +241,7 @@ public void handleThrowUnsupportedOperationExceptionOnHankshakeRejection() throw
241241
.expect(unit -> {
242242
HttpServletRequest request = unit.get(HttpServletRequest.class);
243243

244-
expect(request.getRequestURI()).andReturn("/");
244+
expect(request.getPathInfo()).andReturn("/");
245245
})
246246
.expect(unit -> {
247247
HttpServletRequest req = unit.get(HttpServletRequest.class);
@@ -285,7 +285,7 @@ public void handleThrowUnsupportedOperationExceptionOnWrongType() throws Excepti
285285
.expect(unit -> {
286286
HttpServletRequest request = unit.get(HttpServletRequest.class);
287287

288-
expect(request.getRequestURI()).andReturn("/");
288+
expect(request.getPathInfo()).andReturn("/");
289289
})
290290
.expect(unit -> {
291291
HttpHandler dispatcher = unit.get(HttpHandler.class);
@@ -322,7 +322,7 @@ public void handleShouldReThrowServletException() throws Exception {
322322
.expect(unit -> {
323323
HttpServletRequest request = unit.get(HttpServletRequest.class);
324324

325-
expect(request.getRequestURI()).andReturn("/");
325+
expect(request.getPathInfo()).andReturn("/");
326326
})
327327
.expect(unit -> {
328328
Request request = unit.get(Request.class);
@@ -356,7 +356,7 @@ public void handleShouldReThrowIOException() throws Exception {
356356
.expect(unit -> {
357357
HttpServletRequest request = unit.get(HttpServletRequest.class);
358358

359-
expect(request.getRequestURI()).andReturn("/");
359+
expect(request.getPathInfo()).andReturn("/");
360360
})
361361
.expect(unit -> {
362362
Request request = unit.get(Request.class);
@@ -390,7 +390,7 @@ public void handleShouldReThrowIllegalArgumentException() throws Exception {
390390
.expect(unit -> {
391391
HttpServletRequest request = unit.get(HttpServletRequest.class);
392392

393-
expect(request.getRequestURI()).andReturn("/");
393+
expect(request.getPathInfo()).andReturn("/");
394394
})
395395
.expect(unit -> {
396396
Request request = unit.get(Request.class);
@@ -424,7 +424,7 @@ public void handleShouldReThrowIllegalStateException() throws Exception {
424424
.expect(unit -> {
425425
HttpServletRequest request = unit.get(HttpServletRequest.class);
426426

427-
expect(request.getRequestURI()).andReturn("/");
427+
expect(request.getPathInfo()).andReturn("/");
428428
})
429429
.expect(unit -> {
430430
Request request = unit.get(Request.class);

‎jooby-servlet/src/main/java/org/jooby/servlet/ServletServletRequest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public ServletServletRequest(final HttpServletRequest req, final String tmpdir,
6060
this.req = requireNonNull(req, "HTTP req is required.");
6161
this.tmpdir = requireNonNull(tmpdir, "A tmpdir is required.");
6262
this.multipart = multipart;
63-
this.path = URLDecoder.decode(req.getRequestURI(), "UTF-8");
63+
this.path = URLDecoder.decode(req.getPathInfo(), "UTF-8");
6464
}
6565

6666
public HttpServletRequest servletRequest() {

‎jooby-servlet/src/test/java/org/jooby/servlet/ServletServletRequestTest.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public void defaults() throws IOException, Exception {
2424
.expect(unit -> {
2525
HttpServletRequest req = unit.get(HttpServletRequest.class);
2626
expect(req.getContentType()).andReturn("text/html");
27-
expect(req.getRequestURI()).andReturn("/");
27+
expect(req.getPathInfo()).andReturn("/");
2828
})
2929
.run(unit -> {
3030
new ServletServletRequest(unit.get(HttpServletRequest.class), tmpdir);
@@ -38,7 +38,7 @@ public void defaultsNullCT() throws IOException, Exception {
3838
.expect(unit -> {
3939
HttpServletRequest req = unit.get(HttpServletRequest.class);
4040
expect(req.getContentType()).andReturn(null);
41-
expect(req.getRequestURI()).andReturn("/");
41+
expect(req.getPathInfo()).andReturn("/");
4242
})
4343
.run(unit -> {
4444
new ServletServletRequest(unit.get(HttpServletRequest.class), tmpdir);
@@ -53,7 +53,7 @@ public void multipartDefaults() throws IOException, Exception {
5353
.expect(unit -> {
5454
HttpServletRequest req = unit.get(HttpServletRequest.class);
5555
expect(req.getContentType()).andReturn(MediaType.multipart.name());
56-
expect(req.getRequestURI()).andReturn("/");
56+
expect(req.getPathInfo()).andReturn("/");
5757
})
5858
.run(unit -> {
5959
new ServletServletRequest(unit.get(HttpServletRequest.class), tmpdir);
@@ -67,7 +67,7 @@ public void reqMethod() throws IOException, Exception {
6767
.expect(unit -> {
6868
HttpServletRequest req = unit.get(HttpServletRequest.class);
6969
expect(req.getContentType()).andReturn("text/html");
70-
expect(req.getRequestURI()).andReturn("/");
70+
expect(req.getPathInfo()).andReturn("/");
7171
expect(req.getMethod()).andReturn("GET");
7272
})
7373
.run(unit -> {
@@ -84,7 +84,7 @@ public void path() throws IOException, Exception {
8484
.expect(unit -> {
8585
HttpServletRequest req = unit.get(HttpServletRequest.class);
8686
expect(req.getContentType()).andReturn("text/html");
87-
expect(req.getRequestURI()).andReturn("/spaces%20in%20it");
87+
expect(req.getPathInfo()).andReturn("/spaces%20in%20it");
8888
})
8989
.run(unit -> {
9090
assertEquals("/spaces in it",
@@ -100,7 +100,7 @@ public void paramNames() throws IOException, Exception {
100100
.expect(unit -> {
101101
HttpServletRequest req = unit.get(HttpServletRequest.class);
102102
expect(req.getContentType()).andReturn("text/html");
103-
expect(req.getRequestURI()).andReturn("/");
103+
expect(req.getPathInfo()).andReturn("/");
104104
expect(req.getParameterNames()).andReturn(
105105
Iterators.asEnumeration(Lists.newArrayList("p1", "p2").iterator()));
106106
})
@@ -119,7 +119,7 @@ public void params() throws IOException, Exception {
119119
.expect(unit -> {
120120
HttpServletRequest req = unit.get(HttpServletRequest.class);
121121
expect(req.getContentType()).andReturn("text/html");
122-
expect(req.getRequestURI()).andReturn("/");
122+
expect(req.getPathInfo()).andReturn("/");
123123
expect(req.getParameterValues("x")).andReturn(new String[]{"a", "b" });
124124
})
125125
.run(unit -> {
@@ -137,7 +137,7 @@ public void noparams() throws IOException, Exception {
137137
.expect(unit -> {
138138
HttpServletRequest req = unit.get(HttpServletRequest.class);
139139
expect(req.getContentType()).andReturn("text/html");
140-
expect(req.getRequestURI()).andReturn("/");
140+
expect(req.getPathInfo()).andReturn("/");
141141
expect(req.getParameterValues("x")).andReturn(null);
142142
})
143143
.run(unit -> {
@@ -155,7 +155,7 @@ public void filesFailure() throws IOException, Exception {
155155
.expect(unit -> {
156156
HttpServletRequest req = unit.get(HttpServletRequest.class);
157157
expect(req.getContentType()).andReturn(MediaType.multipart.name());
158-
expect(req.getRequestURI()).andReturn("/");
158+
expect(req.getPathInfo()).andReturn("/");
159159
expect(req.getParts()).andThrow(new ServletException("intentional err"));
160160
})
161161
.run(unit -> {
@@ -172,7 +172,7 @@ public void noupgrade() throws IOException, Exception {
172172
.expect(unit -> {
173173
HttpServletRequest req = unit.get(HttpServletRequest.class);
174174
expect(req.getContentType()).andReturn(MediaType.multipart.name());
175-
expect(req.getRequestURI()).andReturn("/");
175+
expect(req.getPathInfo()).andReturn("/");
176176
})
177177
.run(unit -> {
178178
assertEquals(Lists.newArrayList(),

‎jooby/src/main/java/org/jooby/internal/RoutePattern.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@
2020

2121
import static java.util.Objects.requireNonNull;
2222

23-
import java.util.*;
23+
import java.util.ArrayList;
24+
import java.util.HashMap;
25+
import java.util.LinkedList;
26+
import java.util.List;
27+
import java.util.Map;
2428
import java.util.function.Function;
2529
import java.util.regex.Matcher;
2630
import java.util.regex.Pattern;
@@ -32,7 +36,10 @@
3236
public class RoutePattern {
3337

3438
private static final Pattern GLOB = Pattern
35-
.compile("\\?|/\\*\\*(:(?:[^/]+)+?)?+|\\*|\\:((?:[^/]+)+?)|\\{((?:\\{[^/]+?(?:[^/]+?\\*\\*)\\}|[^/{}]|\\\\[{}])+?)\\}");
39+
/** ?| ** | * | :var | {var(:.*)} */
40+
//.compile("\\?|/\\*\\*|\\*|\\:((?:[^/]+)+?) |\\{((?:\\{[^/]+?\\}|[^/{}]|\\\\[{}])+?)\\}");
41+
/** ? | **:name | * | :var | */
42+
.compile("\\?|/\\*\\*(\\:(?:[^/]+))?|\\*|\\:((?:[^/]+)+?)|\\{((?:\\{[^/]+?\\}|[^/{}]|\\\\[{}])+?)\\}");
3643

3744
private static final Pattern SLASH = Pattern.compile("//+");
3845

‎jooby/src/test/java/org/jooby/issues/Issue526u.java‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
package org.jooby.issues;
2+
3+
import static org.junit.Assert.assertEquals;
4+
import static org.junit.Assert.assertFalse;
5+
import static org.junit.Assert.assertTrue;
6+
7+
import java.util.Map;
8+
import java.util.function.Consumer;
9+
10+
import org.jooby.internal.RouteMatcher;
11+
import org.jooby.internal.RoutePattern;
12+
import org.junit.Test;
13+
14+
public class Issue526u {
15+
16+
class RoutePathAssert {
17+
18+
RoutePattern path;
19+
20+
public RoutePathAssert(final String method, final String pattern) {
21+
path = new RoutePattern(method, pattern);
22+
}
23+
24+
public RoutePathAssert matches(final String path) {
25+
return matches(path, (vars) -> {
26+
});
27+
}
28+
29+
public RoutePathAssert matches(final String path, final Consumer<Map<Object, String>> vars) {
30+
String message = this.path + " != " + path;
31+
RouteMatcher matcher = this.path.matcher(path);
32+
boolean matches = matcher.matches();
33+
if (!matches) {
34+
System.err.println(message);
35+
}
36+
assertTrue(message, matches);
37+
vars.accept(matcher.vars());
38+
return this;
39+
}
40+
41+
public RoutePathAssert butNot(final String path) {
42+
String message = this.path + " == " + path;
43+
RouteMatcher matcher = this.path.matcher(path);
44+
boolean matches = matcher.matches();
45+
if (matches) {
46+
System.err.println(message);
47+
}
48+
assertFalse(message, matches);
49+
return this;
50+
}
51+
}
52+
53+
@Test
54+
public void shouldAcceptAdvancedRegexPathExpression() {
55+
new RoutePathAssert("GET", "/V{var:\\d{4,7}}")
56+
.matches("GET/V1234", (vars) -> {
57+
assertEquals("1234", vars.get("var"));
58+
})
59+
.matches("GET/V1234567", (vars) -> {
60+
assertEquals("1234567", vars.get("var"));
61+
})
62+
.butNot("GET/V123")
63+
.butNot("GET/V12345678");
64+
}
65+
66+
@Test
67+
public void shouldAcceptSpecialChars() {
68+
new RoutePathAssert("GET", "/:var")
69+
.matches("GET/x%252Fy%252Fz", (vars) -> {
70+
assertEquals("x%252Fy%252Fz", vars.get("var"));
71+
})
72+
.butNot("GET/user/123/x")
73+
.butNot("GET/user/123x")
74+
.butNot("GET/user/xqi");
75+
}
76+
}

0 commit comments

Comments
 (0)