diff options
Diffstat (limited to 'container-core')
6 files changed, 118 insertions, 55 deletions
diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java index ccb41ca3055..af1179fadba 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java @@ -57,6 +57,7 @@ class AccessLogRequestLog extends AbstractLifeCycle implements org.eclipse.jetty int peerPort = request.getRemotePort(); long startTime = request.getTimeStamp(); long endTime = System.currentTimeMillis(); + Integer statusCodeOverride = (Integer) request.getAttribute(HttpRequestDispatch.ACCESS_LOG_STATUS_CODE_OVERRIDE); builder.peerAddress(peerAddress) .peerPort(peerPort) .localPort(getLocalPort(request)) @@ -64,7 +65,7 @@ class AccessLogRequestLog extends AbstractLifeCycle implements org.eclipse.jetty .duration(Duration.ofMillis(Math.max(0, endTime - startTime))) .responseSize(response.getHttpChannel().getBytesWritten()) .requestSize(request.getHttpInput().getContentReceived()) - .statusCode(response.getCommittedMetaData().getStatus()); + .statusCode(statusCodeOverride != null ? statusCodeOverride : response.getCommittedMetaData().getStatus()); addNonNullValue(builder, request.getMethod(), RequestLogEntry.Builder::httpMethod); addNonNullValue(builder, request.getRequestURI(), RequestLogEntry.Builder::rawPath); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java index a2c2a5df4df..0b021ec8bcd 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java @@ -42,9 +42,10 @@ import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnector; */ class HttpRequestDispatch { - private static final Logger log = Logger.getLogger(HttpRequestDispatch.class.getName()); + public static final String ACCESS_LOG_STATUS_CODE_OVERRIDE = "com.yahoo.jdisc.http.server.jetty.ACCESS_LOG_STATUS_CODE_OVERRIDE"; - private final static String CHARSET_ANNOTATION = ";charset="; + private static final Logger log = Logger.getLogger(HttpRequestDispatch.class.getName()); + private static final String CHARSET_ANNOTATION = ";charset="; private final JDiscContext jDiscContext; private final Request jettyRequest; @@ -129,10 +130,12 @@ class HttpRequestDispatch { error, () -> "Network connection was unexpectedly terminated: " + jettyRequest.getRequestURI()); metricReporter.prematurelyClosed(); + asyncCtx.getRequest().setAttribute(ACCESS_LOG_STATUS_CODE_OVERRIDE, 499); } else if (isErrorOfType(error, TimeoutException.class)) { log.log(Level.FINE, error, () -> "Request/stream was timed out by Jetty: " + jettyRequest.getRequestURI()); + asyncCtx.getRequest().setAttribute(ACCESS_LOG_STATUS_CODE_OVERRIDE, 408); } else if (!isErrorOfType(error, OverloadException.class, BindingNotFoundException.class, RequestException.class)) { log.log(Level.WARNING, "Request failed: " + jettyRequest.getRequestURI(), error); } diff --git a/container-core/src/main/java/com/yahoo/processing/request/CompoundName.java b/container-core/src/main/java/com/yahoo/processing/request/CompoundName.java index b4536a1c56b..440df4f9be9 100644 --- a/container-core/src/main/java/com/yahoo/processing/request/CompoundName.java +++ b/container-core/src/main/java/com/yahoo/processing/request/CompoundName.java @@ -8,6 +8,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Objects; import static com.yahoo.text.Lowercase.toLowerCase; @@ -74,41 +75,52 @@ public final class CompoundName { * @param compounds the compounds of this name */ private CompoundName(String name, String [] compounds, boolean useCache) { - if (name == null) throw new NullPointerException("Name can not be null"); - - this.name = name; + this.name = Objects.requireNonNull(name, "Name can not be null"); this.lowerCasedName = toLowerCase(name); - if (compounds.length == 1 && compounds[0].isEmpty()) { - this.compounds = List.of(); - this.hashCode = 0; - rest = this; - first = this; + if (compounds.length == 1) { + if (compounds[0].isEmpty()) { + this.compounds = List.of(); + this.hashCode = 0; + rest = first = this; + return; + } + this.compounds = new ImmutableArrayList(compounds); + this.hashCode = this.compounds.hashCode(); + rest = first = empty; return; } - this.compounds = new ImmutableArrayList(compounds); - this.hashCode = this.compounds.hashCode(); - - if (compounds.length > 1) { - String restName = name.substring(compounds[0].length()+1); - if (useCache) { - rest = cache.computeIfAbsent(restName, (key) -> new CompoundName(key, Arrays.copyOfRange(compounds, 1, compounds.length), useCache)); - } else { - rest = new CompoundName(restName, Arrays.copyOfRange(compounds, 1, compounds.length), useCache); + CompoundName[] children = new CompoundName[compounds.length]; + for (int i = 0; i + 1 < children.length; i++) { + int start = 0, end = i == 0 ? -1 : children[0].name.length(); + for (int j = 0; j + i < children.length; j++) { + end += compounds[j + i].length() + 1; + if (end == start) throw new IllegalArgumentException("'" + name + "' is not a legal compound name. " + + "Consecutive, leading or trailing dots are not allowed."); + String subName = this.name.substring(start, end); + CompoundName cached = cache.get(subName); + children[j] = cached != null ? cached + : new CompoundName(subName, + this.lowerCasedName.substring(start, end), + Arrays.copyOfRange(compounds, j, j + i + 1), + i == 0 ? empty : children[j + 1], + i == 0 ? empty : children[j]); + if (useCache && cached == null) cache.put(subName, children[j]); + start += compounds[j].length() + 1; } - } else { - rest = empty; } + this.compounds = new ImmutableArrayList(compounds); + this.hashCode = this.compounds.hashCode(); + this.rest = children[1]; + this.first = children[0]; + } - if (compounds.length > 1) { - String firstName = name.substring(0, name.length() - (compounds[compounds.length-1].length()+1)); - if (useCache) { - first = cache.computeIfAbsent(firstName, (key) -> new CompoundName(key, Arrays.copyOfRange(compounds, 0, compounds.length-1), useCache)); - } else { - first = new CompoundName(firstName, Arrays.copyOfRange(compounds, 0, compounds.length-1), useCache); - } - } else { - first = empty; - } + private CompoundName(String name, String lowerCasedName, String[] compounds, CompoundName rest, CompoundName first) { + this.name = name; + this.lowerCasedName = lowerCasedName; + this.compounds = new ImmutableArrayList(compounds); + this.hashCode = this.compounds.hashCode(); + this.rest = rest; + this.first = first; } private static List<String> parse(String s) { diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java index e4c8c86d93e..32190999df8 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java @@ -15,6 +15,7 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -23,6 +24,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; * @author bjorncs */ public class AccessLogRequestLogTest { + @Test void requireThatQueryWithUnquotedSpecialCharactersIsHandled() { Request jettyRequest = createRequestBuilder() @@ -37,6 +39,22 @@ public class AccessLogRequestLogTest { assertTrue(entry.rawQuery().isPresent()); } + + @Test + void requireThatStatusCodeCanBeOverridden() { + Request jettyRequest = createRequestBuilder() + .uri("http", "localhost", 12345, "/api/", null) + .build(); + + InMemoryRequestLog requestLog = new InMemoryRequestLog(); + new AccessLogRequestLog(requestLog).log(jettyRequest, JettyMockResponseBuilder.newBuilder().build()); + assertEquals(200, requestLog.entries().remove(0).statusCode().getAsInt()); + + jettyRequest.setAttribute(HttpRequestDispatch.ACCESS_LOG_STATUS_CODE_OVERRIDE, 404); + new AccessLogRequestLog(requestLog).log(jettyRequest, JettyMockResponseBuilder.newBuilder().build()); + assertEquals(404, requestLog.entries().remove(0).statusCode().getAsInt()); + } + @Test void requireThatDoubleQuotingIsNotPerformed() { String path = "/search/"; @@ -129,4 +147,5 @@ public class AccessLogRequestLogTest { private Response createResponseMock() { return JettyMockResponseBuilder.newBuilder().build(); } + } diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockResponseBuilder.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockResponseBuilder.java index efdc9eab283..e83c446b96d 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockResponseBuilder.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JettyMockResponseBuilder.java @@ -15,14 +15,23 @@ import static org.mockito.Mockito.when; */ public class JettyMockResponseBuilder { + private int statusCode = 200; + private JettyMockResponseBuilder() {} public static JettyMockResponseBuilder newBuilder() { return new JettyMockResponseBuilder(); } + public JettyMockResponseBuilder withStatusCode(int statusCode) { + this.statusCode = statusCode; + return this; + } + public Response build() { + MetaData.Response metaData = mock(MetaData.Response.class); + when(metaData.getStatus()).thenReturn(statusCode); Response response = mock(Response.class); when(response.getHttpChannel()).thenReturn(mock(HttpChannel.class)); - when(response.getCommittedMetaData()).thenReturn(mock(MetaData.Response.class)); + when(response.getCommittedMetaData()).thenReturn(metaData); return response; } diff --git a/container-core/src/test/java/com/yahoo/processing/request/CompoundNameTestCase.java b/container-core/src/test/java/com/yahoo/processing/request/CompoundNameTestCase.java index b5143f89c78..7523a68501f 100644 --- a/container-core/src/test/java/com/yahoo/processing/request/CompoundNameTestCase.java +++ b/container-core/src/test/java/com/yahoo/processing/request/CompoundNameTestCase.java @@ -13,7 +13,7 @@ import static org.junit.jupiter.api.Assertions.*; /** * Module local test of the basic property name building block. * - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen */ public class CompoundNameTestCase { @@ -30,22 +30,22 @@ public class CompoundNameTestCase { } @Test - final void testLast() { + void testLast() { assertEquals(NAME.substring(NAME.lastIndexOf('.') + 1), C_NAME.last()); } @Test - final void testFirst() { + void testFirst() { assertEquals(NAME.substring(0, NAME.indexOf('.')), C_NAME.first()); } @Test - final void testRest() { + void testRest() { verifyStrict(NAME.substring(NAME.indexOf('.') + 1), C_NAME.rest()); } @Test - final void testRestN() { + void testRestN() { verifyStrict("a.b.c.d.e", C_abcde.rest(0)); verifyStrict("b.c.d.e", C_abcde.rest(1)); verifyStrict("c.d.e", C_abcde.rest(2)); @@ -53,8 +53,9 @@ public class CompoundNameTestCase { verifyStrict("e", C_abcde.rest(4)); verifyStrict(CompoundName.empty, C_abcde.rest(5)); } + @Test - final void testFirstN() { + void testFirstN() { verifyStrict("a.b.c.d.e", C_abcde.first(5)); verifyStrict("a.b.c.d", C_abcde.first(4)); verifyStrict("a.b.c", C_abcde.first(3)); @@ -64,15 +65,32 @@ public class CompoundNameTestCase { } @Test - final void testPrefix() { - CompoundName abc = CompoundName.from("a.b.c"); - assertTrue(abc.hasPrefix(CompoundName.empty)); - assertTrue(abc.hasPrefix(CompoundName.from("a"))); - assertTrue(abc.hasPrefix(CompoundName.from("a.b"))); - assertTrue(abc.hasPrefix(CompoundName.from("a.b.c"))); + void testPrefix() { + CompoundName abcc = CompoundName.from("a.b.cc"); + assertTrue(abcc.hasPrefix(CompoundName.empty)); + assertTrue(abcc.hasPrefix(CompoundName.from("a"))); + assertTrue(abcc.hasPrefix(CompoundName.from("a.b"))); + assertTrue(abcc.hasPrefix(CompoundName.from("a.b.cc"))); - assertFalse(abc.hasPrefix(CompoundName.from("a.b.c.d"))); - assertFalse(abc.hasPrefix(CompoundName.from("a.b.d"))); + assertFalse(abcc.hasPrefix(CompoundName.from("a.b.c"))); + assertFalse(abcc.hasPrefix(CompoundName.from("a.b.c.d"))); + assertFalse(abcc.hasPrefix(CompoundName.from("a.b.d"))); + } + + @Test + void testIllegalCompound() { + assertEquals("'a.' is not a legal compound name. Names can not end with a dot.", + assertThrows(IllegalArgumentException.class, + () -> CompoundName.from("a.")) + .getMessage()); + assertEquals("'.b' is not a legal compound name. Consecutive, leading or trailing dots are not allowed.", + assertThrows(IllegalArgumentException.class, + () -> CompoundName.from(".b")) + .getMessage()); + assertEquals("'a..b' is not a legal compound name. Consecutive, leading or trailing dots are not allowed.", + assertThrows(IllegalArgumentException.class, + () -> CompoundName.from("a..b")) + .getMessage()); } @Test @@ -82,7 +100,7 @@ public class CompoundNameTestCase { } @Test - final void testSize() { + void testSize() { Splitter s = Splitter.on('.'); Iterable<String> i = s.split(NAME); int n = 0; @@ -93,23 +111,23 @@ public class CompoundNameTestCase { } @Test - final void testGet() { + void testGet() { String s = C_NAME.get(0); assertEquals(NAME.substring(0, NAME.indexOf('.')), s); } @Test - final void testIsCompound() { + void testIsCompound() { assertTrue(C_NAME.isCompound()); } @Test - final void testIsEmpty() { + void testIsEmpty() { assertFalse(C_NAME.isEmpty()); } @Test - final void testAsList() { + void testAsList() { List<String> l = C_NAME.asList(); Splitter peoplesFront = Splitter.on('.'); Iterable<String> answer = peoplesFront.split(NAME); @@ -121,7 +139,7 @@ public class CompoundNameTestCase { } @Test - final void testEqualsObject() { + void testEqualsObject() { assertNotEquals(C_NAME, NAME); assertNotEquals(C_NAME, null); verifyStrict(C_NAME, C_NAME); @@ -129,7 +147,7 @@ public class CompoundNameTestCase { } @Test - final void testEmptyNonEmpty() { + void testEmptyNonEmpty() { assertTrue(CompoundName.empty.isEmpty()); assertEquals(0, CompoundName.empty.size()); assertFalse(CompoundName.from("a").isEmpty()); @@ -140,7 +158,7 @@ public class CompoundNameTestCase { } @Test - final void testGetLowerCasedName() { + void testGetLowerCasedName() { assertEquals(Lowercase.toLowerCase(NAME), C_NAME.getLowerCasedName()); } @@ -223,4 +241,5 @@ public class CompoundNameTestCase { assertEquals("[one]", CompoundName.from("one").asList().toString()); assertEquals("[one, two, three]", CompoundName.from("one.two.three").asList().toString()); } + } |