Fails to follow redirects on HEAD, PUT or POST requests with content
Closed this issue · 5 comments
Jetty 12.0.3, jetty-reactive-httpclient 4.0.1, java 21
Jetty http client fails to follow redirects when performing HEAD, PUT or POST requests with content (even if content is actually empty).
Check the following test code to reproduce, it contains several methods for various requests methods/reactive/not-reactive jetty, method failsSomeWithBody fails to indicate the case where jetty-reactive-httpclient does not work as expected.
import io.reactivex.Flowable;
import io.reactivex.rxjava3.core.Single;
import java.nio.ByteBuffer;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeoutException;
import org.eclipse.jetty.client.BytesRequestContent;
import org.eclipse.jetty.client.ContentResponse;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.io.Content;
import org.eclipse.jetty.reactive.client.ReactiveRequest;
import org.eclipse.jetty.reactive.client.ReactiveResponse;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.util.Callback;
import org.hamcrest.MatcherAssert;
import org.hamcrest.core.IsEqual;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.reactivestreams.Publisher;
public class RedirectTest {
private Server server;
private HttpClient client;
private int port;
@BeforeEach
void init() throws Exception {
this.server = new Server();
final ServerConnector conn = new ServerConnector(this.server);
this.server.addConnector(conn);
this.client = new HttpClient();
this.client.setFollowRedirects(true);
this.client.start();
this.server.setHandler(
new Handler.Abstract() {
@Override
public boolean handle(final Request request, final Response response, final Callback callback) throws Exception {
request.read().release();
if (request.getHttpURI().getPath().contains("final")) {
response.setStatus(200);
} else {
response.setStatus(307);
response.getHeaders().add("Location", "/final");
}
callback.succeeded();
return true;
}
});
this.server.start();
this.port = conn.getLocalPort();
}
@Test
void getWithBodyWorks() {
org.eclipse.jetty.client.Request rq = this.client.newRequest(
String.format("http://localhost:%s", this.port)
).method("GET");
ReactiveRequest build = ReactiveRequest.newBuilder(rq).content(
ReactiveRequest.Content.fromPublisher(
Flowable.just(Content.Chunk.from(ByteBuffer.wrap(new byte[]{}), true)), "*"
)
).build();
final Publisher<ReactiveResponse> publisher = build.response(ReactiveResponse.Content.discard());
int status = Single.fromPublisher(publisher)
.map(ReactiveResponse::getStatus)
.blockingGet();
MatcherAssert.assertThat(status, new IsEqual<>(200));
}
@ParameterizedTest
@ValueSource(strings = {"HEAD", "PUT", "POST"})
void failsSomeWithBody(final String method) {
org.eclipse.jetty.client.Request rq =
this.client.newRequest(String.format("http://localhost:%s/some/location", this.port)).method(method);
final ReactiveRequest build = ReactiveRequest.newBuilder(rq)
.content(
ReactiveRequest.Content.fromPublisher(
Flowable.just(Content.Chunk.from(ByteBuffer.wrap(new byte[]{}), true)),
"*"
)
).build();
final Publisher<ReactiveResponse> publisher = build.response(ReactiveResponse.Content.discard());
int status = Single.fromPublisher(publisher)
.map(ReactiveResponse::getStatus)
.blockingGet();
MatcherAssert.assertThat(status, new IsEqual<>(200));
}
@ParameterizedTest
@ValueSource(strings = {"HEAD", "PUT", "POST"})
void notReactiveWorksFine(final String method) throws ExecutionException, InterruptedException, TimeoutException {
ContentResponse rs = this.client.newRequest(String.format("http://localhost:%s/some/location", this.port))
.method(method).body(new BytesRequestContent("abc")).send();
MatcherAssert.assertThat(rs.getStatus(), new IsEqual<>(200));
}
@ParameterizedTest
@ValueSource(strings = {"HEAD", "PUT", "POST"})
void worksWithNoBody(final String method) {
org.eclipse.jetty.client.Request rq =
this.client.newRequest(String.format("http://localhost:%s/some/location", this.port)).method(method);
final ReactiveRequest build = ReactiveRequest.newBuilder(rq).build();
final Publisher<ReactiveResponse> publisher = build.response(ReactiveResponse.Content.discard());
int status = Single.fromPublisher(publisher)
.map(ReactiveResponse::getStatus)
.blockingGet();
MatcherAssert.assertThat(status, new IsEqual<>(200));
}
@AfterEach
void stop() throws Exception {
this.server.stop();
this.client.stop();
}
}This is the same issue as #194.
The implementation needs to know if the request content is reproducible, and this is not supported yet for reactive content, but it is for non-reactive content.
hm, @sbordet in jetty 12.0.3 (not reactive) this same not correct behavior is reproduced with AsyncRequestContent:
@ParameterizedTest
@ValueSource(strings = {"HEAD", "PUT", "POST"})
void failsWithAsyncContent(final String method) throws ExecutionException, InterruptedException, TimeoutException {
final AsyncRequestContent async = new AsyncRequestContent();
async.write(ByteBuffer.wrap("abc".getBytes()), Callback.NOOP);
async.close();
ContentResponse rs = this.client.newRequest(String.format("http://localhost:%s/some/location", this.port))
.method(method).body(async).send();
MatcherAssert.assertThat(rs.getStatus(), new IsEqual<>(200));
}(this fail-test method can be added to the class posted by me above)
@olenagerasimova this is because AsyncRequestContent is not reproducible.
You have to understand that if you have request content, you send part of it and halfway through your send the server sends a redirect, you may not be able to send the request content again from the beginning.
For example, imagine that the request content is obtained by reading from an InputStream: once you read from the InputStream the first time, you cannot read from it again, so you will not be able to follow the redirect because you cannot send again the request content.
Certain contents, for example based on String or byte[], can be reproduced, but most of the others cannot, so you can only expect PUT/POST to work iff they send reproducible content.
@sbordet got it, thanks for the explanation