Skip to content

Commit d61095a

Browse files
authored
fix: Drop TypeMapper from exception and security paths (#85)
Mirror PR #76's approach (HealthRenderer) for the problem+json wire shape. The default exception handler and SecurityFilter no longer route ProblemDetail through a TypeMapper; a hand-rolled ProblemDetailRenderer writes the fixed RFC 7807 shape directly, so the library emits problem responses without a JSON library on the classpath and without record-accessor reflection that GraalVM Native Image would otherwise need configured. Also replace the reflective GsonJsonMapper newInstance with a direct constructor call, still gated by the existing Class.forName Gson-presence probe. Drops the matching reflect-config entry for the constructor. Extract the shared JSON string-escape into internal/JsonStrings, used by both HealthRenderer and ProblemDetailRenderer. BREAKING CHANGE: Handlers.defaultExceptionHandler(TypeMapper) becomes defaultExceptionHandler(). SecurityFilter constructor drops its TypeMapper parameter.
1 parent d613f05 commit d61095a

9 files changed

Lines changed: 195 additions & 91 deletions

File tree

src/main/java/com/retailsvc/http/Handlers.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import com.retailsvc.http.internal.HealthRenderer;
1313
import com.retailsvc.http.internal.ProblemDetail;
14+
import com.retailsvc.http.internal.ProblemDetailRenderer;
1415
import com.retailsvc.http.internal.ResourceSource;
1516
import java.io.InputStream;
1617
import java.nio.file.Path;
@@ -27,19 +28,18 @@ public final class Handlers {
2728

2829
private Handlers() {}
2930

30-
public static ExceptionHandler defaultExceptionHandler(TypeMapper jsonMapper) {
31-
Objects.requireNonNull(jsonMapper, "jsonMapper must not be null");
31+
public static ExceptionHandler defaultExceptionHandler() {
3232
return t ->
3333
switch (t) {
3434
case ValidationException ve ->
3535
Response.bytes(
3636
HTTP_BAD_REQUEST,
37-
jsonMapper.writeTo(ProblemDetail.forValidation(ve.error())),
37+
ProblemDetailRenderer.renderJson(ProblemDetail.forValidation(ve.error())),
3838
"application/problem+json");
3939
case BadRequestException bre ->
4040
Response.bytes(
4141
bre.status(),
42-
jsonMapper.writeTo(ProblemDetail.forBadRequest(bre)),
42+
ProblemDetailRenderer.renderJson(ProblemDetail.forBadRequest(bre)),
4343
"application/problem+json");
4444
case NotFoundException _ -> Response.notFound();
4545
case MethodNotAllowedException mna ->

src/main/java/com/retailsvc/http/OpenApiServer.java

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import com.retailsvc.http.internal.Router;
1515
import com.retailsvc.http.internal.SecurityFilter;
1616
import com.retailsvc.http.internal.TextTypeMapper;
17+
import com.retailsvc.http.internal.gson.GsonJsonMapper;
1718
import com.retailsvc.http.spec.Operation;
1819
import com.retailsvc.http.spec.Spec;
1920
import com.retailsvc.http.spec.security.SecurityRequirement;
@@ -47,7 +48,6 @@ public class OpenApiServer implements AutoCloseable {
4748
private static final int DEFAULT_PORT = 8080;
4849
private static final String JSON = "application/json";
4950
private static final String GSON_CLASS = "com.google.gson.Gson";
50-
private static final String GSON_MAPPER_CLASS = "com.retailsvc.http.internal.gson.GsonJsonMapper";
5151

5252
private final HttpServer httpServer;
5353
private final int shutdownTimeoutSeconds;
@@ -112,8 +112,7 @@ record HandlerConfig(
112112
spec.securitySchemes(),
113113
spec.security(),
114114
handlerConfig.securityValidators(),
115-
handlerConfig.externalAuth(),
116-
bodyMappers.get(JSON)));
115+
handlerConfig.externalAuth()));
117116
ctx.setHandler(
118117
new DispatchHandler(
119118
handlerConfig.handlers(),
@@ -342,9 +341,7 @@ public OpenApiServer build() throws IOException {
342341
}
343342
Map<String, TypeMapper> resolved = resolveBodyMappers(bodyMappers);
344343
ExceptionHandler effectiveExceptionHandler =
345-
exceptionHandler != null
346-
? exceptionHandler
347-
: Handlers.defaultExceptionHandler(resolved.get(JSON));
344+
exceptionHandler != null ? exceptionHandler : Handlers.defaultExceptionHandler();
348345
HandlerConfig handlerConfig =
349346
new HandlerConfig(
350347
handlers,
@@ -410,12 +407,7 @@ private static TypeMapper tryLoadGsonMapper() {
410407
} catch (ClassNotFoundException _) {
411408
return null;
412409
}
413-
try {
414-
Class<?> cls = Class.forName(GSON_MAPPER_CLASS, true, OpenApiServer.class.getClassLoader());
415-
return (TypeMapper) cls.getDeclaredConstructor().newInstance();
416-
} catch (ReflectiveOperationException e) {
417-
throw new IllegalStateException("Failed to load " + GSON_MAPPER_CLASS, e);
418-
}
410+
return new GsonJsonMapper();
419411
}
420412
}
421413
}

src/main/java/com/retailsvc/http/internal/HealthRenderer.java

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,36 +23,12 @@ public static String renderJson(boolean up, List<Dependency> dependencies) {
2323
}
2424
Dependency d = dependencies.get(i);
2525
sb.append("{\"id\":");
26-
appendJsonString(sb, d.id());
26+
JsonStrings.appendQuoted(sb, d.id());
2727
sb.append(",\"status\":\"").append(label(d.up())).append("\"}");
2828
}
2929
return sb.append("]}").toString();
3030
}
3131

32-
private static void appendJsonString(StringBuilder sb, String s) {
33-
sb.append('"');
34-
for (int i = 0; i < s.length(); i++) {
35-
char c = s.charAt(i);
36-
switch (c) {
37-
case '"' -> sb.append("\\\"");
38-
case '\\' -> sb.append("\\\\");
39-
case '\b' -> sb.append("\\b");
40-
case '\f' -> sb.append("\\f");
41-
case '\n' -> sb.append("\\n");
42-
case '\r' -> sb.append("\\r");
43-
case '\t' -> sb.append("\\t");
44-
default -> {
45-
if (c < 0x20) {
46-
sb.append(String.format("\\u%04x", (int) c));
47-
} else {
48-
sb.append(c);
49-
}
50-
}
51-
}
52-
}
53-
sb.append('"');
54-
}
55-
5632
private static String label(boolean up) {
5733
return up ? "Up" : "Down";
5834
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package com.retailsvc.http.internal;
2+
3+
/**
4+
* Minimal JSON string-escape helper shared by the library's hand-rolled JSON writers ({@link
5+
* HealthRenderer}, {@link ProblemDetailRenderer}). Lets those renderers emit RFC 8259 compliant
6+
* strings without pulling in a JSON library and without record-accessor reflection that GraalVM
7+
* Native Image would otherwise need configured.
8+
*/
9+
final class JsonStrings {
10+
11+
/** Codepoints below this value are control characters and must be unicode-escaped. */
12+
private static final int FIRST_PRINTABLE_ASCII = 0x20;
13+
14+
private JsonStrings() {}
15+
16+
/** Appends {@code value} surrounded by double quotes, with JSON string-escaping applied. */
17+
static void appendQuoted(StringBuilder out, String value) {
18+
out.append('"');
19+
for (int i = 0; i < value.length(); i++) {
20+
char c = value.charAt(i);
21+
switch (c) {
22+
case '"' -> out.append("\\\"");
23+
case '\\' -> out.append("\\\\");
24+
case '\b' -> out.append("\\b");
25+
case '\f' -> out.append("\\f");
26+
case '\n' -> out.append("\\n");
27+
case '\r' -> out.append("\\r");
28+
case '\t' -> out.append("\\t");
29+
default -> {
30+
if (c < FIRST_PRINTABLE_ASCII) {
31+
out.append(String.format("\\u%04x", (int) c));
32+
} else {
33+
out.append(c);
34+
}
35+
}
36+
}
37+
}
38+
out.append('"');
39+
}
40+
}
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package com.retailsvc.http.internal;
2+
3+
import java.nio.charset.StandardCharsets;
4+
5+
/**
6+
* Built-in JSON writer for the {@code application/problem+json} (RFC 7807) wire shape. Keeps the
7+
* exception and security paths free of any {@code TypeMapper}, so the library can emit problem
8+
* responses without a JSON library on the classpath (and without record-accessor reflection that
9+
* GraalVM Native Image would otherwise need configured).
10+
*
11+
* <p>Null-valued fields are omitted, matching the default Gson encoding the library historically
12+
* produced.
13+
*/
14+
public final class ProblemDetailRenderer {
15+
16+
/** Initial capacity sized for a typical problem-detail document. */
17+
private static final int INITIAL_CAPACITY = 128;
18+
19+
private ProblemDetailRenderer() {}
20+
21+
public static byte[] renderJson(ProblemDetail pd) {
22+
StringBuilder out = new StringBuilder(INITIAL_CAPACITY);
23+
out.append('{');
24+
boolean first = true;
25+
first = appendString(out, first, "type", pd.type());
26+
first = appendString(out, first, "title", pd.title());
27+
first = appendInt(out, first, "status", pd.status());
28+
first = appendString(out, first, "detail", pd.detail());
29+
first = appendString(out, first, "pointer", pd.pointer());
30+
appendString(out, first, "keyword", pd.keyword());
31+
out.append('}');
32+
return out.toString().getBytes(StandardCharsets.UTF_8);
33+
}
34+
35+
private static boolean appendString(StringBuilder out, boolean first, String name, String value) {
36+
if (value == null) {
37+
return first;
38+
}
39+
if (!first) {
40+
out.append(',');
41+
}
42+
out.append('"').append(name).append("\":");
43+
JsonStrings.appendQuoted(out, value);
44+
return false;
45+
}
46+
47+
private static boolean appendInt(StringBuilder out, boolean first, String name, int value) {
48+
if (!first) {
49+
out.append(',');
50+
}
51+
out.append('"').append(name).append("\":").append(value);
52+
return false;
53+
}
54+
}

src/main/java/com/retailsvc/http/internal/SecurityFilter.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
import com.retailsvc.http.Request;
77
import com.retailsvc.http.SchemeValidator;
8-
import com.retailsvc.http.TypeMapper;
98
import com.retailsvc.http.spec.Operation;
109
import com.retailsvc.http.spec.security.SecurityRequirement;
1110
import com.retailsvc.http.spec.security.SecurityScheme;
@@ -19,7 +18,6 @@
1918
import java.util.List;
2019
import java.util.Locale;
2120
import java.util.Map;
22-
import java.util.Objects;
2321
import java.util.Optional;
2422

2523
public final class SecurityFilter extends Filter {
@@ -29,21 +27,18 @@ public final class SecurityFilter extends Filter {
2927
private final List<SecurityRequirement> rootSecurity;
3028
private final Map<String, SchemeValidator> validators;
3129
private final boolean externalAuth;
32-
private final TypeMapper jsonMapper;
3330

3431
public SecurityFilter(
3532
Map<String, Operation> operationsById,
3633
Map<String, SecurityScheme> schemes,
3734
List<SecurityRequirement> rootSecurity,
3835
Map<String, SchemeValidator> validators,
39-
boolean externalAuth,
40-
TypeMapper jsonMapper) {
36+
boolean externalAuth) {
4137
this.operationsById = Map.copyOf(operationsById);
4238
this.schemes = Map.copyOf(schemes);
4339
this.rootSecurity = List.copyOf(rootSecurity);
4440
this.validators = Map.copyOf(validators);
4541
this.externalAuth = externalAuth;
46-
this.jsonMapper = Objects.requireNonNull(jsonMapper, "jsonMapper must not be null");
4742
}
4843

4944
@Override
@@ -125,7 +120,7 @@ private void renderRejection(HttpExchange exchange, List<GroupOutcome.Failed> fa
125120

126121
ProblemDetail problemDetail =
127122
new ProblemDetail("about:blank", title, status, detail, null, null);
128-
byte[] body = jsonMapper.writeTo(problemDetail);
123+
byte[] body = ProblemDetailRenderer.renderJson(problemDetail);
129124
exchange.getResponseHeaders().add("Content-Type", "application/problem+json");
130125
if (!anyDenied) {
131126
LinkedHashSet<String> attempted = new LinkedHashSet<>();

src/test/java/com/retailsvc/http/HandlersDefaultExceptionTest.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class HandlersDefaultExceptionTest {
1616
@Test
1717
void validationExceptionRendersProblemJson() {
1818
Response resp =
19-
Handlers.defaultExceptionHandler(JSON)
19+
Handlers.defaultExceptionHandler()
2020
.handle(
2121
new ValidationException(
2222
new ValidationError("/x", "type", "expected string", null)));
@@ -35,7 +35,7 @@ void validationExceptionRendersProblemJson() {
3535
@Test
3636
void badRequestExceptionRendersProblemJsonWithCustomStatus() {
3737
Response resp =
38-
Handlers.defaultExceptionHandler(JSON)
38+
Handlers.defaultExceptionHandler()
3939
.handle(new BadRequestException(422, "email taken", "/email", "unique"));
4040

4141
assertThat(resp.status()).isEqualTo(422);
@@ -53,7 +53,7 @@ void badRequestExceptionRendersProblemJsonWithCustomStatus() {
5353

5454
@Test
5555
void notFoundReturns404() {
56-
Response resp = Handlers.defaultExceptionHandler(JSON).handle(new NotFoundException("GET /x"));
56+
Response resp = Handlers.defaultExceptionHandler().handle(new NotFoundException("GET /x"));
5757

5858
assertThat(resp.status()).isEqualTo(404);
5959
assertThat(resp.body()).isNull();
@@ -62,7 +62,7 @@ void notFoundReturns404() {
6262
@Test
6363
void methodNotAllowedReturns405WithAllowHeader() {
6464
Response resp =
65-
Handlers.defaultExceptionHandler(JSON)
65+
Handlers.defaultExceptionHandler()
6666
.handle(new MethodNotAllowedException(Set.of(HttpMethod.GET, HttpMethod.POST)));
6767

6868
assertThat(resp.status()).isEqualTo(405);
@@ -72,7 +72,7 @@ void methodNotAllowedReturns405WithAllowHeader() {
7272

7373
@Test
7474
void unknownExceptionReturns500() {
75-
Response resp = Handlers.defaultExceptionHandler(JSON).handle(new RuntimeException("kaboom"));
75+
Response resp = Handlers.defaultExceptionHandler().handle(new RuntimeException("kaboom"));
7676

7777
assertThat(resp.status()).isEqualTo(500);
7878
assertThat(resp.body()).isNull();
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package com.retailsvc.http.internal;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import java.nio.charset.StandardCharsets;
6+
import org.junit.jupiter.api.Test;
7+
8+
class ProblemDetailRendererTest {
9+
10+
@Test
11+
void rendersAllFieldsWhenPresent() {
12+
ProblemDetail pd =
13+
new ProblemDetail("about:blank", "Bad Request", 400, "expected string", "/x", "type");
14+
assertThat(asString(ProblemDetailRenderer.renderJson(pd)))
15+
.isEqualTo(
16+
"{\"type\":\"about:blank\",\"title\":\"Bad Request\",\"status\":400,"
17+
+ "\"detail\":\"expected string\",\"pointer\":\"/x\",\"keyword\":\"type\"}");
18+
}
19+
20+
@Test
21+
void omitsNullPointerAndKeyword() {
22+
ProblemDetail pd =
23+
new ProblemDetail("about:blank", "Unauthorized", 401, "missing token", null, null);
24+
assertThat(asString(ProblemDetailRenderer.renderJson(pd)))
25+
.isEqualTo(
26+
"{\"type\":\"about:blank\",\"title\":\"Unauthorized\",\"status\":401,"
27+
+ "\"detail\":\"missing token\"}");
28+
}
29+
30+
@Test
31+
void omitsNullDetail() {
32+
ProblemDetail pd = new ProblemDetail("about:blank", "Not Found", 404, null, null, null);
33+
assertThat(asString(ProblemDetailRenderer.renderJson(pd)))
34+
.isEqualTo("{\"type\":\"about:blank\",\"title\":\"Not Found\",\"status\":404}");
35+
}
36+
37+
@Test
38+
void escapesQuoteAndBackslashInDetail() {
39+
ProblemDetail pd = new ProblemDetail("about:blank", "Bad Request", 400, "a\"b\\c", null, null);
40+
assertThat(asString(ProblemDetailRenderer.renderJson(pd)))
41+
.isEqualTo(
42+
"{\"type\":\"about:blank\",\"title\":\"Bad Request\",\"status\":400,"
43+
+ "\"detail\":\"a\\\"b\\\\c\"}");
44+
}
45+
46+
@Test
47+
void escapesNamedControlCharsInDetail() {
48+
ProblemDetail pd =
49+
new ProblemDetail("about:blank", "Bad Request", 400, "\b\f\n\r\t", null, null);
50+
assertThat(asString(ProblemDetailRenderer.renderJson(pd)))
51+
.isEqualTo(
52+
"{\"type\":\"about:blank\",\"title\":\"Bad Request\",\"status\":400,"
53+
+ "\"detail\":\"\\b\\f\\n\\r\\t\"}");
54+
}
55+
56+
@Test
57+
void escapesUnnamedControlCharsAsHexUnicode() {
58+
ProblemDetail pd = new ProblemDetail("about:blank", "Bad Request", 400, "", null, null);
59+
assertThat(asString(ProblemDetailRenderer.renderJson(pd)))
60+
.isEqualTo(
61+
"{\"type\":\"about:blank\",\"title\":\"Bad Request\",\"status\":400,"
62+
+ "\"detail\":\"\\u0001\\u001f\"}");
63+
}
64+
65+
@Test
66+
void passesThroughNonAsciiCharactersVerbatim() {
67+
ProblemDetail pd = new ProblemDetail("about:blank", "Bad Request", 400, "café-é", null, null);
68+
assertThat(asString(ProblemDetailRenderer.renderJson(pd)))
69+
.isEqualTo(
70+
"{\"type\":\"about:blank\",\"title\":\"Bad"
71+
+ " Request\",\"status\":400,\"detail\":\"café-é\"}");
72+
}
73+
74+
private static String asString(byte[] bytes) {
75+
return new String(bytes, StandardCharsets.UTF_8);
76+
}
77+
}

0 commit comments

Comments
 (0)