Skip to content

Commit

Permalink
Issue #6973 - Setup Request/Response for success with RequestLog.
Browse files Browse the repository at this point in the history 
+ Restore committed Response headers as well

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
  • Loading branch information
@joakime
joakime committed Oct 20, 2021
1 parent 9a9e3c8 commit 92cd01b
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 22 deletions.
8 changes: 8 additions & 0 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1282,6 +1282,14 @@ private void remove(int i)
_fields[_size] = null;
}

public void setAll(HttpFields httpFields)
{
for (HttpField field : httpFields)
{
put(field);
}
}

public int size()
{
return _size;
Expand Down
8 changes: 7 additions & 1 deletion jetty-server/src/main/java/org/eclipse/jetty/server/Request.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1418,11 +1418,17 @@ protected void onRequestLog()
// Don't allow pulling more parameters
_contentParamsExtracted = true;

// Reset the status code to what was committed
// Reset the response to what it was when committed
MetaData.Response committedResponse = getResponse().getCommittedMetaData();
if (committedResponse != null)
{
// restore status
getResponse().setStatus(committedResponse.getStatus());

// restore headers
HttpFields.Mutable responseFields = getResponse().getHttpFields();
responseFields.clear();
responseFields.setAll(committedResponse.getFields());
}
}

Expand Down
131 changes: 110 additions & 21 deletions jetty-server/src/test/java/org/eclipse/jetty/server/RequestLogTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.junit.jupiter.api.Assertions.assertTrue;

/**
* Test raw behaviors of RequestLog and how Request / Response objects behave during
Expand All @@ -53,26 +54,26 @@ public class RequestLogTest
{
private static final Logger LOG = LoggerFactory.getLogger(RequestLogTest.class);

public Server createServer(RequestLog requestLog) throws Exception
private static class NormalResponse extends AbstractHandler
{
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
response.setCharacterEncoding("UTF-8");
response.setContentType("text/plain");
response.getWriter().printf("Got %s to %s%n", request.getMethod(), request.getRequestURI());
baseRequest.setHandled(true);
}
}

public Server createServer(RequestLog requestLog, Handler serverHandler) throws Exception
{
Server server = new Server();
ServerConnector connector = new ServerConnector(server);
connector.setPort(0);
server.addConnector(connector);

server.setRequestLog(requestLog);
server.setHandler(new AbstractHandler()
{
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
response.setCharacterEncoding("UTF-8");
response.setContentType("text/plain");
response.getWriter().printf("Got %s to %s%n", request.getMethod(), request.getRequestURI());
baseRequest.setHandled(true);
}
});

server.setHandler(serverHandler);
return server;
}

Expand All @@ -84,8 +85,9 @@ public void testNormalGetRequest() throws Exception
{
BlockingArrayQueue<String> requestLogLines = new BlockingArrayQueue<>();

server = createServer((request, response) ->
requestLogLines.add(String.format("method:%s|uri:%s|status:%d", request.getMethod(), request.getRequestURI(), response.getStatus())));
server = createServer((request, response1) ->
requestLogLines.add(String.format("method:%s|uri:%s|status:%d", request.getMethod(), request.getRequestURI(), response1.getStatus())),
new NormalResponse());
server.start();

URI baseURI = server.getURI();
Expand Down Expand Up @@ -143,13 +145,15 @@ public void testNormalPostFormRequest() throws Exception
{
BlockingArrayQueue<String> requestLogLines = new BlockingArrayQueue<>();

server = createServer((request, response) ->
// Use a Servlet API that would cause a read of the Request inputStream.
// This should result in no paramNames, as nothing is read during RequestLog execution
server = createServer((request, response1) ->
{
// Use a Servlet API that would cause a read of the Request inputStream.
List<String> paramNames = Collections.list(request.getParameterNames());
// This should result in no paramNames, as nothing is read during RequestLog execution
requestLogLines.add(String.format("method:%s|uri:%s|paramNames.size:%d|status:%d", request.getMethod(), request.getRequestURI(), paramNames.size(), response.getStatus()));
});
requestLogLines.add(String.format("method:%s|uri:%s|paramNames.size:%d|status:%d", request.getMethod(), request.getRequestURI(), paramNames.size(), response1.getStatus()));
}, new NormalResponse());
server.start();

URI baseURI = server.getURI();
Expand Down Expand Up @@ -218,13 +222,15 @@ public void testBadPostFormRequest() throws Exception
{
BlockingArrayQueue<String> requestLogLines = new BlockingArrayQueue<>();

server = createServer((request, response) ->
// Use a Servlet API that would cause a read of the Request inputStream.
// This should result in no paramNames, as nothing is read during RequestLog execution
server = createServer((request, response1) ->
{
// Use a Servlet API that would cause a read of the Request inputStream.
List<String> paramNames = Collections.list(request.getParameterNames());
// This should result in no paramNames, as nothing is read during RequestLog execution
requestLogLines.add(String.format("method:%s|uri:%s|paramNames.size:%d|status:%d", request.getMethod(), request.getRequestURI(), paramNames.size(), response.getStatus()));
});
requestLogLines.add(String.format("method:%s|uri:%s|paramNames.size:%d|status:%d", request.getMethod(), request.getRequestURI(), paramNames.size(), response1.getStatus()));
}, new NormalResponse());
server.start();

URI baseURI = server.getURI();
Expand Down Expand Up @@ -287,4 +293,87 @@ public void testBadPostFormRequest() throws Exception
LifeCycle.stop(server);
}
}

@Test
public void testResponseThenChangeStatusAndHeaders() throws Exception
{
Server server = null;
try
{
BlockingArrayQueue<String> requestLogLines = new BlockingArrayQueue<>();

RequestLog requestLog = (request, response) ->
{
String xname = response.getHeader("X-Name");
requestLogLines.add(String.format("method:%s|uri:%s|header[x-name]:%s|status:%d", request.getMethod(), request.getRequestURI(), xname, response.getStatus()));
};

Handler handler = new AbstractHandler()
{
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
response.setStatus(202);
response.setHeader("X-Name", "actual");
response.setCharacterEncoding("UTF-8");
response.setContentType("text/plain");
response.getWriter().printf("Got %s to %s%n", request.getMethod(), request.getRequestURI());
response.flushBuffer();
assertTrue(response.isCommitted(), "Response should be committed");
baseRequest.setHandled(true);
response.setStatus(204);
response.setHeader("X-Name", "post-commit");
}
};

server = createServer(requestLog, handler);
server.start();

URI baseURI = server.getURI();

try (Socket socket = new Socket(baseURI.getHost(), baseURI.getPort());
OutputStream out = socket.getOutputStream();
InputStream in = socket.getInputStream())
{
StringBuilder req = new StringBuilder();
req.append("GET /world HTTP/1.1\r\n");
req.append("Host: ").append(baseURI.getRawAuthority()).append("\r\n");
req.append("Connection: close\r\n");
req.append("\r\n");

byte[] bufRequest = req.toString().getBytes(UTF_8);

if (LOG.isDebugEnabled())
LOG.debug("--Request--\n" + req);
out.write(bufRequest);
out.flush();

ByteArrayOutputStream outBuf = new ByteArrayOutputStream();
IO.copy(in, outBuf);
String response = outBuf.toString(UTF_8);
if (LOG.isDebugEnabled())
LOG.debug("--Response--\n" + response);

List<String> responseLines = response.lines()
.map(String::trim)
.collect(Collectors.toList());

// Find status code
String responseStatusLine = responseLines.get(0);
assertThat("Status Code Response", responseStatusLine, containsString("HTTP/1.1 202 Accepted"));

// Find body content (always last line)
String bodyContent = responseLines.get(responseLines.size() - 1);
assertThat("Body Content", bodyContent, containsString("Got GET to /world"));

// We should see a requestlog entry for this 400 response
String reqlog = requestLogLines.poll(3, TimeUnit.SECONDS);
assertThat("RequestLog", reqlog, containsString("method:GET|uri:/world|header[x-name]:actual|status:202"));
}
}
finally
{
LifeCycle.stop(server);
}
}
}

0 comments on commit 92cd01b

Please sign in to comment.