Skip to content

Commit

Permalink
3.x: Improves handling of invalid Accept types (#8679)
Browse files Browse the repository at this point in the history
* Improves handling of invalid Accept types by returning 400 Bad Request instead of 500 Internal Error. New tests. See issue #8672.

* Moved test to metrics module to avoid cyclic dependencies.

Signed-off-by: Santiago Pericas-Geertsen <[email protected]>

---------

Signed-off-by: Santiago Pericas-Geertsen <[email protected]>
  • Loading branch information
spericas authored Apr 18, 2024
1 parent 7a0b24b commit 25a6820
Show file tree
Hide file tree
Showing 3 changed files with 194 additions and 20 deletions.
167 changes: 167 additions & 0 deletions metrics/metrics/src/test/java/io/helidon/metrics/BadRequestTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
/*
* Copyright (c) 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.helidon.metrics;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.net.Socket;
import java.nio.charset.StandardCharsets;
import java.util.LinkedList;
import java.util.List;

import io.helidon.common.http.Http;
import io.helidon.webserver.Handler;
import io.helidon.webserver.Routing;
import io.helidon.webserver.WebServer;
import io.helidon.media.jsonp.JsonpSupport;

import jakarta.json.JsonObject;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;

/**
* Tests bad types in Accept header. Cannot be placed in webserver module due to
* cyclic dependencies. It needs to use JSON and Metrics, and requires a "plain"
* HTTP client to issue the request.
*/
public class BadRequestTest {

private static WebServer server;

@BeforeAll
static void createAndStartServer() {
server = WebServer.builder()
.addMediaSupport(JsonpSupport.create())
.addRouting(Routing.builder()
.register(MetricsSupport.create())
.post("/echo", Handler.create(JsonObject.class,
(req, res, entity) -> res.status(Http.Status.OK_200).send(entity))))
.build();
server.start().await();
}

@AfterAll
static void stopServer() {
server.shutdown().await();
}

@Test
void testBadAcceptType() throws Exception {
try (SimpleHttpSocketClient c = new SimpleHttpSocketClient(server)) {
c.request("POST", "/echo", List.of("Accept: application.json"), "{}");
String result = c.receive();
assertThat(result, containsString("400 Bad Request"));
}
}

@Test
void testBadAcceptTypeMetrics() throws Exception {
try (SimpleHttpSocketClient c = new SimpleHttpSocketClient(server)) {
c.request("GET", "/metrics", List.of("Accept: application.json"), "");
String result = c.receive();
assertThat(result, containsString("400 Bad Request"));
}
}

/**
* Simple HTTP socket client to submit HTTP requests. Used by this class to create
* a request with an invalid Accept header.
*/
static class SimpleHttpSocketClient implements AutoCloseable {
static final String EOL = "\r\n";
private final Socket socket;
private final BufferedReader socketReader;

SimpleHttpSocketClient(WebServer webServer) throws IOException {
socket = new Socket("localhost", webServer.port());
socket.setSoTimeout(10000);
socketReader = new BufferedReader(new InputStreamReader(socket.getInputStream(), StandardCharsets.UTF_8));
}

String receive() throws IOException {
StringBuilder sb = new StringBuilder();
String t;
boolean ending = false;
int contentLength = -1;
while ((t = socketReader.readLine()) != null) {
if (t.toLowerCase().startsWith("content-length")) {
int k = t.indexOf(':');
contentLength = Integer.parseInt(t.substring(k + 1).trim());
}
sb.append(t).append("\n");
if ("".equalsIgnoreCase(t) && contentLength >= 0) {
char[] content = new char[contentLength];
socketReader.read(content);
sb.append(content);
break;
}
if (ending && "".equalsIgnoreCase(t)) {
break;
}
if (!ending && ("0".equalsIgnoreCase(t))) {
ending = true;
}
}
return sb.toString();
}

void request(String method, String path, Iterable<String> headers, String payload)
throws IOException {
List<String> usedHeaders = new LinkedList<>();
if (headers != null) {
headers.forEach(usedHeaders::add);
}
usedHeaders.add(0, "Host: " + "localhost");
PrintWriter pw = new PrintWriter(new OutputStreamWriter(socket.getOutputStream(), StandardCharsets.UTF_8));
pw.print(method);
pw.print(" ");
pw.print(path);
pw.print(" ");
pw.print("http");
pw.print(EOL);
for (String header : usedHeaders) {
pw.print(header);
pw.print(EOL);
}
sendPayload(pw, payload);
pw.print(EOL);
pw.print(EOL);
pw.flush();
}

void sendPayload(PrintWriter pw, String payload) {
if (payload != null) {
pw.print("Content-Length: " + payload.length());
pw.print(EOL);
pw.print(EOL);
pw.print(payload);
pw.print(EOL);
}
}

@Override
public void close() throws Exception {
socket.close();
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2022 Oracle and/or its affiliates.
* Copyright (c) 2017, 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -118,7 +118,6 @@ public List<MediaType> acceptedTypes() {

if (acceptValues.size() == 1 && HUC_ACCEPT_DEFAULT.equals(acceptValues.get(0))) {
result = HUC_ACCEPT_DEFAULT_TYPES;

} else {
result = LazyList.create(acceptValues.stream()
.flatMap(h -> Utils.tokenize(',', "\"", false, h).stream())
Expand All @@ -145,29 +144,33 @@ public Optional<MediaType> bestAccepted(MediaType... mediaTypes) {
if (mediaTypes == null || mediaTypes.length == 0) {
return Optional.empty();
}
List<MediaType> accepts = acceptedTypes();
if (accepts == null || accepts.isEmpty()) {
return Optional.ofNullable(mediaTypes[0]);
}
try {
List<MediaType> accepts = acceptedTypes();
if (accepts == null || accepts.isEmpty()) {
return Optional.ofNullable(mediaTypes[0]);
}

double best = 0;
MediaType result = null;
for (MediaType mt : mediaTypes) {
if (mt != null) {
for (MediaType acc : accepts) {
double q = acc.qualityFactor();
if (q > best && acc.test(mt)) {
if (q == 1) {
return Optional.of(mt);
} else {
best = q;
result = mt;
double best = 0;
MediaType result = null;
for (MediaType mt : mediaTypes) {
if (mt != null) {
for (MediaType acc : accepts) {
double q = acc.qualityFactor();
if (q > best && acc.test(mt)) {
if (q == 1) {
return Optional.of(mt);
} else {
best = q;
result = mt;
}
}
}
}
}
return Optional.ofNullable(result);
} catch (IllegalArgumentException e) {
throw new BadRequestException("Unable to parse Accept header", e);
}
return Optional.ofNullable(result);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2022 Oracle and/or its affiliates.
* Copyright (c) 2017, 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -181,6 +181,10 @@ public <T> Single<ServerResponse> send(T content) {
sendPublisher.subscribe(bareResponse);
}, content == null);
return whenSent();
} catch (IllegalStateException e) {
eventListener.finish();
throw e.getCause() instanceof IllegalArgumentException
? new BadRequestException("Unable to process request", e) : e;
} catch (RuntimeException | Error e) {
eventListener.finish();
throw e;
Expand Down

0 comments on commit 25a6820

Please sign in to comment.