Skip to content

Commit 4669144

Browse files
committed
New config section in listeners for error handling. Default settings prevent any entity from being returned to avoid reflecting any data from a request. Default settings can be modified to return safe messages and to log all messages.
1 parent c325930 commit 4669144

File tree

9 files changed

+291
-18
lines changed

9 files changed

+291
-18
lines changed

webserver/http2/src/main/java/io/helidon/webserver/http2/Http2ServerStream.java

+32-7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2022, 2024 Oracle and/or its affiliates.
2+
* Copyright (c) 2022, 2025 Oracle and/or its affiliates.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -61,12 +61,14 @@
6161
import io.helidon.http.http2.WindowSize;
6262
import io.helidon.webserver.CloseConnectionException;
6363
import io.helidon.webserver.ConnectionContext;
64+
import io.helidon.webserver.ErrorHandling;
6465
import io.helidon.webserver.Router;
6566
import io.helidon.webserver.ServerConnectionException;
6667
import io.helidon.webserver.http.HttpRouting;
6768
import io.helidon.webserver.http2.spi.Http2SubProtocolSelector;
6869
import io.helidon.webserver.http2.spi.SubProtocolResult;
6970

71+
import static java.lang.System.Logger.Level.DEBUG;
7072
import static java.lang.System.Logger.Level.TRACE;
7173

7274
/**
@@ -307,28 +309,51 @@ public void run() {
307309
writer.write(rst.toFrameData(serverSettings, streamId, Http2Flag.NoFlags.create()));
308310
// no sense in throwing an exception, as this is invoked from an executor service directly
309311
} catch (RequestException e) {
312+
// gather error handling properties
313+
ErrorHandling errorHandling = ctx.listenerContext()
314+
.config()
315+
.errorHandling()
316+
.orElse(null);
317+
boolean includeEntity = false;
318+
boolean logAllMessages = false;
319+
if (errorHandling != null) {
320+
includeEntity = errorHandling.includeEntity();
321+
logAllMessages = errorHandling.logAllMessages();
322+
}
323+
324+
// log message in DEBUG mode
325+
if (LOGGER.isLoggable(DEBUG) && (e.safeMessage() || logAllMessages)) {
326+
LOGGER.log(DEBUG, e);
327+
}
328+
329+
// create message to return based on settings
330+
String message = null;
331+
if (includeEntity) {
332+
message = e.safeMessage() ? e.getMessage() : "Bad request, see server log for more information";
333+
}
334+
310335
DirectHandler handler = ctx.listenerContext()
311336
.directHandlers()
312337
.handler(e.eventType());
313338
DirectHandler.TransportResponse response = handler.handle(e.request(),
314339
e.eventType(),
315340
e.status(),
316341
e.responseHeaders(),
317-
e);
342+
message);
318343

319344
ServerResponseHeaders headers = response.headers();
320-
byte[] message = response.entity().orElse(BufferData.EMPTY_BYTES);
321-
if (message.length != 0) {
322-
headers.set(HeaderValues.create(HeaderNames.CONTENT_LENGTH, String.valueOf(message.length)));
345+
byte[] entity = response.entity().orElse(BufferData.EMPTY_BYTES);
346+
if (entity.length != 0) {
347+
headers.set(HeaderValues.create(HeaderNames.CONTENT_LENGTH, String.valueOf(entity.length)));
323348
}
324349
Http2Headers http2Headers = Http2Headers.create(headers);
325-
if (message.length == 0) {
350+
if (entity.length == 0) {
326351
writer.writeHeaders(http2Headers,
327352
streamId,
328353
Http2Flag.HeaderFlags.create(Http2Flag.END_OF_HEADERS | Http2Flag.END_OF_STREAM),
329354
flowControl.outbound());
330355
} else {
331-
Http2FrameHeader dataHeader = Http2FrameHeader.create(message.length,
356+
Http2FrameHeader dataHeader = Http2FrameHeader.create(entity.length,
332357
Http2FrameTypes.DATA,
333358
Http2Flag.DataFlags.create(Http2Flag.END_OF_STREAM),
334359
streamId);

webserver/tests/webserver/src/test/java/io/helidon/webserver/tests/BadHostTest.java

+11-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2024 Oracle and/or its affiliates.
2+
* Copyright (c) 2024, 2025 Oracle and/or its affiliates.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -22,10 +22,13 @@
2222
import io.helidon.http.Status;
2323
import io.helidon.webclient.api.ClientResponseTyped;
2424
import io.helidon.webclient.http1.Http1Client;
25+
import io.helidon.webserver.ErrorHandling;
26+
import io.helidon.webserver.WebServerConfig;
2527
import io.helidon.webserver.http.HttpRouting;
2628
import io.helidon.webserver.http1.Http1Route;
2729
import io.helidon.webserver.testing.junit5.ServerTest;
2830
import io.helidon.webserver.testing.junit5.SetUpRoute;
31+
import io.helidon.webserver.testing.junit5.SetUpServer;
2932

3033
import org.junit.jupiter.api.Test;
3134

@@ -42,6 +45,13 @@ class BadHostTest {
4245
this.client = client;
4346
}
4447

48+
@SetUpServer
49+
static void setupServer(WebServerConfig.Builder builder) {
50+
builder.errorHandling(ErrorHandling.builder()
51+
.includeEntity(true) // enable error message entities
52+
.build());
53+
}
54+
4555
@SetUpRoute
4656
static void routing(HttpRouting.Builder builder) {
4757
builder.route(Http1Route.route(Method.GET,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
/*
2+
* Copyright (c) 2024, 2025 Oracle and/or its affiliates.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package io.helidon.webserver.tests;
18+
19+
import java.util.List;
20+
21+
import io.helidon.common.testing.http.junit5.SocketHttpClient;
22+
import io.helidon.http.HttpPrologue;
23+
import io.helidon.http.Method;
24+
import io.helidon.webclient.http1.Http1Client;
25+
import io.helidon.webserver.http.HttpRouting;
26+
import io.helidon.webserver.http1.Http1Route;
27+
import io.helidon.webserver.testing.junit5.ServerTest;
28+
import io.helidon.webserver.testing.junit5.SetUpRoute;
29+
30+
import org.junit.jupiter.api.Test;
31+
32+
import static org.hamcrest.CoreMatchers.containsString;
33+
import static org.hamcrest.CoreMatchers.is;
34+
import static org.hamcrest.CoreMatchers.not;
35+
import static org.hamcrest.MatcherAssert.assertThat;
36+
37+
/**
38+
* Verifies that 400 bad request is returned when a bad prologue is sent. Response
39+
* entities must be empty given that {@link io.helidon.webserver.ErrorHandling#includeEntity()}
40+
* is {@code false} by default.
41+
*/
42+
@ServerTest
43+
class BadPrologueNoEntityTest {
44+
private final Http1Client client;
45+
private final SocketHttpClient socketClient;
46+
47+
BadPrologueNoEntityTest(Http1Client client, SocketHttpClient socketClient) {
48+
this.client = client;
49+
this.socketClient = socketClient;
50+
}
51+
52+
@SetUpRoute
53+
static void routing(HttpRouting.Builder builder) {
54+
builder.route(Http1Route.route(Method.GET,
55+
"/",
56+
(req, res) -> {
57+
HttpPrologue prologue = req.prologue();
58+
String fragment = prologue.fragment().hasValue()
59+
? prologue.fragment().rawValue()
60+
: "";
61+
res.send("path: " + prologue.uriPath().rawPath()
62+
+ ", query: " + prologue.query().rawValue()
63+
+ ", fragment: " + fragment);
64+
}));
65+
}
66+
67+
@Test
68+
void testOk() {
69+
String response = client.method(Method.GET)
70+
.requestEntity(String.class);
71+
72+
assertThat(response, is("path: /, query: , fragment: "));
73+
}
74+
75+
@Test
76+
void testBadQuery() {
77+
String response = socketClient.sendAndReceive(Method.GET,
78+
"/?a=<a%20href='/bad-uri.com'/>bad</a>",
79+
null,
80+
List.of());
81+
82+
assertThat(response, containsString("400 Bad Request"));
83+
// beginning of message to the first double quote
84+
assertThat(response, not(containsString("Query contains invalid char: ")));
85+
// end of message from double quote, index of bad char, and bad char
86+
assertThat(response, not(containsString(", index: 2, char: 0x3c")));
87+
assertThat(response, not(containsString(">")));
88+
}
89+
90+
@Test
91+
void testBadQueryCurly() {
92+
String response = socketClient.sendAndReceive(Method.GET,
93+
"/?name=test1{{",
94+
null,
95+
List.of());
96+
97+
assertThat(response, containsString("400 Bad Request"));
98+
// beginning of message to the first double quote
99+
assertThat(response, not(containsString("Query contains invalid char: ")));
100+
// end of message from double quote, index of bad char, and bad char
101+
assertThat(response, not(containsString(", index: 10, char: 0x7b")));
102+
}
103+
104+
@Test
105+
void testBadPath() {
106+
String response = socketClient.sendAndReceive(Method.GET,
107+
"/name{{{{{{{Sdsds<Dhttps:--www.example.com",
108+
null,
109+
List.of());
110+
111+
assertThat(response, containsString("400 Bad Request"));
112+
// for path we are on the safe side, and never return it back (even HTML encoded)
113+
assertThat(response, not(containsString("Bad request, see server log for more information")));
114+
}
115+
116+
@Test
117+
void testBadFragment() {
118+
String response = socketClient.sendAndReceive(Method.GET,
119+
"/?a=b#invalid-fragment>",
120+
null,
121+
List.of());
122+
123+
assertThat(response, containsString("400 Bad Request"));
124+
// beginning of message to the first double quote
125+
assertThat(response, not(containsString("Fragment contains invalid char: ")));
126+
// end of message from double quote, index of bad char, and bad char
127+
assertThat(response, not(containsString(", index: 16, char: 0x3e")));
128+
assertThat(response, not(containsString(">")));
129+
}
130+
}

webserver/tests/webserver/src/test/java/io/helidon/webserver/tests/BadPrologueTest.java

+16-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2024 Oracle and/or its affiliates.
2+
* Copyright (c) 2024, 2025 Oracle and/or its affiliates.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -22,10 +22,13 @@
2222
import io.helidon.http.HttpPrologue;
2323
import io.helidon.http.Method;
2424
import io.helidon.webclient.http1.Http1Client;
25+
import io.helidon.webserver.ErrorHandling;
26+
import io.helidon.webserver.WebServerConfig;
2527
import io.helidon.webserver.http.HttpRouting;
2628
import io.helidon.webserver.http1.Http1Route;
2729
import io.helidon.webserver.testing.junit5.ServerTest;
2830
import io.helidon.webserver.testing.junit5.SetUpRoute;
31+
import io.helidon.webserver.testing.junit5.SetUpServer;
2932

3033
import org.junit.jupiter.api.Test;
3134

@@ -34,6 +37,11 @@
3437
import static org.hamcrest.CoreMatchers.not;
3538
import static org.hamcrest.MatcherAssert.assertThat;
3639

40+
/**
41+
* Verifies that 400 bad request is returned when a bad prologue is sent. Enables
42+
* entities in {@link io.helidon.webserver.ErrorHandling} to get details about
43+
* the errors in entities.
44+
*/
3745
@ServerTest
3846
class BadPrologueTest {
3947
private final Http1Client client;
@@ -44,6 +52,13 @@ class BadPrologueTest {
4452
this.socketClient = socketClient;
4553
}
4654

55+
@SetUpServer
56+
static void setupServer(WebServerConfig.Builder builder) {
57+
builder.errorHandling(ErrorHandling.builder()
58+
.includeEntity(true) // enable error message entities
59+
.build());
60+
}
61+
4762
@SetUpRoute
4863
static void routing(HttpRouting.Builder builder) {
4964
builder.route(Http1Route.route(Method.GET,

webserver/tests/webserver/src/test/java/io/helidon/webserver/tests/ContentEncodingDisabledTest.java

+9-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2023 Oracle and/or its affiliates.
2+
* Copyright (c) 2023, 2025 Oracle and/or its affiliates.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -20,6 +20,7 @@
2020
import io.helidon.http.Status;
2121
import io.helidon.webclient.http1.Http1Client;
2222
import io.helidon.webclient.http1.Http1ClientResponse;
23+
import io.helidon.webserver.ErrorHandling;
2324
import io.helidon.webserver.WebServerConfig;
2425
import io.helidon.webserver.http1.Http1Config;
2526
import io.helidon.webserver.http1.Http1ConnectionSelector;
@@ -49,6 +50,10 @@ class ContentEncodingDisabledTest extends ContentEncodingDisabledAbstract {
4950
super(client);
5051
}
5152

53+
@SetUpServer
54+
static void setupServer(WebServerConfig.Builder builder) {
55+
56+
}
5257
@SetUpServer
5358
static void server(WebServerConfig.Builder server) {
5459
ServerConnectionSelector http1 = Http1ConnectionSelector.builder()
@@ -58,6 +63,9 @@ static void server(WebServerConfig.Builder server) {
5863
server.addConnectionSelector(http1)
5964
// Content encoding needs to be completely disabled
6065
.contentEncoding(emptyEncodingContext());
66+
server.errorHandling(ErrorHandling.builder()
67+
.includeEntity(true) // enable error message entities
68+
.build());
6169
}
6270

6371
@Test

webserver/tests/webserver/src/test/java/io/helidon/webserver/tests/ExceptionMessageTest.java

+11-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2023 Oracle and/or its affiliates.
2+
* Copyright (c) 2023, 2025 Oracle and/or its affiliates.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -21,7 +21,10 @@
2121
import io.helidon.common.testing.http.junit5.SocketHttpClient;
2222
import io.helidon.http.Method;
2323
import io.helidon.http.Status;
24+
import io.helidon.webserver.ErrorHandling;
25+
import io.helidon.webserver.WebServerConfig;
2426
import io.helidon.webserver.testing.junit5.ServerTest;
27+
import io.helidon.webserver.testing.junit5.SetUpServer;
2528

2629
import org.junit.jupiter.api.Test;
2730

@@ -43,6 +46,13 @@ class ExceptionMessageTest {
4346
this.socketClient = socketClient;
4447
}
4548

49+
@SetUpServer
50+
static void setupServer(WebServerConfig.Builder builder) {
51+
builder.errorHandling(ErrorHandling.builder()
52+
.includeEntity(true) // enable error message entities
53+
.build());
54+
}
55+
4656
@Test
4757
void testNoUrlReflect() {
4858
String path = "/anyjavascript%3a/*%3c/script%3e%3cimg/onerror%3d'\\''"

0 commit comments

Comments
 (0)