Skip to content

Commit

Permalink
Gracefully reject HTTP 1.0 connections that are either ignored by all…
Browse files Browse the repository at this point in the history
… our providers or directly routed to our 1.1 provider. We always route to 1.1 if only a single provider is found. Returns HTTP error code 505 (HTTP version not supported) instead of 400 as before.
  • Loading branch information
spericas committed Jan 29, 2025
1 parent cffb8e2 commit 546497f
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 14 deletions.
22 changes: 11 additions & 11 deletions http/http/src/main/java/io/helidon/http/DirectHandler.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, 2023 Oracle and/or its affiliates.
* Copyright (c) 2021, 2025 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 @@ -85,15 +85,11 @@ default TransportResponse handle(TransportRequest request,
Throwable thrown,
System.Logger logger) {
if (thrown instanceof RequestException re) {
if (re.safeMessage()) {
return handle(request, eventType, defaultStatus, responseHeaders, thrown.getMessage());
} else {
if (logger != null) {
logger.log(Level.DEBUG, thrown);
}
return handle(request, eventType, defaultStatus, responseHeaders,
"Bad request, see server log for more information");
if (logger != null) {
logger.log(Level.DEBUG, thrown);
}
return handle(request, eventType, defaultStatus, responseHeaders,
re.safeMessage() ? thrown.getMessage() : "Bad request, see server log for more information");
}
return handle(request, eventType, defaultStatus, responseHeaders, thrown.getMessage());
}
Expand Down Expand Up @@ -188,7 +184,11 @@ enum EventType {
/**
* Other type, please specify expected status code.
*/
OTHER(Status.INTERNAL_SERVER_ERROR_500, true);
OTHER(Status.INTERNAL_SERVER_ERROR_500, true),
/**
* HTTP version not supported.
*/
HTTP_VERSION_NOT_SUPPORTED(Status.HTTP_VERSION_NOT_SUPPORTED_505, false);

private final Status defaultStatus;
private final boolean keepAlive;
Expand All @@ -210,7 +210,7 @@ public Status defaultStatus() {
/**
* Whether keep alive should be maintained for this event type.
*
* @return whether to keep connectino alive
* @return whether to keep connection alive
*/
public boolean keepAlive() {
return keepAlive;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, 2024 Oracle and/or its affiliates.
* Copyright (c) 2022, 2025 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 @@ -30,6 +30,7 @@
import javax.net.ssl.SSLSocket;

import io.helidon.common.buffers.BufferData;
import io.helidon.common.buffers.Bytes;
import io.helidon.common.buffers.DataReader;
import io.helidon.common.buffers.DataWriter;
import io.helidon.common.concurrency.limits.Limit;
Expand All @@ -40,6 +41,7 @@
import io.helidon.common.socket.TlsSocket;
import io.helidon.common.task.InterruptableTask;
import io.helidon.common.tls.Tls;
import io.helidon.http.DirectHandler;
import io.helidon.http.HttpException;
import io.helidon.http.RequestException;
import io.helidon.webserver.spi.ServerConnection;
Expand All @@ -55,6 +57,7 @@
*/
class ConnectionHandler implements InterruptableTask<Void>, ConnectionContext {
private static final System.Logger LOGGER = System.getLogger(ConnectionHandler.class.getName());
private static final String HTTP_1_0 = "HTTP/1.0\r";

private final ListenerContext listenerContext;
// we must safely release the semaphore whenever this connection is finished, so other connections can be created!
Expand Down Expand Up @@ -163,6 +166,14 @@ public final void run() {
}

if (connection == null) {
// handle old HTTP 1.0 connections rejected by all providers
if (isHttp10Connection(reader)) {
throw RequestException.builder()
.type(DirectHandler.EventType.HTTP_VERSION_NOT_SUPPORTED)
.message("HTTP 1.0 is not supported, consider using HTTP 1.1")
.safeMessage(true)
.build();
}
throw new CloseConnectionException("No suitable connection provider");
}
activeConnections.put(socketsId, connection);
Expand Down Expand Up @@ -333,6 +344,17 @@ private void closeChannel() {
}
}

static boolean isHttp10Connection(DataReader reader) {
try {
reader.ensureAvailable();
} catch (DataReader.InsufficientDataAvailableException e) {
throw new CloseConnectionException("No data available", e);
}
BufferData request = reader.getBuffer(reader.available());
int lf = request.indexOf(Bytes.LF_BYTE);
return lf != -1 && request.readString(lf).endsWith(HTTP_1_0);
}

private static class MapExceptionDataSupplier implements Supplier<byte[]> {
private final HelidonSocket helidonSocket;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, 2024 Oracle and/or its affiliates.
* Copyright (c) 2022, 2025 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 @@ -44,6 +44,17 @@ public final class Http1Prologue {
| (long) '.' << 48
| (long) '1' << 56;
/*
The string HTTP/1.0: we don't support 1.0, but we detect it
*/
private static final long HTTP_1_0_LONG = 'H'
| 'T' << 8
| 'T' << 16
| 'P' << 24
| (long) '/' << 32
| (long) '1' << 40
| (long) '.' << 48
| (long) '0' << 56;
/*
GET string as int
*/
private static final int GET_INT = 'G'
Expand All @@ -63,6 +74,7 @@ public final class Http1Prologue {
| 'S' << 16
| 'T' << 24;
private static final String HTTP_1_1 = "HTTP/1.1";
private static final String HTTP_1_0 = "HTTP/1.0";

private final DataReader reader;
private final int maxLength;
Expand Down Expand Up @@ -206,6 +218,15 @@ Read HTTP Version (we only support HTTP/1.1
// we always use the same constant
//noinspection StringEquality
if (protocol != HTTP_1_1) {
//noinspection StringEquality
if (protocol == HTTP_1_0) { // be friendly rejecting 1.0
throw RequestException.builder()
.type(DirectHandler.EventType.HTTP_VERSION_NOT_SUPPORTED)
.request(DirectTransportRequest.create(HTTP_1_0, method.text(), path))
.message("HTTP 1.0 is not supported, consider using HTTP 1.1")
.safeMessage(true)
.build();
}
throw badRequest("Invalid protocol and/or version", method.text(), path, protocol, "");
}

Expand All @@ -232,6 +253,8 @@ private String readProtocol(byte[] bytes, int index) {
long word = Bytes.toWord(bytes, index);
if (word == HTTP_1_1_LONG) {
return HTTP_1_1;
} else if (word == HTTP_1_0_LONG) {
return HTTP_1_0;
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright (c) 2025 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.webserver;

import java.nio.charset.StandardCharsets;

import io.helidon.common.buffers.DataReader;

import org.junit.jupiter.api.Test;

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;

class ConnectionHandlerTest {

@Test
void testHttp10Prologue() {
DataReader reader = new DataReader(() -> "GET / HTTP/1.0\r\n".getBytes(StandardCharsets.US_ASCII));
assertThat(ConnectionHandler.isHttp10Connection(reader), is(true));
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2024 Oracle and/or its affiliates.
* Copyright (c) 2024, 2025 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 @@ -27,9 +27,11 @@

import org.junit.jupiter.api.Test;

import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.fail;

class Http1PrologueTest {
@Test
Expand All @@ -55,4 +57,19 @@ void testUriTooLong() {
assertThat(e.status(), is(Status.REQUEST_URI_TOO_LONG_414));
assertThat(e.eventType(), is(DirectHandler.EventType.BAD_REQUEST));
}

@Test
void testHttp10Error() {
DataReader reader = new DataReader(() -> "GET / HTTP/1.0\r\n".getBytes(StandardCharsets.US_ASCII));
Http1Prologue p = new Http1Prologue(reader, 100, false);

try {
p.readPrologue();
fail(); // exception not thrown
} catch (RequestException e) {
assertThat(e.status(), is(Status.HTTP_VERSION_NOT_SUPPORTED_505));
assertThat(e.safeMessage(), is(true));
assertThat(e.getMessage(), containsString("HTTP 1.0 is not supported"));
}
}
}

0 comments on commit 546497f

Please sign in to comment.