Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reset trailers on recycled response #4712

Merged
merged 2 commits into from Mar 25, 2020
Merged

Reset trailers on recycled response #4712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f4da976
Reset trailers on recycled response
gregw Mar 25, 2020
1a3f39e
Issue #4711 - Reset trailers.
sbordet Mar 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions jetty-server/src/main/java/org/eclipse/jetty/server/Response.java
View file
Open in desktop
Expand Up @@ -132,6 +132,7 @@ protected void recycle()
_out.recycle();
_fields.clear();
_encodingFrom = EncodingFrom.NOT_SET;
_trailers = null;
}

public HttpOutput getHttpOutput()
Expand Down Expand Up @@ -1080,6 +1081,7 @@ public void reset()
_mimeType = null;
_characterEncoding = null;
_encodingFrom = EncodingFrom.NOT_SET;
_trailers = null;

// Clear all response headers
_fields.clear();
Expand Down
112 changes: 85 additions & 27 deletions ...t-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpTrailersTest.java
View file
Open in desktop
Expand Up @@ -23,7 +23,9 @@
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.util.Random;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import javax.servlet.ServletInputStream;
import javax.servlet.http.HttpServletRequest;
Expand All @@ -39,7 +41,6 @@
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ArgumentsSource;
Expand All @@ -49,6 +50,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class HttpTrailersTest extends AbstractTest<TransportScenario>
{
Expand Down Expand Up @@ -79,13 +81,11 @@ private void testRequestTrailers(byte[] content) throws Exception
{
String trailerName = "Trailer";
String trailerValue = "value";
scenario.start(new AbstractHandler()
scenario.start(new EmptyServerHandler()
{
@Override
public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
jettyRequest.setHandled(true);

// Read the content first.
ServletInputStream input = jettyRequest.getInputStream();
while (true)
Expand Down Expand Up @@ -118,13 +118,11 @@ public void handle(String target, Request jettyRequest, HttpServletRequest reque
public void testEmptyRequestTrailers(Transport transport) throws Exception
{
init(transport);
scenario.start(new AbstractHandler()
scenario.start(new EmptyServerHandler()
{
@Override
public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
jettyRequest.setHandled(true);

// Read the content first.
ServletInputStream input = jettyRequest.getInputStream();
while (true)
Expand Down Expand Up @@ -165,20 +163,23 @@ public void testResponseTrailersWithContent(Transport transport) throws Exceptio

private void testResponseTrailers(byte[] content) throws Exception
{
AtomicBoolean once = new AtomicBoolean();
String trailerName = "Trailer";
String trailerValue = "value";
scenario.start(new AbstractHandler()
scenario.start(new EmptyServerHandler()
{
@Override
public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
jettyRequest.setHandled(true);
Response jettyResponse = jettyRequest.getResponse();

HttpFields trailers = new HttpFields();
trailers.put(trailerName, trailerValue);
if (once.compareAndSet(false, true))
{
HttpFields trailers = new HttpFields();
trailers.put(trailerName, trailerValue);
jettyResponse.setTrailers(() -> trailers);
}

Response jettyResponse = (Response)response;
jettyResponse.setTrailers(() -> trailers);
if (content != null)
response.getOutputStream().write(content);
}
Expand All @@ -205,23 +206,40 @@ public void handle(String target, Request jettyRequest, HttpServletRequest reque
.send();
assertEquals(HttpStatus.OK_200, response.getStatus());
assertNull(failure.get());

// Subsequent requests should not have trailers.
response = scenario.client.newRequest(scenario.newURI())
.onResponseSuccess(r ->
{
try
{
HttpResponse httpResponse = (HttpResponse)r;
assertNull(httpResponse.getTrailers());
failure.set(null);
}
catch (Throwable x)
{
failure.set(x);
}
})
.timeout(5, TimeUnit.SECONDS)
.send();
assertEquals(HttpStatus.OK_200, response.getStatus());
assertNull(failure.get());
}

@ParameterizedTest
@ArgumentsSource(TransportProvider.class)
public void testEmptyResponseTrailers(Transport transport) throws Exception
{
init(transport);
scenario.start(new AbstractHandler()
scenario.start(new EmptyServerHandler()
{
@Override
public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response)
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response)
{
jettyRequest.setHandled(true);

HttpFields trailers = new HttpFields();

Response jettyResponse = (Response)response;
Response jettyResponse = jettyRequest.getResponse();
jettyResponse.setTrailers(() -> trailers);
}
});
Expand Down Expand Up @@ -257,17 +275,15 @@ public void testResponseTrailersWithLargeContent(Transport transport) throws Exc
String trailerName = "Trailer";
String trailerValue = "value";
init(transport);
scenario.start(new AbstractHandler()
scenario.start(new EmptyServerHandler()
{
@Override
public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
jettyRequest.setHandled(true);

HttpFields trailers = new HttpFields();
trailers.put(trailerName, trailerValue);

Response jettyResponse = (Response)response;
Response jettyResponse = jettyRequest.getResponse();
jettyResponse.setTrailers(() -> trailers);

// Write a large content
Expand Down Expand Up @@ -303,4 +319,46 @@ public void handle(String target, Request jettyRequest, HttpServletRequest reque
assertNotNull(trailers);
assertEquals(trailerValue, trailers.get(trailerName));
}

@ParameterizedTest
@ArgumentsSource(TransportProvider.class)
public void testResponseResetAlsoResetsTrailers(Transport transport) throws Exception
{
init(transport);
scenario.start(new EmptyServerHandler()
{
@Override
protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
Response jettyResponse = jettyRequest.getResponse();
HttpFields trailers = new HttpFields();
trailers.put("name", "value");
jettyResponse.setTrailers(() -> trailers);
// Fill some other response field.
response.setHeader("name", "value");
response.setStatus(HttpServletResponse.SC_NOT_ACCEPTABLE);
response.getWriter();

// Reset the response.
response.reset();
// Must not throw because we have called
// getWriter() above, since we have reset().
response.getOutputStream();
}
});

CountDownLatch latch = new CountDownLatch(1);
scenario.client.newRequest(scenario.newURI())
.timeout(5, TimeUnit.SECONDS)
.send(result ->
{
HttpResponse response = (HttpResponse)result.getResponse();
assertEquals(HttpStatus.OK_200, response.getStatus());
assertNull(response.getTrailers());
assertNull(response.getHeaders().get("name"));
latch.countDown();
});

assertTrue(latch.await(5, TimeUnit.SECONDS));
}
}