Skip to content

Commit c7f9796

Browse files
committed
404 NPE: fix null pointer exception on custom 404 (introduced in previous refactor)
1 parent 97b8e4a commit c7f9796

File tree

10 files changed

+60
-22
lines changed

10 files changed

+60
-22
lines changed

jooby/src/main/java/io/jooby/DefaultContext.java

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import io.jooby.internal.SingleValue;
1313
import io.jooby.internal.UrlParser;
1414
import io.jooby.internal.ValueConverters;
15+
import org.slf4j.Logger;
1516

1617
import javax.annotation.Nonnull;
1718
import javax.annotation.Nullable;
@@ -559,30 +560,38 @@ default Context setResponseHeader(@Nonnull String name, @Nonnull Object value) {
559560
}
560561

561562
/**
562-
* Send an error response.
563+
* Send an error response. This method set the error code.
563564
*
564565
* @param cause Error. If this is a fatal error it is going to be rethrow it.
565-
* @param statusCode Status code.
566+
* @param code Default error code.
566567
* @return This context.
567568
*/
568569
@Override @Nonnull default Context sendError(@Nonnull Throwable cause,
569-
@Nonnull StatusCode statusCode) {
570+
@Nonnull StatusCode code) {
570571
Router router = getRouter();
572+
Logger log = router.getLog();
571573
if (isResponseStarted()) {
572-
router.getLog().error(ErrorHandler.errorMessage(this, statusCode), cause);
574+
log.error(ErrorHandler.errorMessage(this, code), cause);
573575
} else {
574576
try {
575577
if (getResetHeadersOnError()) {
576578
removeResponseHeaders();
577579
}
578-
router.getErrorHandler().apply(this, cause, statusCode);
580+
// set default error code
581+
setResponseCode(code);
582+
router.getErrorHandler().apply(this, cause, code);
579583
} catch (Exception x) {
584+
if (!isResponseStarted()) {
585+
// edge case when there is a bug in a the error handler (probably custom error) what we
586+
// do is to use the default error handler
587+
ErrorHandler.create().apply(this, cause, code);
588+
}
580589
if (Server.connectionLost(x)) {
581-
router.getLog()
582-
.debug("error handler resulted in exception {} {}", getMethod(), getRequestPath(), x);
590+
log.debug("error handler resulted in a exception while processing `{}`", cause.toString(),
591+
x);
583592
} else {
584-
router.getLog()
585-
.error("error handler resulted in exception {} {}", getMethod(), getRequestPath(), x);
593+
log.error("error handler resulted in a exception while processing `{}`", cause.toString(),
594+
x);
586595
}
587596
}
588597
}

jooby/src/main/java/io/jooby/DefaultErrorHandler.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public class DefaultErrorHandler implements ErrorHandler {
5656
@Nonnull @Override public void apply(@Nonnull Context ctx, @Nonnull Throwable cause,
5757
@Nonnull StatusCode code) {
5858
Logger log = ctx.getRouter().getLog();
59-
if (mute(cause, code)) {
59+
if (isMuted(cause, code)) {
6060
log.debug(ErrorHandler.errorMessage(ctx, code), cause);
6161
} else {
6262
log.error(ErrorHandler.errorMessage(ctx, code), cause);
@@ -117,11 +117,12 @@ public class DefaultErrorHandler implements ErrorHandler {
117117
}
118118
}
119119

120-
private boolean mute(Throwable cause, StatusCode statusCode) {
120+
private boolean isMuted(Throwable cause, StatusCode statusCode) {
121121
return muteCodes.contains(statusCode)
122122
// same class filter
123123
|| muteTypes.stream().anyMatch(type -> type == cause.getClass())
124124
// sub-class filter
125125
|| muteTypes.stream().anyMatch(type -> type.isInstance(cause));
126126
}
127+
127128
}

jooby/src/main/java/io/jooby/ForwardingContext.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -591,8 +591,8 @@ public Context sendRedirect(@Nonnull StatusCode redirect, @Nonnull String locati
591591
}
592592

593593
@Nonnull @Override
594-
public Context sendError(@Nonnull Throwable cause, @Nonnull StatusCode statusCode) {
595-
ctx.sendError(cause, statusCode);
594+
public Context sendError(@Nonnull Throwable cause, @Nonnull StatusCode code) {
595+
ctx.sendError(cause, code);
596596
return this;
597597
}
598598

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ class Chi implements RouteTree {
3333
static final StaticRoute NO_MATCH = new StaticRoute(Collections.emptyMap());
3434

3535
static final char ZERO_CHAR = (char) 0;
36+
private MessageEncoder encoder;
3637

3738
static class StaticRoute {
3839
private final Map<String, StaticRouterMatch> methods;
@@ -811,14 +812,21 @@ public boolean exists(String method, String path) {
811812
return true;
812813
}
813814

814-
public Router.Match find(String method, String path) {
815+
@Override public Router.Match find(String method, String path) {
815816
StaticRouterMatch match = staticPaths.getOrDefault(path, NO_MATCH).methods.get(method);
816817
if (match == null) {
817818
// use radix tree
818819
RouterMatch result = new RouterMatch();
819820
Route route = root.findRoute(result, method, new ZeroCopyString(path));
820-
return route == null ? result.missing(method, path) : result.found(route);
821+
if (route == null) {
822+
return result.missing(method, path, encoder);
823+
}
824+
return result.found(route);
821825
}
822826
return match;
823827
}
828+
829+
public void setEncoder(MessageEncoder encoder) {
830+
this.encoder = encoder;
831+
}
824832
}

jooby/src/main/java/io/jooby/internal/RouterImpl.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,7 @@ private Route newRoute(@Nonnull String method, @Nonnull String pattern,
477477
return route;
478478
}
479479

480-
@Nonnull public Router start(@Nonnull Jooby owner) {
480+
@Nonnull public Router start(@Nonnull Jooby app) {
481481
if (err == null) {
482482
err = ErrorHandler.create();
483483
} else {
@@ -492,7 +492,7 @@ private Route newRoute(@Nonnull String method, @Nonnull String pattern,
492492
ClassSource source = new ClassSource(classLoader);
493493
RouteAnalyzer analyzer = new RouteAnalyzer(source, false);
494494

495-
ExecutionMode mode = owner.getExecutionMode();
495+
ExecutionMode mode = app.getExecutionMode();
496496
for (Route route : routes) {
497497
String executorKey = route.getExecutorKey();
498498
Executor executor;
@@ -538,6 +538,8 @@ private Route newRoute(@Nonnull String method, @Nonnull String pattern,
538538
/** Final render */
539539
route.setEncoder(encoder);
540540
}
541+
((Chi)chi).setEncoder(encoder);
542+
541543
/** router options: */
542544
if (routerOptions.contains(RouterOption.IGNORE_CASE)) {
543545
chi = new RouteTreeLowerCasePath(chi);

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,14 +88,15 @@ public void execute(Context context) {
8888
}
8989
}
9090

91-
public RouterMatch missing(String method, String path) {
91+
public RouterMatch missing(String method, String path, MessageEncoder encoder) {
9292
Route.Handler h;
9393
if (this.handler == null) {
9494
h = path.endsWith("/favicon.ico") ? Route.FAVICON : Route.NOT_FOUND;
9595
} else {
9696
h = this.handler;
9797
}
9898
this.route = new Route(method, path, h);
99+
this.route.setEncoder(encoder);
99100
this.route.setReturnType(Context.class);
100101
return this;
101102
}

modules/jooby-bom/pom.xml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
<!-- THIS FILE IS AUTO GENERATED. DON'T EDIT -->
1717

1818
<properties>
19-
<jooby.version>2.8.0</jooby.version>
19+
<jooby.version>2.8.1-SNAPSHOT</jooby.version>
2020
<HikariCP.version>3.4.2</HikariCP.version>
2121
<archetype-packaging.version>3.1.2</archetype-packaging.version>
2222
<asm.version>8.0.1</asm.version>
@@ -55,8 +55,8 @@
5555
<jetty.version>9.4.27.v20200227</jetty.version>
5656
<jfiglet.version>0.0.8</jfiglet.version>
5757
<jmespath-java.version>1.11.757</jmespath-java.version>
58-
<jooby-maven-plugin.version>2.8.0</jooby-maven-plugin.version>
59-
<jooby.version>2.8.0</jooby.version>
58+
<jooby-maven-plugin.version>2.8.1-SNAPSHOT</jooby-maven-plugin.version>
59+
<jooby.version>2.8.1-SNAPSHOT</jooby.version>
6060
<json.version>20190722</json.version>
6161
<jsonwebtoken.version>0.11.1</jsonwebtoken.version>
6262
<jsr305.version>3.0.2</jsr305.version>

modules/jooby-test/src/main/java/io/jooby/MockContext.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,7 @@ public MockContext setResponseType(@Nonnull MediaType contentType, @Nullable Cha
606606
}
607607

608608
@Nonnull @Override
609-
public MockContext sendError(@Nonnull Throwable cause, @Nonnull StatusCode statusCode) {
609+
public MockContext sendError(@Nonnull Throwable cause, @Nonnull StatusCode code) {
610610
responseStarted = true;
611611
this.response.setResult(cause)
612612
.setStatusCode(router.errorCode(cause));

tests/src/test/java/io/jooby/FeaturedTest.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2932,6 +2932,22 @@ public void forward(ServerTestRunner runner) {
29322932
});
29332933
}
29342934

2935+
@ServerTest
2936+
public void custom404WithTemplateEngine(ServerTestRunner runner) {
2937+
runner.define(app -> {
2938+
app.install(new HandlebarsModule());
2939+
2940+
app.error((ctx, cause, code) -> {
2941+
ctx.render(new ModelAndView("error.hbs").put("statusCode", code));
2942+
});
2943+
}).ready(client -> {
2944+
client.get("/missing", rsp -> {
2945+
assertEquals("Not Found (404)", rsp.body().string().trim());
2946+
assertEquals(404, rsp.code());
2947+
});
2948+
});
2949+
}
2950+
29352951
private byte[][] partition(byte[] bytes, int size) {
29362952
List<byte[]> result = new ArrayList<>();
29372953
int offset = 0;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{{statusCode}}

0 commit comments

Comments
 (0)