Skip to content

Commit 32cf7c2

Browse files
jknackjknack
committed
Multiple regexps don't work in the same route definition fix #1515 
1 parent 5a9057d commit 32cf7c2

File tree

6 files changed

+131
-29
lines changed

6 files changed

+131
-29
lines changed

‎jooby/src/main/java/io/jooby/Router.java‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -843,18 +843,25 @@ default Router error(@Nonnull Predicate<StatusCode> predicate,
843843
int start = -1;
844844
int end = Integer.MAX_VALUE;
845845
int len = pattern.length();
846+
int curly = 0;
846847
for (int i = 0; i < len; i++) {
847848
char ch = pattern.charAt(i);
848849
if (ch == '{') {
849-
start = i + 1;
850-
end = Integer.MAX_VALUE;
850+
if (curly == 0) {
851+
start = i + 1;
852+
end = Integer.MAX_VALUE;
853+
}
854+
curly += 1;
851855
} else if (ch == ':') {
852856
end = i;
853857
} else if (ch == '}') {
854-
String id = pattern.substring(start, Math.min(i, end));
855-
result.add(id);
856-
start = -1;
857-
end = Integer.MAX_VALUE;
858+
curly -= 1;
859+
if (curly == 0) {
860+
String id = pattern.substring(start, Math.min(i, end));
861+
result.add(id);
862+
start = -1;
863+
end = Integer.MAX_VALUE;
864+
}
858865
} else if (ch == '*') {
859866
if (i == len - 1) {
860867
result.add("*");

‎jooby/src/main/java/io/jooby/internal/Chi.java‎

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@
1818
import java.util.stream.Collectors;
1919
import java.util.stream.Stream;
2020

21+
/**
22+
* Sync: 20-01-20
23+
* Commit: 17fb1065d2b256d20f68bed0b7bca6c2942aff49
24+
*/
2125
class Chi implements RouteTree {
2226
private static final byte ntStatic = 0;// /home
2327
private static final byte ntRegexp = 1; // /{id:[0-9]+}
@@ -379,9 +383,8 @@ Node addChild(Node child, ZeroCopyString search) {
379383
// Search prefix contains a param, regexp or wildcard
380384

381385
if (segTyp == ntRegexp) {
382-
rex = Pattern.compile(seg.rexPat.toString());
383386
child.prefix = seg.rexPat;
384-
child.rex = rex;
387+
child.rex = Pattern.compile(seg.rexPat.toString());
385388
}
386389

387390
if (segStartIdx == 0) {
@@ -399,7 +402,7 @@ Node addChild(Node child, ZeroCopyString search) {
399402
child.tail = seg.tail; // for params, we set the tail
400403

401404
if (segStartIdx != search.length()) {
402-
// deploy static edge for the remaining part, split the end.
405+
// add static edge for the remaining part, split the end.
403406
// its not possible to have adjacent param nodes, so its certainly
404407
// going to be a static node next.
405408

@@ -418,7 +421,7 @@ Node addChild(Node child, ZeroCopyString search) {
418421
child.prefix = search.substring(0, segStartIdx);
419422
child.rex = null;
420423

421-
// deploy the param edge node
424+
// add the param edge node
422425
search = search.substring(segStartIdx);
423426

424427
Node nn = new Node().typ(segTyp).label(search.charAt(0)).tail(seg.tail);
@@ -491,81 +494,82 @@ void setEndpoint(String method, Route route) {
491494

492495
// Recursive edge traversal by checking all nodeTyp groups along the way.
493496
// It's like searching through a multi-dimensional radix trie.
494-
Route findRoute(RouterMatch rctx, String method, ZeroCopyString pattern) {
497+
Route findRoute(RouterMatch rctx, String method, ZeroCopyString path) {
495498
Node n = this;
496499
Node nn = n;
497500

501+
ZeroCopyString search = path;
502+
498503
for (int ntyp = 0; ntyp < NODE_SIZE; ntyp++) {
499504
Node[] nds = nn.children[ntyp];
500505
if (nds != null) {
501506
Node xn = null;
502-
ZeroCopyString search = pattern;
507+
ZeroCopyString xsearch = search;
503508

504509
char label = search.length() > 0 ? search.charAt(0) : ZERO_CHAR;
505510

506511
switch (ntyp) {
507512
case ntStatic:
508513
xn = findEdge(nds, label);
509-
if (xn == null || !search.startsWith(xn.prefix)) {
514+
if (xn == null || !xsearch.startsWith(xn.prefix)) {
510515
continue;
511516
}
512-
search = search.substring(xn.prefix.length());
517+
xsearch = xsearch.substring(xn.prefix.length());
513518
break;
514519

515520
case ntParam:
516521
case ntRegexp:
517522
// short-circuit and return no matching route for empty param values
518-
if (search.length() == 0) {
523+
if (xsearch.length() == 0) {
519524
continue;
520525
}
521-
522526
// serially loop through each node grouped by the tail delimiter
523527
for (int idx = 0; idx < nds.length; idx++) {
524528
xn = nds[idx];
525529

526530
// label for param nodes is the delimiter byte
527-
int p = search.indexOf(xn.tail);
531+
int p = xsearch.indexOf(xn.tail);
528532

529-
if (p <= 0) {
533+
if (p < 0) {
530534
if (xn.tail == '/') {
531-
p = search.length();
535+
p = xsearch.length();
532536
} else {
533537
continue;
534538
}
535539
}
536540

537-
if (ntyp == ntRegexp) {
538-
if (!xn.rex.matcher(search.substring(0, p).toString()).matches()) {
541+
if (ntyp == ntRegexp && xn.rex != null) {
542+
if (!xn.rex.matcher(xsearch.substring(0, p).toString()).matches()) {
539543
continue;
540544
}
541-
} else if (search.substring(0, p).indexOf('/') != -1) {
545+
} else if (xsearch.substring(0, p).indexOf('/') != -1) {
542546
// avoid a newRuntimeRoute across path segments
543547
continue;
544548
}
545549

546550
// rctx.routeParams.Values = append(rctx.routeParams.Values, xsearch[:p])
547-
rctx.value(search.substring(0, p));
548-
search = search.substring(p);
551+
rctx.value(xsearch.substring(0, p));
552+
xsearch = xsearch.substring(p);
549553
break;
550554
}
551555
break;
552556

553557
default:
554558
// catch-all nodes
555559
// rctx.routeParams.Values = append(rctx.routeParams.Values, search)
556-
if (search.length() > 0) {
557-
rctx.value(search);
560+
if (xsearch.length() > 0) {
561+
rctx.value(xsearch);
558562
}
559563
xn = nds[0];
560-
search = ZeroCopyString.EMPTY;
564+
xsearch = ZeroCopyString.EMPTY;
561565
}
562566

563567
if (xn == null) {
564568
continue;
565569
}
566570

567571
// did we returnType it yet?
568-
if (search.length() == 0) {
572+
if (xsearch.length() == 0) {
569573
if (xn.isLeaf()) {
570574
Route h = xn.endpoints.get(method);
571575
if (h != null) {
@@ -581,7 +585,7 @@ Route findRoute(RouterMatch rctx, String method, ZeroCopyString pattern) {
581585
}
582586

583587
// recursively returnType the next node..
584-
Route fin = xn.findRoute(rctx, method, search);
588+
Route fin = xn.findRoute(rctx, method, xsearch);
585589
if (fin != null) {
586590
return fin;
587591
}

‎jooby/src/main/java/io/jooby/internal/RouterMatch.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ public void key(List<String> keys) {
4141
}
4242
}
4343

44+
public void value(String value) {
45+
46+
}
47+
4448
public void value(Chi.ZeroCopyString value) {
4549
if (vars == Collections.EMPTY_MAP) {
4650
vars = new LinkedHashMap();

‎jooby/src/test/java/io/jooby/RouterTest.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@
1515
public class RouterTest {
1616
@Test
1717
public void pathKeys() {
18+
pathKeys("/{lang:[a-z]{2}}", keys -> {
19+
assertEquals(Collections.singletonList("lang"), keys);
20+
});
21+
1822
pathKeys("/*", keys -> assertEquals(1, keys.size()));
1923

2024
pathKeys("/foo/?*", keys -> assertEquals(1, keys.size()));

‎jooby/src/test/java/io/jooby/internal/ChiTest.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import org.junit.jupiter.api.Test;
99

1010
import static org.junit.jupiter.api.Assertions.assertEquals;
11+
import static org.junit.jupiter.api.Assertions.assertFalse;
1112
import static org.junit.jupiter.api.Assertions.assertTrue;
1213
import static org.mockito.Mockito.mock;
1314
import static org.mockito.Mockito.when;
@@ -131,6 +132,45 @@ public void searchParam() throws Exception {
131132
});
132133
}
133134

135+
@Test
136+
public void multipleRegex() {
137+
Chi router = new Chi();
138+
139+
router.insert(route("GET", "/{lang:[a-z][a-z]}/{page:[^.]+}/", stringHandler("1515")));
140+
141+
find(router, "/12/f/", (ctx, result) -> {
142+
assertFalse(result.matches());
143+
assertEquals(null, result.route().getPipeline().apply(ctx));
144+
});
145+
146+
find(router, "/ar/page/", (ctx, result) -> {
147+
assertTrue(result.matches());
148+
assertEquals("1515", result.route().getPipeline().apply(ctx));
149+
});
150+
151+
find(router, "/arx/page/", (ctx, result) -> {
152+
assertFalse(result.matches());
153+
assertEquals(null, result.route().getPipeline().apply(ctx));
154+
});
155+
}
156+
157+
@Test
158+
public void regexWithQuantity() {
159+
Chi router = new Chi();
160+
161+
router.insert(route("GET", "/{lang:[a-z]{2}}/", stringHandler("qx")));
162+
163+
find(router, "/12/", (ctx, result) -> {
164+
assertFalse(result.matches());
165+
assertEquals(null, result.route().getPipeline().apply(ctx));
166+
});
167+
168+
find(router, "/ar/", (ctx, result) -> {
169+
assertTrue(result.matches());
170+
assertEquals("qx", result.route().getPipeline().apply(ctx));
171+
});
172+
}
173+
134174
private void find(Chi router, String pattern,
135175
SneakyThrows.Consumer2<Context, Router.Match> consumer) {
136176
Router.Match result = router

‎tests/src/test/java/io/jooby/Issue1515.java‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package io.jooby;
2+
3+
import io.jooby.junit.ServerTest;
4+
import io.jooby.junit.ServerTestRunner;
5+
6+
import static org.junit.jupiter.api.Assertions.assertEquals;
7+
8+
public class Issue1515 {
9+
10+
@ServerTest
11+
public void issue1515(ServerTestRunner runner) {
12+
runner.define(app -> {
13+
app.get("/{lang:[a-z]{2}}/{page:[^.]+}/", ctx -> ctx.pathMap());
14+
15+
app.get("/{lang:[a-z]{2}}/", ctx -> ctx.pathMap());
16+
17+
}).ready(client -> {
18+
client.get("/ar/page/", rsp -> {
19+
assertEquals("{lang=ar, page=page}", rsp.body().string());
20+
});
21+
22+
client.get("/12/page/", rsp -> {
23+
assertEquals(404, rsp.code());
24+
});
25+
26+
client.get("/abc/page/", rsp -> {
27+
assertEquals(404, rsp.code());
28+
});
29+
30+
client.get("/ar/", rsp -> {
31+
assertEquals("{lang=ar}", rsp.body().string());
32+
});
33+
34+
client.get("/12/", rsp -> {
35+
assertEquals(404, rsp.code());
36+
});
37+
38+
client.get("/abc/", rsp -> {
39+
assertEquals(404, rsp.code());
40+
});
41+
});
42+
}
43+
}

0 commit comments

Comments
 (0)