diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java index bfd958d180ef..326c35338665 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java @@ -670,7 +670,7 @@ else if (contentLength != field.getLongValue()) close = true; _persistent = false; } - if (keepAlive && _persistent == Boolean.FALSE) + if (keepAlive && Boolean.FALSE.equals(_persistent)) { field = new HttpField(HttpHeader.CONNECTION, Stream.of(field.getValues()).filter(s -> !HttpHeaderValue.KEEP_ALIVE.is(s)) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java index 0a2e758caf39..02cbe08192c4 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java @@ -108,7 +108,7 @@ enum Violation */ SEPARATOR("Ambiguous path separator"), /** - * Ambiguous path parameters within a URI segment e.g. {@code /foo/..;/bar} + * Ambiguous path parameters within a URI segment e.g. {@code /foo/..;/bar} or {@code /foo/%2e%2e;param/bar} */ PARAM("Ambiguous path parameters"), /** @@ -782,15 +782,14 @@ private void checkSegment(String uri, int segment, int end, boolean param) // Look for segment in the ambiguous segment index. Boolean ambiguous = __ambiguousSegments.get(uri, segment, end - segment); - if (ambiguous == Boolean.TRUE) + if (ambiguous != null) { - // The segment is always ambiguous. - _violations.add(Violation.SEGMENT); - } - else if (param && ambiguous == Boolean.FALSE) - { - // The segment is ambiguous only when followed by a parameter. - _violations.add(Violation.PARAM); + // Is the segment intrinsically ambiguous + if (Boolean.TRUE.equals(ambiguous)) + _violations.add(Violation.SEGMENT); + // Is the segment ambiguous with a parameter? + if (param) + _violations.add(Violation.PARAM); } } diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java index 28be9c41f0ba..ee8e54d0ec2b 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java @@ -358,9 +358,9 @@ public static Stream decodePathTests() {"/path/%2e/info", "/path/info", EnumSet.of(Violation.SEGMENT)}, {"path/%2e/info/", "path/info/", EnumSet.of(Violation.SEGMENT)}, {"/path/%2e%2e/info", "/info", EnumSet.of(Violation.SEGMENT)}, - {"/path/%2e%2e;/info", "/info", EnumSet.of(Violation.SEGMENT)}, - {"/path/%2e%2e;param/info", "/info", EnumSet.of(Violation.SEGMENT)}, - {"/path/%2e%2e;param;other/info;other", "/info", EnumSet.of(Violation.SEGMENT)}, + {"/path/%2e%2e;/info", "/info", EnumSet.of(Violation.SEGMENT, Violation.PARAM)}, + {"/path/%2e%2e;param/info", "/info", EnumSet.of(Violation.SEGMENT, Violation.PARAM)}, + {"/path/%2e%2e;param;other/info;other", "/info", EnumSet.of(Violation.SEGMENT, Violation.PARAM)}, {"%2e/info", "info", EnumSet.of(Violation.SEGMENT)}, {"%u002e/info", "info", EnumSet.of(Violation.SEGMENT, Violation.UTF16)}, @@ -473,9 +473,9 @@ public static Stream testPathQueryTests() {"/path/%2e/info", "/path/info", EnumSet.of(Violation.SEGMENT)}, {"path/%2e/info/", "path/info/", EnumSet.of(Violation.SEGMENT)}, {"/path/%2e%2e/info", "/info", EnumSet.of(Violation.SEGMENT)}, - {"/path/%2e%2e;/info", "/info", EnumSet.of(Violation.SEGMENT)}, - {"/path/%2e%2e;param/info", "/info", EnumSet.of(Violation.SEGMENT)}, - {"/path/%2e%2e;param;other/info;other", "/info", EnumSet.of(Violation.SEGMENT)}, + {"/path/%2e%2e;/info", "/info", EnumSet.of(Violation.SEGMENT, Violation.PARAM)}, + {"/path/%2e%2e;param/info", "/info", EnumSet.of(Violation.SEGMENT, Violation.PARAM)}, + {"/path/%2e%2e;param;other/info;other", "/info", EnumSet.of(Violation.SEGMENT, Violation.PARAM)}, {"%2e/info", "info", EnumSet.of(Violation.SEGMENT)}, {"%2e", "", EnumSet.of(Violation.SEGMENT)}, diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java index 02ff5ba48314..0c267a078f55 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java @@ -1947,13 +1947,14 @@ public void setComplianceModes(HttpCompliance compliance, HttpComplianceSection. @Test public void testAmbiguousPaths() throws Exception { + // TODO this is a poor test as it is not very exhaustive. See HttpURITest for better tests. _handler._checker = (request, response) -> { response.getOutputStream().println("servletPath=" + request.getServletPath()); response.getOutputStream().println("pathInfo=" + request.getPathInfo()); return true; }; - String request = "GET /unnormal/.././path/ambiguous%2f%2e%2e/%2e;/info HTTP/1.0\r\n" + + String request = "GET /unnormal/.././path/ambiguous%2f%2e%2e/info HTTP/1.0\r\n" + "Host: whatever\r\n" + "\r\n"; @@ -1962,7 +1963,7 @@ public void testAmbiguousPaths() throws Exception startsWith("HTTP/1.1 200"), containsString("pathInfo=/path/info"))); - setComplianceModes(HttpComplianceSection.NO_AMBIGUOUS_PATH_SEGMENTS); + setComplianceModes(HttpComplianceSection.NO_AMBIGUOUS_PATH_SEPARATORS); assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400")); } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExcludeSet.java b/jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExcludeSet.java index c54d08619d4a..164ea02574c4 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExcludeSet.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExcludeSet.java @@ -259,12 +259,12 @@ public static boolean matchCombined(T1 item1, IncludeExcludeSet Boolean match2 = set2.isIncludedAndNotExcluded(item2); // if we are excluded from either set, then we do not match - if (match1 == Boolean.FALSE || match2 == Boolean.FALSE) + if (Boolean.FALSE.equals(match1) || Boolean.FALSE.equals(match2)) return false; // If either set has any includes, then we must be included by one of them if (set1.hasIncludes() || set2.hasIncludes()) - return match1 == Boolean.TRUE || match2 == Boolean.TRUE; + return Boolean.TRUE.equals(match1) || Boolean.TRUE.equals(match2); // If not excluded and no includes, then we match return true; diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/ClasspathPattern.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/ClasspathPattern.java index 160181b06b73..64571ec2a22d 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/ClasspathPattern.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/ClasspathPattern.java @@ -742,7 +742,7 @@ static boolean combine(IncludeExcludeSet names, String name, Incl Boolean byName = names.isIncludedAndNotExcluded(name); // If we excluded by name, then no match - if (Boolean.FALSE == byName) + if (Boolean.FALSE.equals(byName)) return false; // check the location set @@ -750,12 +750,12 @@ static boolean combine(IncludeExcludeSet names, String name, Incl Boolean byLocation = uri == null ? null : locations.isIncludedAndNotExcluded(uri); // If we excluded by location or couldn't check location exclusion, then no match - if (Boolean.FALSE == byLocation || (locations.hasExcludes() && uri == null)) + if (Boolean.FALSE.equals(byLocation) || (locations.hasExcludes() && uri == null)) return false; // If there are includes, then we must be included to match. if (names.hasIncludes() || locations.hasIncludes()) - return byName == Boolean.TRUE || byLocation == Boolean.TRUE; + return Boolean.TRUE.equals(byName) || Boolean.TRUE.equals(byLocation); // Otherwise there are no includes and it was not excluded, so match return true;