Skip to content

Commit

Permalink
#8077 Client keep alive fix
Browse files Browse the repository at this point in the history
  • Loading branch information
danielkec committed Dec 4, 2023
1 parent 8ae04ed commit e6aed53
Show file tree
Hide file tree
Showing 2 changed files with 152 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class Http1ClientResponseImpl implements Http1ClientResponse {
private final CompletableFuture<io.helidon.http.Headers> trailers = new CompletableFuture<>();
private boolean entityRequested;
private long entityLength;
private volatile boolean entityFullyRead = false;

Http1ClientResponseImpl(HttpClientConfig clientConfig,
Status responseStatus,
Expand All @@ -96,7 +97,7 @@ class Http1ClientResponseImpl implements Http1ClientResponse {
this.whenComplete = whenComplete;

if (responseHeaders.contains(HeaderNames.CONTENT_LENGTH)) {
this.entityLength = Long.parseLong(responseHeaders.get(HeaderNames.CONTENT_LENGTH).value());
this.entityLength = Long.parseLong(responseHeaders.get(HeaderNames.CONTENT_LENGTH).get());
} else if (responseHeaders.contains(HeaderValues.TRANSFER_ENCODING_CHUNKED)) {
this.entityLength = ENTITY_LENGTH_CHUNKED;
}
Expand Down Expand Up @@ -166,10 +167,10 @@ public void close() {
} else if (entityLength == ENTITY_LENGTH_CHUNKED) {
if (hasTrailers) {
readTrailers();
connection.releaseResource();
} else {
connection.closeResource();
}
connection.releaseResource();
} else if (entityFullyRead) {
connection.releaseResource();
} else {
connection.closeResource();
}
Expand Down Expand Up @@ -208,13 +209,18 @@ private ReadableEntity entity(ClientRequestHeaders requestHeaders,
}
return ClientResponseEntity.create(
this::readBytes,
this::close,
this::entityFullyRead,
requestHeaders,
responseHeaders,
mediaContext
);
}

private void entityFullyRead() {
this.entityFullyRead = true;
this.close();
}

private void readTrailers() {
this.trailers.complete(Http1HeadersParser.readHeaders(connection.reader(), 1024, true));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
/*
* Copyright (c) 2023 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.webclient.tests;

import io.helidon.http.HeaderValues;
import io.helidon.http.Method;
import io.helidon.http.Status;
import io.helidon.webclient.api.WebClient;
import io.helidon.webserver.http.HttpRouting;
import io.helidon.webserver.testing.junit5.ServerTest;
import io.helidon.webserver.testing.junit5.SetUpRoute;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import java.nio.charset.StandardCharsets;

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


@ServerTest
class KeepAliveTest {

private final WebClient client;

@SetUpRoute
static void route(HttpRouting.Builder router) {
router.any("/entity-port", (req, res) -> res
.send(String.valueOf(req.remotePeer().port())));
router.any("/entity-chunked", (req, res) -> {
res.header(HeaderValues.TRANSFER_ENCODING_CHUNKED);
try (var os = res.outputStream()) {
os.write(String.valueOf(req.remotePeer().port()).getBytes(StandardCharsets.UTF_8));
}
});
router.any("/empty-chunked", (req, res) -> {
res.header(HeaderValues.TRANSFER_ENCODING_CHUNKED);
res.status(req.remotePeer().port());
res.outputStream().close();
});
router.any("/empty", (req, res) -> res.status(req.remotePeer().port()).send());
}

KeepAliveTest(WebClient client) {
this.client = client;
}

@ParameterizedTest
@ValueSource(strings = {"GET", "PUT", "POST", "DELETE"})
void noEntity(String method) {
var m = Method.create(method);
Status port;
try(var res = client.method(m).path("/empty").request()){
port = res.status();
}

try(var res2 = client.method(m).path("/empty").request()){
assertThat(res2.status(), is(port));
}
}

@ParameterizedTest
@ValueSource(strings = {"GET", "PUT", "POST", "DELETE"})
void noReqEntity(String method) {
var m = Method.create(method);
String port;
try(var res = client.method(m).path("/entity-port").request()){
port = res.as(String.class);
}
try(var res2 = client.method(m).path("/entity-port").request()){
assertThat(res2.as(String.class), is(port));
}
}

@ParameterizedTest
@ValueSource(strings = {"GET", "PUT", "POST", "DELETE"})
void noReqEntityChunked(String method) {
var m = Method.create(method);
String port;
try(var res = client.method(m).path("/entity-chunked").request()){
port = res.as(String.class);
}
try(var res2 = client.method(m).path("/entity-chunked").request()){
assertThat(res2.as(String.class), is(port));
}
}

@ParameterizedTest
@ValueSource(strings = {"PUT", "POST"})
void noResEntity(String method) {
var m = Method.create(method);
Status port;
try(var res = client.method(m).path("/empty").submit("Hello")){
port = res.status();
}
try(var res2 = client.method(m).path("/empty").submit("Hello")){
assertThat(res2.status(), is(port));
}
}

@ParameterizedTest
@ValueSource(strings = {"PUT", "POST"})
void noResEntityChunked(String method) {
var m = Method.create(method);
Status port;
try(var res = client.method(m).path("/empty-chunked").submit("Hello")){
port = res.status();
}
try(var res2 = client.method(m).path("/empty-chunked").submit("Hello")){
assertThat(res2.status(), is(port));
}
}

@ParameterizedTest
@ValueSource(strings = {"PUT", "POST"})
void withReqResEntity(String method) {
var m = Method.create(method);
String port;
try(var res = client.method(m).path("/entity-port").submit("Hello")){
port = res.as(String.class);
}
try(var res2 = client.method(m).path("/entity-port").submit("Hello")){
assertThat(res2.as(String.class), is(port));
}
}
}

0 comments on commit e6aed53

Please sign in to comment.