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.

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

Issue #5684 Fix and re-enable ServletRequestLogTest. #5877

Merged
merged 1 commit into from Jan 13, 2021
Merged
Changes from all commits
Commits
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
Expand Up @@ -19,10 +19,7 @@
package org.eclipse.jetty.servlet;

import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.net.HttpURLConnection;
import java.net.URI;
import java.util.ArrayList;
Expand All @@ -39,6 +36,7 @@

import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.HttpChannel;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.RequestLog;
import org.eclipse.jetty.server.Response;
Expand All @@ -48,24 +46,24 @@
import org.eclipse.jetty.server.handler.DefaultHandler;
import org.eclipse.jetty.server.handler.ErrorHandler;
import org.eclipse.jetty.server.handler.HandlerCollection;
import org.eclipse.jetty.toolchain.test.IO;
import org.eclipse.jetty.util.component.AbstractLifeCycle;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.util.log.StacklessLogging;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import static java.time.Duration.ofSeconds;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;

/**
* Servlet equivalent of the jetty-server's RequestLogHandlerTest, but with more ErrorHandler twists.
*/
@Disabled
public class ServletRequestLogTest
{
private static final Logger LOG = Log.getLogger(ServletRequestLogTest.class);
Expand Down Expand Up @@ -109,7 +107,7 @@ private static class ResponseSendErrorServlet extends AbstractTestServlet
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
response.sendError(500, "Whoops");
response.sendError(500, "FromResponseSendErrorServlet");
}
}

Expand All @@ -119,7 +117,7 @@ private static class ServletExceptionServlet extends AbstractTestServlet
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
throw new ServletException("Whoops");
throw new ServletException("FromServletExceptionServlet");
}
}

Expand All @@ -129,7 +127,7 @@ private static class IOExceptionServlet extends AbstractTestServlet
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
throw new IOException("Whoops");
throw new IOException("FromIOExceptionServlet");
}
}

Expand All @@ -139,7 +137,7 @@ private static class RuntimeExceptionServlet extends AbstractTestServlet
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
throw new RuntimeException("Whoops");
throw new RuntimeException("FromRuntimeExceptionServlet");
}
}

Expand Down Expand Up @@ -273,7 +271,6 @@ public static Stream<Arguments> data()
data.add(new Object[]{new HelloServlet(), "/test/", "GET /test/ HTTP/1.1 200"});
data.add(new Object[]{new AsyncOnTimeoutCompleteServlet(), "/test/", "GET /test/ HTTP/1.1 200"});
data.add(new Object[]{new AsyncOnTimeoutDispatchServlet(), "/test/", "GET /test/ HTTP/1.1 200"});

data.add(new Object[]{new AsyncOnStartIOExceptionServlet(), "/test/", "GET /test/ HTTP/1.1 500"});
data.add(new Object[]{new ResponseSendErrorServlet(), "/test/", "GET /test/ HTTP/1.1 500"});
data.add(new Object[]{new ServletExceptionServlet(), "/test/", "GET /test/ HTTP/1.1 500"});
Expand Down Expand Up @@ -323,46 +320,40 @@ public void testLogHandlerCollection(Servlet testServlet, String requestPath, St

// Add the test servlet
ServletHolder testHolder = new ServletHolder(testServlet);
app.addServlet(testHolder, "/test");
app.addServlet(testHolder, "/test/*");

try
try (StacklessLogging scope = new StacklessLogging(HttpChannel.class))
{
server.start();

Assertions.assertTimeoutPreemptively(ofSeconds(4), () ->
{
server.start();

connector.addBean(new HttpChannel.Listener()
{
@Override
public void onComplete(Request request)
{
assertRequestLog(expectedLogEntry, captureLog);
}
});

String host = connector.getHost();
if (host == null)
{
host = "localhost";
}

int port = connector.getLocalPort();

URI serverUri = new URI("http", null, host, port, requestPath, null, null);

// Make call to test handler
HttpURLConnection connection = (HttpURLConnection)serverUri.toURL().openConnection();
try
{
connection.setAllowUserInteraction(false);

// log response status code
int statusCode = connection.getResponseCode();
LOG.debug("Response Status Code: {}", statusCode);

if (statusCode == 200)
{
// collect response message and log it
String content = getResponseContent(connection);
LOG.debug("Response Content: {}", content);
}
}
finally
{
connection.disconnect();
}

assertRequestLog(expectedLogEntry, captureLog);
});
}
finally
Expand Down Expand Up @@ -413,47 +404,40 @@ public void testLogHandlerCollectionErrorHandlerServerBean(Servlet testServlet,

// Add the test servlet
ServletHolder testHolder = new ServletHolder(testServlet);
app.addServlet(testHolder, "/test");
app.addServlet(testHolder, "/test/*");

try
try (StacklessLogging scope = new StacklessLogging(HttpChannel.class))
{
server.start();

Assertions.assertTimeoutPreemptively(ofSeconds(4), () ->
{

connector.addBean(new HttpChannel.Listener()
{
@Override
public void onComplete(Request request)
{
assertRequestLog(expectedLogEntry, captureLog);
}
});

String host = connector.getHost();
if (host == null)
{
host = "localhost";
}
int port = connector.getLocalPort();

int port = connector.getLocalPort();
URI serverUri = new URI("http", null, host, port, requestPath, null, null);

// Make call to test handler
HttpURLConnection connection = (HttpURLConnection)serverUri.toURL().openConnection();
try
{
connection.setAllowUserInteraction(false);

// log response status code
int statusCode = connection.getResponseCode();
LOG.debug("Response Status Code: {}", statusCode);

if (statusCode == 200)
{
// collect response message and log it
String content = getResponseContent(connection);
LOG.debug("Response Content: {}", content);
}
}
finally
{
connection.disconnect();
}

assertRequestLog(expectedLogEntry, captureLog);
});
}
finally
Expand Down Expand Up @@ -509,45 +493,38 @@ public void testLogHandlerCollectionSimpleErrorPageMapping(Servlet testServlet,
errorMapper.addErrorPage(500, "/errorpage");
app.setErrorHandler(errorMapper);

try
try (StacklessLogging scope = new StacklessLogging(HttpChannel.class))
{
server.start();

Assertions.assertTimeoutPreemptively(ofSeconds(4), () ->
{

connector.addBean(new HttpChannel.Listener()
{
@Override
public void onComplete(Request request)
{
assertRequestLog(expectedLogEntry, captureLog);
}
});

String host = connector.getHost();
if (host == null)
{
host = "localhost";
}
int port = connector.getLocalPort();

int port = connector.getLocalPort();
URI serverUri = new URI("http", null, host, port, requestPath, null, null);

// Make call to test handler
HttpURLConnection connection = (HttpURLConnection)serverUri.toURL().openConnection();
try
{
connection.setAllowUserInteraction(false);

// log response status code
int statusCode = connection.getResponseCode();
LOG.debug("Response Status Code: {}", statusCode);

if (statusCode == 200)
{
// collect response message and log it
String content = getResponseContent(connection);
LOG.debug("Response Content: {}", content);
}
}
finally
{
connection.disconnect();
}

assertRequestLog(expectedLogEntry, captureLog);
});
}
finally
Expand Down Expand Up @@ -608,38 +585,32 @@ public void testLogHandlerWrapped(Servlet testServlet, String requestPath, Strin

Assertions.assertTimeoutPreemptively(ofSeconds(4), () ->
{
connector.addBean(new HttpChannel.Listener()
{
@Override
public void onComplete(Request request)
{
assertRequestLog(expectedLogEntry, captureLog);
}
});

String host = connector.getHost();
if (host == null)
{
host = "localhost";
}
int port = connector.getLocalPort();

int port = connector.getLocalPort();
URI serverUri = new URI("http", null, host, port, "/test", null, null);

// Make call to test handler
HttpURLConnection connection = (HttpURLConnection)serverUri.toURL().openConnection();
try
{
connection.setAllowUserInteraction(false);

// log response status code
int statusCode = connection.getResponseCode();
LOG.info("Response Status Code: {}", statusCode);

if (statusCode == 200)
{
// collect response message and log it
String content = getResponseContent(connection);
LOG.info("Response Content: {}", content);
}
}
finally
{
connection.disconnect();
}

assertRequestLog(expectedLogEntry, captureLog);
});
}
finally
Expand All @@ -650,33 +621,7 @@ public void testLogHandlerWrapped(Servlet testServlet, String requestPath, Strin

private void assertRequestLog(final String expectedLogEntry, CaptureLog captureLog)
{
int captureCount = captureLog.captured.size();

if (captureCount != 1)
{
LOG.warn("Capture Log size is {}, expected to be 1", captureCount);
if (captureCount > 1)
{
for (int i = 0; i < captureCount; i++)
{
LOG.warn("[{}] {}", i, captureLog.captured.get(i));
}
}
assertThat("Capture Log Entry Count", captureLog.captured.size(), is(1));
}

String actual = captureLog.captured.get(0);
assertThat("Capture Log", actual, is(expectedLogEntry));
}

private String getResponseContent(HttpURLConnection connection) throws IOException
{
try (InputStream in = connection.getInputStream();
InputStreamReader reader = new InputStreamReader(in))
{
StringWriter writer = new StringWriter();
IO.copy(reader, writer);
return writer.toString();
}
assertThat("Request log size", captureLog.captured, not(empty()));
assertThat("Request log entry",captureLog.captured.get(0), is(expectedLogEntry));
}
}