Remove AccessLogEntry.setURI/getURI

2 files changed, 9 insertions, 81 deletions

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/";