diff options
4 files changed, 9 insertions, 109 deletions
diff --git a/container-accesslogging/src/main/java/com/yahoo/container/logging/AccessLogEntry.java b/container-accesslogging/src/main/java/com/yahoo/container/logging/AccessLogEntry.java index 452466d7d05..46e5aa95dab 100644 --- a/container-accesslogging/src/main/java/com/yahoo/container/logging/AccessLogEntry.java +++ b/container-accesslogging/src/main/java/com/yahoo/container/logging/AccessLogEntry.java @@ -61,7 +61,6 @@ public class AccessLogEntry { private long timeStampMillis; private long durationBetweenRequestResponseMillis; private long numBytesReturned; - private URI uri; private String remoteAddress; @@ -600,27 +599,6 @@ public class AccessLogEntry { } } - /** - * @deprecated Use {@link #setRawPath(String)} and {@link #setRawQuery(String)} instead. - */ - @Deprecated - public void setURI(final URI uri) { - synchronized (monitor) { - requireNull(this.uri); - this.uri = uri; - } - } - - /** - * @deprecated Use {@link #getRawPath()} and {@link #getRawQuery()} instead. This method may return wrong path. - */ - @Deprecated - public URI getURI() { - synchronized (monitor) { - return uri; - } - } - public void setRemoteAddress(String remoteAddress) { synchronized (monitor) { requireNull(this.remoteAddress); diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/LoggingRequestHandler.java b/container-core/src/main/java/com/yahoo/container/jdisc/LoggingRequestHandler.java index 73b56cd89f5..705092ef12e 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/LoggingRequestHandler.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/LoggingRequestHandler.java @@ -295,7 +295,6 @@ public abstract class LoggingRequestHandler extends ThreadedHttpRequestHandler { logEntry.setRemotePort(remoteAddress.getPort()); } URI uri = AccessLogUtil.getUri(httpRequest); - setDeprecatedUri(logEntry, uri); logEntry.setRawPath(uri.getRawPath()); logEntry.setRawQuery(uri.getRawQuery()); logEntry.setUserAgent(AccessLogUtil.getUserAgentHeader(httpRequest)); @@ -307,9 +306,4 @@ public abstract class LoggingRequestHandler extends ThreadedHttpRequestHandler { } } - @SuppressWarnings("deprecation") - private static void setDeprecatedUri(AccessLogEntry logEntry, URI uri) { - logEntry.setURI(uri); - } - } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java index 26db07f9ed7..a445230769b 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java @@ -28,13 +28,14 @@ import java.util.logging.Logger; * This class is a bridge between Jetty's {@link org.eclipse.jetty.server.handler.RequestLogHandler} * and our own configurable access logging in different formats provided by {@link AccessLog}. * - * @author bakksjo + * @author Oyvind Bakksjo * @author bjorncs */ public class AccessLogRequestLog extends AbstractLifeCycle implements RequestLog { private static final Logger logger = Logger.getLogger(AccessLogRequestLog.class.getName()); + // TODO These hardcoded headers should be provided by config instead private static final String HEADER_NAME_X_FORWARDED_FOR = "x-forwarded-for"; private static final String HEADER_NAME_Y_RA = "y-ra"; private static final String HEADER_NAME_Y_RP = "y-rp"; @@ -83,7 +84,6 @@ public class AccessLogRequestLog extends AbstractLifeCycle implements RequestLog public static void populateAccessLogEntryFromHttpServletRequest( final HttpServletRequest request, final AccessLogEntry accessLogEntry) { - setUriFromRequest(request, accessLogEntry); accessLogEntry.setRawPath(request.getRequestURI()); String queryString = request.getQueryString(); @@ -135,53 +135,4 @@ public class AccessLogRequestLog extends AbstractLifeCycle implements RequestLog .map(Integer::valueOf) .orElseGet(request::getRemotePort); } - - @SuppressWarnings("deprecation") - private static void setUriFromRequest(HttpServletRequest request, AccessLogEntry accessLogEntry) { - tryCreateUriFromRequest(request) - .ifPresent(accessLogEntry::setURI); // setURI is deprecated - } - - // This is a mess and does not work correctly - private static Optional<URI> tryCreateUriFromRequest(HttpServletRequest request) { - final String quotedQuery = request.getQueryString(); - final String quotedPath = request.getRequestURI(); - try { - final StringBuilder uriBuffer = new StringBuilder(); - uriBuffer.append(quotedPath); - if (quotedQuery != null) { - uriBuffer.append('?').append(quotedQuery); - } - return Optional.of(new URI(uriBuffer.toString())); - } catch (URISyntaxException e) { - return setUriFromMalformedInput(quotedPath, quotedQuery); - } - } - - private static Optional<URI> setUriFromMalformedInput(final String quotedPath, final String quotedQuery) { - try { - final String scheme = null; - final String authority = null; - final String fragment = null; - return Optional.of(new URI(scheme, authority, unquote(quotedPath), unquote(quotedQuery), fragment)); - } catch (URISyntaxException e) { - // I have no idea how this can happen here now... - logger.log(Level.WARNING, "Could not convert String URI to URI object", e); - return Optional.empty(); - } - } - - private static String unquote(final String quotedQuery) { - if (quotedQuery == null) { - return null; - } - try { - // inconsistent handling of semi-colon added here... - return URLDecoder.decode(quotedQuery, StandardCharsets.UTF_8.name()); - } catch (IllegalArgumentException e) { - return quotedQuery; - } catch (UnsupportedEncodingException e) { - throw new RuntimeException(e); // should not happen - } - } } diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java index 1048d7b6422..d5043f7b989 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java @@ -15,11 +15,12 @@ import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.testng.Assert.assertTrue; /** - * @author bakksjo + * @author Oyvind Bakksjo + * @author bjorncs */ -@SuppressWarnings("deprecation") // AccessLogEntry.setURI/getURI are deprecated public class AccessLogRequestLogTest { @Test public void requireThatQueryWithUnquotedSpecialCharactersIsHandled() { @@ -30,7 +31,8 @@ public class AccessLogRequestLogTest { AccessLogRequestLog.populateAccessLogEntryFromHttpServletRequest(httpServletRequest, accessLogEntry); - assertThat(accessLogEntry.getURI(), is(not(nullValue()))); + assertThat(accessLogEntry.getRawPath(), is(not(nullValue()))); + assertTrue(accessLogEntry.getRawQuery().isPresent()); } @Test @@ -44,37 +46,12 @@ public class AccessLogRequestLogTest { AccessLogRequestLog.populateAccessLogEntryFromHttpServletRequest(httpServletRequest, accessLogEntry); - assertThat(accessLogEntry.getURI().toString(), is(path + '?' + query)); + assertThat(accessLogEntry.getRawPath(), is(path)); + assertThat(accessLogEntry.getRawQuery().get(), is(query)); } @Test - public void requireThatNoQueryPartIsHandledWhenRequestIsMalformed() { - final HttpServletRequest httpServletRequest = mock(HttpServletRequest.class); - final String path = "/s>earch/"; - when(httpServletRequest.getRequestURI()).thenReturn(path); - final String query = null; - when(httpServletRequest.getQueryString()).thenReturn(query); - final AccessLogEntry accessLogEntry = new AccessLogEntry(); - - AccessLogRequestLog.populateAccessLogEntryFromHttpServletRequest(httpServletRequest, accessLogEntry); - - assertThat(accessLogEntry.getURI().toString(), is("/s%3Eearch/")); - - } - - @Test - public void invalid_percent_escape_patterns_in_query_string_are_escaped() { - HttpServletRequest httpServletRequest = mock(HttpServletRequest.class); - when(httpServletRequest.getRequestURI()).thenReturn("/search/"); - when(httpServletRequest.getQueryString()).thenReturn("q=%%2"); - - AccessLogEntry accessLogEntry = new AccessLogEntry(); - AccessLogRequestLog.populateAccessLogEntryFromHttpServletRequest(httpServletRequest, accessLogEntry); - assertThat(accessLogEntry.getURI().toString(), is("/search/?q=%25%252")); - } - - @Test public void raw_path_and_query_are_set_from_request() { HttpServletRequest httpServletRequest = mock(HttpServletRequest.class); String rawPath = "//search/"; |