honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix Broken HTTP Request Breaking Channel Closing (#45958) (#45973) 

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
This commit is contained in:
Armin Braun 2019-08-26 16:20:58 +02:00 committed by GitHub
parent d78bc487b4
commit af2bd75def
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 24 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
server/src/main/java/org/elasticsearch/http/DefaultRestChannel.java
View File

@ -168,12 +168,13 @@ public class DefaultRestChannel extends AbstractRestChannel implements RestChann
// Determine if the request connection should be closed on completion. // Determine if the request connection should be closed on completion.
private boolean isCloseConnection() { private boolean isCloseConnection() {
final boolean http10 = isHttp10(); try {
return CLOSE.equalsIgnoreCase(request.header(CONNECTION)) || (http10 && !KEEP_ALIVE.equalsIgnoreCase(request.header(CONNECTION))); final boolean http10 = request.getHttpRequest().protocolVersion() == HttpRequest.HttpVersion.HTTP_1_0;
} return CLOSE.equalsIgnoreCase(request.header(CONNECTION))
|| (http10 && !KEEP_ALIVE.equalsIgnoreCase(request.header(CONNECTION)));
// Determine if the request protocol version is HTTP 1.0 } catch (Exception e) {
private boolean isHttp10() { // In case we fail to parse the http protocol version out of the request we always close the connection
return request.getHttpRequest().protocolVersion() == HttpRequest.HttpVersion.HTTP_1_0; return true;
}
} }
} }

23
server/src/test/java/org/elasticsearch/http/DefaultRestChannelTests.java
View File

@ -57,6 +57,7 @@ import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Objects; import java.util.Objects;
import java.util.function.Supplier;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasItem;
@ -272,8 +273,13 @@ public class DefaultRestChannelTests extends ESTestCase {
public void testConnectionClose() throws Exception { public void testConnectionClose() throws Exception {
final Settings settings = Settings.builder().build(); final Settings settings = Settings.builder().build();
final HttpRequest httpRequest; final HttpRequest httpRequest;
final boolean close = randomBoolean(); final boolean brokenRequest = randomBoolean();
if (randomBoolean()) { final boolean close = brokenRequest || randomBoolean();
if (brokenRequest) {
httpRequest = new TestRequest(() -> {
throw new IllegalArgumentException("Can't parse HTTP version");
}, RestRequest.Method.GET, "/");
} else if (randomBoolean()) {
httpRequest = new TestRequest(HttpRequest.HttpVersion.HTTP_1_1, RestRequest.Method.GET, "/"); httpRequest = new TestRequest(HttpRequest.HttpVersion.HTTP_1_1, RestRequest.Method.GET, "/");
if (close) { if (close) {
httpRequest.getHeaders().put(DefaultRestChannel.CONNECTION, Collections.singletonList(DefaultRestChannel.CLOSE)); httpRequest.getHeaders().put(DefaultRestChannel.CONNECTION, Collections.singletonList(DefaultRestChannel.CLOSE));
@ -399,18 +405,21 @@ public class DefaultRestChannelTests extends ESTestCase {
private static class TestRequest implements HttpRequest { private static class TestRequest implements HttpRequest {
private final HttpVersion version; private final Supplier<HttpVersion> version;
private final RestRequest.Method method; private final RestRequest.Method method;
private final String uri; private final String uri;
private HashMap<String, List<String>> headers = new HashMap<>(); private HashMap<String, List<String>> headers = new HashMap<>();
private TestRequest(HttpVersion version, RestRequest.Method method, String uri) { private TestRequest(Supplier<HttpVersion> versionSupplier, RestRequest.Method method, String uri) {
this.version = versionSupplier;
this.version = version;
this.method = method; this.method = method;
this.uri = uri; this.uri = uri;
} }
private TestRequest(HttpVersion version, RestRequest.Method method, String uri) {
this(() -> version, method, uri);
}
@Override @Override
public RestRequest.Method method() { public RestRequest.Method method() {
return method; return method;
@ -438,7 +447,7 @@ public class DefaultRestChannelTests extends ESTestCase {
@Override @Override
public HttpVersion protocolVersion() { public HttpVersion protocolVersion() {
return version; return version.get();
} }
@Override @Override