Skip to content

Commit e003b70

Browse files
Fix Broken HTTP Request Breaking Channel Closing (#45958)
This is essentially the same issue fixed in #43362 but for http request version instead of the request method. We have to deal with the case of not being able to parse the request version, otherwise channel closing fails. Fixes #43850
1 parent 2599e23 commit e003b70

File tree

2 files changed

+24
-14
lines changed

2 files changed

+24
-14
lines changed

server/src/main/java/org/elasticsearch/http/DefaultRestChannel.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -168,12 +168,13 @@ private void addCookies(HttpResponse response) {
168168

169169
// Determine if the request connection should be closed on completion.
170170
private boolean isCloseConnection() {
171-
final boolean http10 = isHttp10();
172-
return CLOSE.equalsIgnoreCase(request.header(CONNECTION)) || (http10 && !KEEP_ALIVE.equalsIgnoreCase(request.header(CONNECTION)));
173-
}
174-
175-
// Determine if the request protocol version is HTTP 1.0
176-
private boolean isHttp10() {
177-
return request.getHttpRequest().protocolVersion() == HttpRequest.HttpVersion.HTTP_1_0;
171+
try {
172+
final boolean http10 = request.getHttpRequest().protocolVersion() == HttpRequest.HttpVersion.HTTP_1_0;
173+
return CLOSE.equalsIgnoreCase(request.header(CONNECTION))
174+
|| (http10 && !KEEP_ALIVE.equalsIgnoreCase(request.header(CONNECTION)));
175+
} catch (Exception e) {
176+
// In case we fail to parse the http protocol version out of the request we always close the connection
177+
return true;
178+
}
178179
}
179180
}

server/src/test/java/org/elasticsearch/http/DefaultRestChannelTests.java

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import java.util.List;
5858
import java.util.Map;
5959
import java.util.Objects;
60+
import java.util.function.Supplier;
6061

6162
import static org.hamcrest.Matchers.equalTo;
6263
import static org.hamcrest.Matchers.hasItem;
@@ -272,8 +273,13 @@ public void testReleaseInListener() throws IOException {
272273
public void testConnectionClose() throws Exception {
273274
final Settings settings = Settings.builder().build();
274275
final HttpRequest httpRequest;
275-
final boolean close = randomBoolean();
276-
if (randomBoolean()) {
276+
final boolean brokenRequest = randomBoolean();
277+
final boolean close = brokenRequest || randomBoolean();
278+
if (brokenRequest) {
279+
httpRequest = new TestRequest(() -> {
280+
throw new IllegalArgumentException("Can't parse HTTP version");
281+
}, RestRequest.Method.GET, "/");
282+
} else if (randomBoolean()) {
277283
httpRequest = new TestRequest(HttpRequest.HttpVersion.HTTP_1_1, RestRequest.Method.GET, "/");
278284
if (close) {
279285
httpRequest.getHeaders().put(DefaultRestChannel.CONNECTION, Collections.singletonList(DefaultRestChannel.CLOSE));
@@ -399,18 +405,21 @@ private TestResponse executeRequest(final Settings settings, final String origin
399405

400406
private static class TestRequest implements HttpRequest {
401407

402-
private final HttpVersion version;
408+
private final Supplier<HttpVersion> version;
403409
private final RestRequest.Method method;
404410
private final String uri;
405411
private HashMap<String, List<String>> headers = new HashMap<>();
406412

407-
private TestRequest(HttpVersion version, RestRequest.Method method, String uri) {
408-
409-
this.version = version;
413+
private TestRequest(Supplier<HttpVersion> versionSupplier, RestRequest.Method method, String uri) {
414+
this.version = versionSupplier;
410415
this.method = method;
411416
this.uri = uri;
412417
}
413418

419+
private TestRequest(HttpVersion version, RestRequest.Method method, String uri) {
420+
this(() -> version, method, uri);
421+
}
422+
414423
@Override
415424
public RestRequest.Method method() {
416425
return method;
@@ -438,7 +447,7 @@ public List<String> strictCookies() {
438447

439448
@Override
440449
public HttpVersion protocolVersion() {
441-
return version;
450+
return version.get();
442451
}
443452

444453
@Override

0 commit comments

Comments
 (0)