From 631f0cd9f61904af92d2e95321a34a1c31270668 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 6 Jun 2019 11:39:55 -0500 Subject: [PATCH 01/22] Issue #3648 - SSL based on WebSocket behavior (CLIENT vs SERVER) Signed-off-by: Joakim Erdfelt --- .../jetty/websocket/common/scopes/SimpleContainerScope.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/scopes/SimpleContainerScope.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/scopes/SimpleContainerScope.java index 8b00a84601b..11dcbefeaad 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/scopes/SimpleContainerScope.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/scopes/SimpleContainerScope.java @@ -84,7 +84,10 @@ public class SimpleContainerScope extends ContainerLifeCycle implements WebSocke if (ssl == null) { - this.sslContextFactory = new SslContextFactory.Server(); + if (policy.getBehavior() == WebSocketBehavior.CLIENT) + this.sslContextFactory = new SslContextFactory.Client(); + else if (policy.getBehavior() == WebSocketBehavior.SERVER) + this.sslContextFactory = new SslContextFactory.Server(); } else { From 33fe55c3398c05e1b51acc2df8e29c3a8eef2822 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 11 Jun 2019 11:25:50 -0500 Subject: [PATCH 02/22] Issue #3708 - use StringUtil alternatives for known slow JVM impls. + StringUtil.replace() + StringUtil.replaceFirst() + StringUtil.sanitizeFileSystemPath() Change existing usages of String.replace() to either use new StringUtil.replace() or other methods elsewhere that better suit that specific need. Signed-off-by: Joakim Erdfelt --- .../jetty/ant/AntWebInfConfiguration.java | 3 +- .../fcgi/client/http/HttpSenderOverFCGI.java | 2 +- .../fcgi/server/proxy/TryFilesFilter.java | 5 +- .../org/eclipse/jetty/http/MimeTypes.java | 2 +- .../jetty/util/StringReplaceBenchmark.java | 119 +++++++++++ .../jetty/nosql/mongodb/MongoUtils.java | 22 +- .../osgi/boot/OSGiWebInfConfiguration.java | 3 +- .../eclipse/jetty/proxy/ProxyServletTest.java | 3 +- .../jetty/quickstart/AttributeNormalizer.java | 9 +- .../rewrite/handler/RedirectRegexRule.java | 10 +- .../rewrite/handler/RewriteRegexRule.java | 17 +- .../jetty/security/PropertyUserStore.java | 3 +- .../org/eclipse/jetty/server/Response.java | 4 +- .../jetty/server/handler/DefaultHandler.java | 2 +- .../jetty/servlet/ResponseHeadersTest.java | 9 +- .../jetty/servlets/CrossOriginFilter.java | 5 +- .../org/eclipse/jetty/util/StringUtil.java | 191 ++++++++++-------- .../java/org/eclipse/jetty/util/URIUtil.java | 14 +- .../jetty/util/component/Dumpable.java | 11 +- .../eclipse/jetty/util/StringUtilTest.java | 83 ++++---- .../jetty/webapp/ClasspathPattern.java | 2 +- .../jetty/webapp/WebInfConfiguration.java | 4 +- .../src/main/java/com/acme/CookieDump.java | 8 +- .../src/main/java/com/acme/Dump.java | 18 +- 24 files changed, 347 insertions(+), 202 deletions(-) create mode 100644 jetty-jmh/src/main/java/org/eclipse/jetty/util/StringReplaceBenchmark.java diff --git a/jetty-ant/src/main/java/org/eclipse/jetty/ant/AntWebInfConfiguration.java b/jetty-ant/src/main/java/org/eclipse/jetty/ant/AntWebInfConfiguration.java index 6c2e9ada926..a82bfcf6478 100644 --- a/jetty-ant/src/main/java/org/eclipse/jetty/ant/AntWebInfConfiguration.java +++ b/jetty-ant/src/main/java/org/eclipse/jetty/ant/AntWebInfConfiguration.java @@ -28,6 +28,7 @@ import java.util.regex.Pattern; import org.apache.tools.ant.AntClassLoader; import org.eclipse.jetty.util.PatternMatcher; +import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.webapp.WebAppClassLoader; import org.eclipse.jetty.webapp.WebAppContext; @@ -88,7 +89,7 @@ public class AntWebInfConfiguration extends WebInfConfiguration } catch (URISyntaxException e) { - containerUris[i] = new URI(u.toString().replaceAll(" ", "%20")); + containerUris[i] = new URI(URIUtil.encodeSpaces(u.toString())); } i++; } diff --git a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpSenderOverFCGI.java b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpSenderOverFCGI.java index 3a5a6b63a4d..4f836fddfe2 100644 --- a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpSenderOverFCGI.java +++ b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpSenderOverFCGI.java @@ -88,7 +88,7 @@ public class HttpSenderOverFCGI extends HttpSender for (HttpField field : headers) { String name = field.getName(); - String fcgiName = "HTTP_" + name.replaceAll("-", "_").toUpperCase(Locale.ENGLISH); + String fcgiName = "HTTP_" + name.replace('-', '_').toUpperCase(Locale.ENGLISH); fcgiHeaders.add(fcgiName, field.getValue()); } diff --git a/jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/proxy/TryFilesFilter.java b/jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/proxy/TryFilesFilter.java index 3964dea1150..7a047c6c1cc 100644 --- a/jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/proxy/TryFilesFilter.java +++ b/jetty-fcgi/fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/proxy/TryFilesFilter.java @@ -24,7 +24,6 @@ import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; - import javax.servlet.Filter; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; @@ -35,6 +34,8 @@ import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.util.StringUtil; + /** * Inspired by nginx's try_files functionality. *

@@ -132,7 +133,7 @@ public class TryFilesFilter implements Filter path += info; if (!path.startsWith("/")) path = "/" + path; - return value.replaceAll("\\$path", path); + return StringUtil.replace(value, "$path", path); } @Override diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java b/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java index 333163dd53f..3b428888e80 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java @@ -196,7 +196,7 @@ public class MimeTypes int charset=type.toString().indexOf(";charset="); if (charset>0) { - String alt=type.toString().replace(";charset=","; charset="); + String alt = StringUtil.replace(type.toString(), ";charset=", "; charset="); CACHE.put(alt,type); TYPES.put(alt,type.asBuffer()); } diff --git a/jetty-jmh/src/main/java/org/eclipse/jetty/util/StringReplaceBenchmark.java b/jetty-jmh/src/main/java/org/eclipse/jetty/util/StringReplaceBenchmark.java new file mode 100644 index 00000000000..61419271778 --- /dev/null +++ b/jetty-jmh/src/main/java/org/eclipse/jetty/util/StringReplaceBenchmark.java @@ -0,0 +1,119 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.util; + +import java.util.concurrent.TimeUnit; + +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; +import org.openjdk.jmh.runner.Runner; +import org.openjdk.jmh.runner.RunnerException; +import org.openjdk.jmh.runner.options.Options; +import org.openjdk.jmh.runner.options.OptionsBuilder; + +@Fork(value = 3) +@State(Scope.Benchmark) +@Warmup(iterations = 4, time = 1, timeUnit = TimeUnit.SECONDS) +@Measurement(iterations = 4, time = 1, timeUnit = TimeUnit.SECONDS) +public class StringReplaceBenchmark +{ + @Param({"3", "100", "1000"}) + int size; + + @Param({"0", "1", "3", "50"}) + int matches; + + String input; + + @Setup(Level.Trial) + public void setupTrial() throws Exception + { + String pattern = "abc"; + StringBuilder str = new StringBuilder(); + while (str.length() < size) + { + str.append(pattern); + } + + if (matches > 0) + { + int partSize = (int)((double)str.length() / (double)matches); + for (int i = 0; i < matches; i++) + { + str.insert((i * partSize), "'"); + } + } + input = str.toString(); + } + + @Benchmark + public void testJavaStringReplace_Growth(Blackhole blackhole) + { + blackhole.consume(input.replace("'", "FOOBAR")); + } + + @Benchmark + public void testJavaStringReplace_Same(Blackhole blackhole) + { + blackhole.consume(input.replace("'", "X")); + } + + @Benchmark + public void testJavaStringReplace_Reduce(Blackhole blackhole) + { + blackhole.consume(input.replace("'", "")); + } + + @Benchmark + public void testJettyStringUtilReplace_Growth(Blackhole blackhole) + { + blackhole.consume(StringUtil.replace(input, "'", "FOOBAR")); + } + + @Benchmark + public void testJettyStringUtilReplace_Same(Blackhole blackhole) + { + blackhole.consume(StringUtil.replace(input, "'", "X")); + } + + @Benchmark + public void testJettyStringUtilReplace_Reduce(Blackhole blackhole) + { + blackhole.consume(StringUtil.replace(input, "'", "")); + } + + public static void main(String[] args) throws RunnerException + { + Options opt = new OptionsBuilder() + .include(StringReplaceBenchmark.class.getSimpleName()) +// .addProfiler(GCProfiler.class) + .forks(1) + .build(); + + new Runner(opt).run(); + } +} diff --git a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoUtils.java b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoUtils.java index 9dfec359bba..a66ca44cc54 100644 --- a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoUtils.java +++ b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoUtils.java @@ -27,10 +27,10 @@ import java.util.Date; import java.util.HashMap; import java.util.Map; -import org.eclipse.jetty.util.ClassLoadingObjectInputStream; - import com.mongodb.BasicDBObject; import com.mongodb.DBObject; +import org.eclipse.jetty.util.ClassLoadingObjectInputStream; +import org.eclipse.jetty.util.StringUtil; /** * MongoUtils @@ -69,23 +69,23 @@ public class MongoUtils throw new IllegalStateException(valueToDecode.getClass().toString()); } } - - - + public static String decodeName(String name) { - return name.replace("%2E",".").replace("%25","%"); + String cleaned = name; + cleaned = StringUtil.replace(cleaned, "%2E", "."); + cleaned = StringUtil.replace(cleaned, "%25", "%"); + return cleaned; } - - public static String encodeName(String name) { - return name.replace("%","%25").replace(".","%2E"); + String cleaned = name; + cleaned = StringUtil.replace(cleaned, "%", "%25"); + cleaned = StringUtil.replace(cleaned, ".", "%2E"); + return cleaned; } - - public static Object encodeName(Object value) throws IOException { if (value instanceof Number || value instanceof String || value instanceof Boolean || value instanceof Date) diff --git a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/OSGiWebInfConfiguration.java b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/OSGiWebInfConfiguration.java index 11af9ca7985..1ba0c301e91 100644 --- a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/OSGiWebInfConfiguration.java +++ b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/OSGiWebInfConfiguration.java @@ -35,6 +35,7 @@ import java.util.regex.Pattern; import org.eclipse.jetty.osgi.boot.utils.BundleFileLocatorHelperFactory; import org.eclipse.jetty.osgi.boot.utils.Util; import org.eclipse.jetty.osgi.boot.utils.internal.PackageAdminServiceTracker; +import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.resource.Resource; @@ -339,7 +340,7 @@ public class OSGiWebInfConfiguration extends WebInfConfiguration } catch (URISyntaxException e) { - uri = new URI(url.toString().replaceAll(" ", "%20")); + uri = new URI(URIUtil.encodeSpaces(url.toString())); } String key = resourcePath.startsWith("/") ? resourcePath.substring(1) : resourcePath; resourceMap.put(key + ";" + fragment.getSymbolicName(), Resource.newResource(uri)); diff --git a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyServletTest.java b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyServletTest.java index 8791899bb18..71afb0e0904 100644 --- a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyServletTest.java +++ b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyServletTest.java @@ -43,7 +43,6 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.stream.Stream; import java.util.zip.GZIPOutputStream; - import javax.servlet.AsyncContext; import javax.servlet.AsyncEvent; import javax.servlet.AsyncListener; @@ -670,7 +669,7 @@ public class ProxyServletTest // Make the request to the proxy, it should transparently forward to the server ContentResponse response = client.newRequest("localhost", proxyConnector.getLocalPort()) - .path((prefix + target).replaceAll("//", "/")) + .path((prefix + target).replace("//", "/")) .timeout(5, TimeUnit.SECONDS) .send(); assertEquals(200, response.getStatus()); diff --git a/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/AttributeNormalizer.java b/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/AttributeNormalizer.java index c153be32d3c..522bcccf6b5 100644 --- a/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/AttributeNormalizer.java +++ b/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/AttributeNormalizer.java @@ -37,6 +37,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Stream; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.resource.Resource; @@ -454,13 +455,7 @@ public class AttributeNormalizer // leftover expanded.append(str.substring(offset)); - // special case for "$$" - if (expanded.indexOf("$$") >= 0) - { - return expanded.toString().replaceAll("\\$\\$","\\$"); - } - - return expanded.toString(); + return StringUtil.replace(expanded.toString(), "$$", "$"); } private String getString(String property) diff --git a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectRegexRule.java b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectRegexRule.java index 1f023bd432a..acd31fa7960 100644 --- a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectRegexRule.java +++ b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RedirectRegexRule.java @@ -24,6 +24,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.annotation.Name; /** @@ -93,7 +94,7 @@ public class RedirectRegexRule extends RegexRule for (int g = 1; g <= matcher.groupCount(); g++) { String group = matcher.group(g); - target = target.replaceAll("\\$" + g, group); + target = StringUtil.replace(target, "$" + g, group); } target = response.encodeRedirectURL(target); @@ -110,6 +111,11 @@ public class RedirectRegexRule extends RegexRule @Override public String toString() { - return String.format("%s[%d>%s]", super.toString(), _statusCode, _location); + StringBuilder str = new StringBuilder(); + str.append(super.toString()); + str.append('[').append(_statusCode); + str.append('>').append(_location); + str.append(']'); + return str.toString(); } } diff --git a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RewriteRegexRule.java b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RewriteRegexRule.java index 1888fdc9815..f41cbdb9032 100644 --- a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RewriteRegexRule.java +++ b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RewriteRegexRule.java @@ -20,11 +20,11 @@ package org.eclipse.jetty.rewrite.handler; import java.io.IOException; import java.util.regex.Matcher; - import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.annotation.Name; /** @@ -96,15 +96,22 @@ public class RewriteRegexRule extends RegexRule implements Rule.ApplyURI group=""; else group = Matcher.quoteReplacement(group); - target=target.replaceAll("\\$"+g,group); - if (query!=null) - query=query.replaceAll("\\$"+g,group); + String dollarGroup = "$" + g; + target = StringUtil.replace(target, dollarGroup, group); + if (query != null) + query = StringUtil.replace(query, dollarGroup, group); } if (query!=null) { if (_queryGroup) - query=query.replace("$Q",request.getQueryString()==null?"":request.getQueryString()); + { + String replacement = ""; + if (request.getQueryString() != null) + replacement = request.getQueryString(); + + query = StringUtil.replace(query, "$Q", replacement); + } request.setAttribute("org.eclipse.jetty.rewrite.handler.RewriteRegexRule.Q",query); } diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/PropertyUserStore.java b/jetty-security/src/main/java/org/eclipse/jetty/security/PropertyUserStore.java index e43f4719d30..9d0ea1004b4 100644 --- a/jetty-security/src/main/java/org/eclipse/jetty/security/PropertyUserStore.java +++ b/jetty-security/src/main/java/org/eclipse/jetty/security/PropertyUserStore.java @@ -140,7 +140,8 @@ public class PropertyUserStore extends UserStore implements PathWatcher.Listener int bang_slash = uri.indexOf("!/"); if (colon < 0 || bang_slash < 0 || colon > bang_slash) throw new IllegalArgumentException("Not resolved JarFile resource: " + uri); - String entry_path = uri.substring(colon + 2).replace("!/", "__").replace('/', '_').replace('.', '_'); + + String entry_path = StringUtil.sanitizeFileSystemPath(uri.substring(colon + 2)); Path tmpDirectory = Files.createTempDirectory("users_store"); tmpDirectory.toFile().deleteOnExit(); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java index f5733470fb2..1809d5b1cea 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java @@ -52,12 +52,10 @@ import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.http.PreEncodedHttpField; -import org.eclipse.jetty.http.Syntax; import org.eclipse.jetty.io.RuntimeIOException; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.handler.ErrorHandler; import org.eclipse.jetty.server.session.SessionHandler; -import org.eclipse.jetty.util.QuotedStringTokenizer; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.log.Log; @@ -237,7 +235,7 @@ public class Response implements HttpServletResponse if (i >= 0) { httpOnly = true; - comment = comment.replace(HTTP_ONLY_COMMENT, "").trim(); + comment = StringUtil.replace(comment.trim(), HTTP_ONLY_COMMENT, ""); if (comment.length() == 0) comment = null; } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/DefaultHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/DefaultHandler.java index 06c74a91a21..21e814f03d6 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/DefaultHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/DefaultHandler.java @@ -164,7 +164,7 @@ public class DefaultHandler extends AbstractHandler { writer.append(""); } - writer.append(contextPath.replaceAll("%", "%")); + writer.append(StringUtil.replace(contextPath, "%", "%")); if (context.isRunning()) { writer.append(""); diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ResponseHeadersTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ResponseHeadersTest.java index ffaaed78bf0..eb801824bea 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ResponseHeadersTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ResponseHeadersTest.java @@ -18,13 +18,9 @@ package org.eclipse.jetty.servlet; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; - import java.io.IOException; import java.net.URLDecoder; import java.nio.ByteBuffer; - import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -38,6 +34,9 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + public class ResponseHeadersTest { public static class SimulateUpgradeServlet extends HttpServlet @@ -144,7 +143,7 @@ public class ResponseHeadersTest assertThat("Response Code",response.getStatus(),is(200)); assertThat("Response Header Content-Type",response.get("Content-Type"),is("text/plain;charset=UTF-8")); - String expected = actualPathInfo.replaceAll("%0A", " "); // replace OBS fold with space + String expected = actualPathInfo.replace("%0A", " "); // replace OBS fold with space expected = URLDecoder.decode(expected, "utf-8"); // decode the rest expected = expected.trim(); // trim whitespace at start/end assertThat("Response Header X-example", response.get("X-Example"), is(expected)); diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/CrossOriginFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/CrossOriginFilter.java index 692be858f1c..4c3393a90a4 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/CrossOriginFilter.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/CrossOriginFilter.java @@ -26,7 +26,6 @@ import java.util.Enumeration; import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; - import javax.servlet.Filter; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; @@ -368,8 +367,8 @@ public class CrossOriginFilter implements Filter private String parseAllowedWildcardOriginToRegex(String allowedOrigin) { - String regex = allowedOrigin.replace(".", "\\."); - return regex.replace("*", ".*"); // we want to be greedy here to match multiple subdomains, thus we use .* + String regex = StringUtil.replace(allowedOrigin, ".", "\\."); + return StringUtil.replace(regex, "*", ".*"); // we want to be greedy here to match multiple subdomains, thus we use .* } private boolean isSimpleRequest(HttpServletRequest request) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java index 9b1ef63603b..0bf5a431194 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java @@ -26,14 +26,13 @@ import java.util.List; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; -/** Fast String Utilities. +/** + * Fast String Utilities. * * These string utilities provide both convenience methods and * performance improvements over most standard library versions. The * main aim of the optimizations is to avoid object creation unless * absolutely required. - * - * */ public class StringUtil { @@ -62,8 +61,7 @@ public class StringUtil CHARSETS.put("iso-8859-1",__ISO_8859_1); CHARSETS.put("iso_8859_1",__ISO_8859_1); } - - /* ------------------------------------------------------------ */ + /** Convert alternate charset names (eg utf8) to normalized * name (eg UTF-8). * @param s the charset to normalize @@ -74,8 +72,7 @@ public class StringUtil String n=CHARSETS.get(s); return (n==null)?s:n; } - - /* ------------------------------------------------------------ */ + /** Convert alternate charset names (eg utf8) to normalized * name (eg UTF-8). * @param s the charset to normalize @@ -88,9 +85,7 @@ public class StringUtil String n=CHARSETS.get(s,offset,length); return (n==null)?s.substring(offset,offset+length):n; } - - /* ------------------------------------------------------------ */ public static final char[] lowercases = { '\000','\001','\002','\003','\004','\005','\006','\007', '\010','\011','\012','\013','\014','\015','\016','\017', @@ -109,7 +104,6 @@ public class StringUtil '\160','\161','\162','\163','\164','\165','\166','\167', '\170','\171','\172','\173','\174','\175','\176','\177' }; - /* ------------------------------------------------------------ */ /** * fast lower case conversion. Only works on ascii (not unicode) * @param s the string to convert @@ -122,7 +116,6 @@ public class StringUtil char[] c = null; int i=s.length(); - // look for first conversion while (i-->0) { @@ -138,7 +131,6 @@ public class StringUtil } } } - while (i-->0) { if(c[i]<=127) @@ -148,8 +140,43 @@ public class StringUtil return c==null?s:new String(c); } + /** + * Replace all characters from input string that are known to have + * special meaning in various filesytems. + * + *

+ * This will replace all of the following characters + * with a "{@code _}" (underscore). + *

+ *
    + *
  • Control Characters
    • + *
  • Anything not 7-bit printable ASCII
    • + *
  • Special characters "{@code !|/.,:&><\?*"}"
    • + *
+ * + * @param str the raw input string + * @return the sanitized output string. + */ + public static String sanitizeFileSystemPath(String str) + { + char[] chars = str.toCharArray(); + int len = chars.length; + for (int i = 0; i < len; i++) + { + char c = chars[i]; + if ((c <= 0x1F) || // control characters + (c >= 0x7F) || // over 7-bit printable ASCII + (c == '!') || (c == '|') || (c == '.') || + (c == ':') || (c == '>') || (c == '<') || + (c == '?') || (c == '/') || (c == '\\') || + (c == '*') || (c == '"') || (c == '&')) + { + chars[i] = '_'; + } + } + return String.valueOf(chars); + } - /* ------------------------------------------------------------ */ public static boolean startsWithIgnoreCase(String s,String w) { if (w==null) @@ -174,13 +201,11 @@ public class StringUtil } return true; } - - /* ------------------------------------------------------------ */ + public static boolean endsWithIgnoreCase(String s,String w) { if (w==null) return true; - if (s==null) return false; @@ -206,8 +231,7 @@ public class StringUtil } return true; } - - /* ------------------------------------------------------------ */ + /** * returns the next index of a character from the chars string * @param s the input string to search @@ -221,10 +245,13 @@ public class StringUtil return i; return -1; } - - /* ------------------------------------------------------------ */ + /** - * replace substrings within string. + * Replace substrings within string. + *

+ * Fast replacement for {@code java.lang.String#}{@link String#replace(CharSequence, CharSequence)} + *

+ * * @param s the input string * @param sub the string to look for * @param with the string to replace with @@ -232,27 +259,56 @@ public class StringUtil */ public static String replace(String s, String sub, String with) { - int c=0; - int i=s.indexOf(sub,c); + int c = 0; + int i = s.indexOf(sub, c); if (i == -1) + { return s; - - StringBuilder buf = new StringBuilder(s.length()+with.length()); - + } + StringBuilder buf = new StringBuilder(s.length() + with.length()); do { - buf.append(s.substring(c,i)); + buf.append(s, c, i); buf.append(with); - c=i+sub.length(); - } while ((i=s.indexOf(sub,c))!=-1); - - if (c + * Fast replacement for {@code java.lang.String#}{@link String#replaceFirst(String, String)}, but without + * Regex support. + *

+ * + * @param original the original string + * @param target the target string to look for + * @param replacement the replacement string to use + * @return the replaced string + */ + public static String replaceFirst(String original, String target, String replacement) + { + int idx = original.indexOf(target); + if (idx == -1) + return original; + + int offset = 0; + int originalLen = original.length(); + StringBuilder buf = new StringBuilder(originalLen + replacement.length()); + buf.append(original, offset, idx); + offset += idx + target.length(); + buf.append(replacement); + buf.append(original, offset, originalLen); + + return buf.toString(); + } + /** Remove single or double quotes. * @param s the input string * @return the string with quotes removed @@ -263,8 +319,6 @@ public class StringUtil return QuotedStringTokenizer.unquote(s); } - - /* ------------------------------------------------------------ */ /** Append substring to StringBuilder * @param buf StringBuilder to append to * @param s String to append from @@ -288,8 +342,6 @@ public class StringUtil } } - - /* ------------------------------------------------------------ */ /** * append hex digit * @param buf the buffer to append to @@ -310,7 +362,6 @@ public class StringUtil buf.append((char)c); } - /* ------------------------------------------------------------ */ /** * Append 2 digits (zero padded) to the StringBuffer * @@ -325,8 +376,7 @@ public class StringUtil buf.append((char)(i%10+'0')); } } - - /* ------------------------------------------------------------ */ + /** * Append 2 digits (zero padded) to the StringBuilder * @@ -341,8 +391,7 @@ public class StringUtil buf.append((char)(i%10+'0')); } } - - /* ------------------------------------------------------------ */ + /** Return a non null string. * @param s String * @return The string passed in or empty string if it is null. @@ -353,8 +402,7 @@ public class StringUtil return ""; return s; } - - /* ------------------------------------------------------------ */ + public static boolean equals(String s,char[] buf, int offset, int length) { if (s.length()!=length) @@ -365,13 +413,11 @@ public class StringUtil return true; } - /* ------------------------------------------------------------ */ public static String toUTF8String(byte[] b,int offset,int length) { return new String(b,offset,length,StandardCharsets.UTF_8); } - /* ------------------------------------------------------------ */ public static String toString(byte[] b,int offset,int length,String charset) { try @@ -380,6 +426,7 @@ public class StringUtil } catch (UnsupportedEncodingException e) { + LOG.warn(e); throw new IllegalArgumentException(e); } } @@ -431,7 +478,6 @@ public class StringUtil return -1; } - /* ------------------------------------------------------------ */ /** * Test if a string is null or only has whitespace characters in it. *

@@ -494,7 +540,6 @@ public class StringUtil return str == null || str.isEmpty(); } - /* ------------------------------------------------------------ */ /** * Test if a string is not null and contains at least 1 non-whitespace characters in it. *

@@ -534,14 +579,11 @@ public class StringUtil return false; } - /* ------------------------------------------------------------ */ public static boolean isUTF8(String charset) { return __UTF8.equalsIgnoreCase(charset)||__UTF8.equalsIgnoreCase(normalizeCharset(charset)); } - - /* ------------------------------------------------------------ */ public static String printable(String name) { if (name==null) @@ -556,7 +598,7 @@ public class StringUtil return buf.toString(); } - /* ------------------------------------------------------------ */ + public static String printable(byte[] b) { StringBuilder buf = new StringBuilder(); @@ -592,13 +634,10 @@ public class StringUtil } catch(Exception e) { - LOG.warn(e); return s.getBytes(); } } - - - + /** * Converts a binary SID to a string SID * @@ -631,7 +670,6 @@ public class StringUtil // the number of subAuthorities we need to attach int subAuthorityCount = sidBytes[1]; - // attach each of the subAuthorities for (int i = 0; i < subAuthorityCount; ++i) { @@ -670,10 +708,8 @@ public class StringUtil // the revision byte sidBytes[byteCount++] = (byte)Integer.parseInt(sidTokens[1]); - // the # of sub authorities byte sidBytes[byteCount++] = (byte)subAuthorityCount; - // the certAuthority String hexStr = Long.toHexString(Long.parseLong(sidTokens[2])); @@ -681,7 +717,6 @@ public class StringUtil { hexStr = "0" + hexStr; } - // place the certAuthority 6 bytes for ( int i = 0 ; i < hexStr.length(); i = i + 2) { @@ -720,7 +755,6 @@ public class StringUtil int val = 0; boolean started = false; boolean minus = false; - for (int i = from; i < string.length(); i++) { char b = string.charAt(i); @@ -741,7 +775,6 @@ public class StringUtil else break; } - if (started) return minus?(-val):val; throw new NumberFormatException(string); @@ -759,7 +792,6 @@ public class StringUtil long val = 0; boolean started = false; boolean minus = false; - for (int i = 0; i < string.length(); i++) { char b = string.charAt(i); @@ -780,7 +812,6 @@ public class StringUtil else break; } - if (started) return minus?(-val):val; throw new NumberFormatException(string); @@ -799,12 +830,10 @@ public class StringUtil { return null; } - if (str.length() <= maxSize) { return str; } - return str.substring(0,maxSize); } @@ -817,20 +846,18 @@ public class StringUtil { if (s==null) return new String[]{}; - if (!s.startsWith("[") || !s.endsWith("]")) throw new IllegalArgumentException(); if (s.length()==2) return new String[]{}; - return csvSplit(s,1,s.length()-2); } /** - * Parse a CSV string using {@link #csvSplit(List,String, int, int)} - * @param s The string to parse - * @return An array of parsed values. - */ + * Parse a CSV string using {@link #csvSplit(List,String, int, int)} + * @param s The string to parse + * @return An array of parsed values. + */ public static String[] csvSplit(String s) { if (s==null) @@ -851,13 +878,12 @@ public class StringUtil return null; if (off<0 || len<0 || off>s.length()) throw new IllegalArgumentException(); - List list = new ArrayList<>(); csvSplit(list,s,off,len); return list.toArray(new String[list.size()]); } - enum CsvSplitState { PRE_DATA, QUOTE, SLOSH, DATA, WHITE, POST_DATA }; + enum CsvSplitState { PRE_DATA, QUOTE, SLOSH, DATA, WHITE, POST_DATA } /** Split a quoted comma separated string to a list *

Handle rfc4180-like @@ -890,7 +916,6 @@ public class StringUtil case PRE_DATA: if (Character.isWhitespace(ch)) continue; - if ('"'==ch) { state=CsvSplitState.QUOTE; @@ -902,11 +927,9 @@ public class StringUtil list.add(""); continue; } - state=CsvSplitState.DATA; out.append(ch); continue; - case DATA: if (Character.isWhitespace(ch)) { @@ -923,7 +946,6 @@ public class StringUtil state=CsvSplitState.PRE_DATA; continue; } - out.append(ch); continue; @@ -947,7 +969,6 @@ public class StringUtil out.append(ch); last=-1; continue; - case QUOTE: if ('\\'==ch) { @@ -978,13 +999,11 @@ public class StringUtil continue; } } - switch(state) { case PRE_DATA: case POST_DATA: break; - case DATA: case QUOTE: case SLOSH: @@ -1011,7 +1030,6 @@ public class StringUtil loop: for (;iThis method calls {@link String#valueOf(Object)} unless the object is null, * in which case null is returned

diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java index 50394f7c6fb..17da05adf80 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java @@ -268,7 +268,19 @@ public class URIUtil return buf; } - + + /** + * Encode a raw URI String and convert any raw spaces to + * their "%20" equivalent. + * + * @param str input raw string + * @return output with spaces converted to "%20" + */ + public static String encodeSpaces(String str) + { + return StringUtil.replace(str, " ", "%20"); + } + /* ------------------------------------------------------------ */ /** Encode a URI path. * @param path The path the encode diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Dumpable.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Dumpable.java index 78229fc30ba..ee9fc9df110 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Dumpable.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Dumpable.java @@ -26,6 +26,7 @@ import java.util.Iterator; import java.util.Map; import java.util.stream.Stream; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.annotation.ManagedOperation; @@ -99,9 +100,15 @@ public interface Dumpable else if (o instanceof Map) s = String.format("%s@%x{size=%d}",o.getClass().getName(),o.hashCode(),((Map)o).size()); else if (o instanceof Dumpable) - s = ((Dumpable)o).dumpSelf().replace("\r\n","|").replace("\n","|"); + { + s = StringUtil.replace(((Dumpable)o).dumpSelf(), "\r\n", "|") + .replace('\n', '|'); + } else - s = String.valueOf(o).replace("\r\n","|").replace("\n","|"); + { + s = StringUtil.replace(String.valueOf(o), "\r\n", "|") + .replace('\n', '|'); + } if (o instanceof LifeCycle) out.append(s).append(" - ").append((AbstractLifeCycle.getState((LifeCycle)o))).append("\n"); diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/StringUtilTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/StringUtilTest.java index 51d53110410..f244de11b03 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/StringUtilTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/StringUtilTest.java @@ -18,19 +18,21 @@ package org.eclipse.jetty.util; -import java.nio.charset.StandardCharsets; -import java.util.concurrent.TimeUnit; - +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.emptyArray; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertTrue; public class StringUtilTest @@ -98,7 +100,36 @@ public class StringUtilTest s=" \u0690bc "; assertEquals(StringUtil.replace(s, "\u0690bc", "xyz")," xyz "); + } + public static Stream replaceFirstArgs() { + List data = new ArrayList<>(); + + // [original, target, replacement, expected] + + // no match + data.add(new String[]{ "abc", "z", "foo", "abc" }); + + // matches at start of string + data.add(new String[]{ "abc", "a", "foo", "foobc" }); + data.add(new String[]{ "abcabcabc", "a", "foo", "foobcabcabc" }); + + // matches in middle of string + data.add(new String[]{ "abc", "b", "foo", "afooc" }); + data.add(new String[]{ "abcabcabc", "b", "foo", "afoocabcabc" }); + data.add(new String[]{ "abcabcabc", "cab", "X", "abXcabc" }); + + // matches at end of string + data.add(new String[]{ "abc", "c", "foo", "abfoo" }); + + return data.stream(); + } + + @ParameterizedTest + @MethodSource(value = "replaceFirstArgs") + public void testReplaceFirst(String original, String target, String replacement, String expected) + { + assertThat(StringUtil.replaceFirst(original, target, replacement), is(expected)); } @Test @@ -165,50 +196,6 @@ public class StringUtilTest assertEquals(sid5, StringUtil.sidBytesToString(sid5Bytes)); assertEquals(sid6, StringUtil.sidBytesToString(sid6Bytes)); assertEquals(sid12, StringUtil.sidBytesToString(sid12Bytes)); - - } - - - public static void main(String[] arg) throws Exception - { - String string = "Now \u0690xxxxxxxx"; - System.err.println(string); - byte[] bytes=string.getBytes(StandardCharsets.UTF_8); - System.err.println(new String(bytes)); - System.err.println(bytes.length); - long calc=0; - Utf8StringBuffer strbuf = new Utf8StringBuffer(bytes.length); - for (int i=0;i<10;i++) - { - long s1=TimeUnit.NANOSECONDS.toMillis(System.nanoTime()); - for (int j=1000000; j-->0;) - { - calc+=new String(bytes,0,bytes.length,StandardCharsets.UTF_8).hashCode(); - } - long s2=TimeUnit.NANOSECONDS.toMillis(System.nanoTime()); - for (int j=1000000; j-->0;) - { - calc+=StringUtil.toUTF8String(bytes,0,bytes.length).hashCode(); - } - long s3=TimeUnit.NANOSECONDS.toMillis(System.nanoTime()); - for (int j=1000000; j-->0;) - { - Utf8StringBuffer buffer = new Utf8StringBuffer(bytes.length); - buffer.append(bytes,0,bytes.length); - calc+=buffer.toString().hashCode(); - } - long s4=TimeUnit.NANOSECONDS.toMillis(System.nanoTime()); - for (int j=1000000; j-->0;) - { - strbuf.reset(); - strbuf.append(bytes,0,bytes.length); - calc+=strbuf.toString().hashCode(); - } - long s5=TimeUnit.NANOSECONDS.toMillis(System.nanoTime()); - - System.err.println((s2-s1)+", "+(s3-s2)+", "+(s4-s3)+", "+(s5-s4)); - } - System.err.println(calc); } @Test 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 ee203b35e50..1ca05559cc9 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 @@ -703,7 +703,7 @@ public class ClasspathPattern extends AbstractSet name=name.substring(0,name.length()-6); // Treat path elements as packages for name matching - name=name.replace("/","."); + name = name.replace('/', '.'); return combine(_packageOrNamePatterns, name, _locations, ()-> { diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java index b94f7881a69..20aec0b824d 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java @@ -38,6 +38,7 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.JavaVersion; import org.eclipse.jetty.util.PatternMatcher; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -210,7 +211,8 @@ public class WebInfConfiguration extends AbstractConfiguration } catch (URISyntaxException e) { - containerUris.add(new URI(u.toString().replaceAll(" ", "%20"))); + String fixedUriStr = StringUtil.replace(u.toString(), " ", "%20"); + containerUris.add(new URI(fixedUriStr)); } } } diff --git a/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/CookieDump.java b/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/CookieDump.java index 48d79149dec..68c0c98c128 100644 --- a/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/CookieDump.java +++ b/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/CookieDump.java @@ -21,7 +21,6 @@ package com.acme; import java.io.IOException; import java.io.PrintWriter; import java.util.concurrent.TimeUnit; - import javax.servlet.RequestDispatcher; import javax.servlet.ServletException; import javax.servlet.http.Cookie; @@ -29,7 +28,6 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; - /** * Test Servlet Cookies. */ @@ -117,9 +115,9 @@ public class CookieDump extends HttpServlet { if (string==null) return null; - string=string.replace("&", "&"); - string=string.replace( "<", "<"); - string=string.replace( ">", ">"); + string = string.replace("&", "&"); + string = string.replace( "<", "<"); + string = string.replace( ">", ">"); return string; } diff --git a/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/Dump.java b/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/Dump.java index 1110e8bfeff..e2a2f575feb 100644 --- a/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/Dump.java +++ b/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/Dump.java @@ -36,7 +36,6 @@ import java.util.Enumeration; import java.util.Locale; import java.util.Timer; import java.util.TimerTask; - import javax.servlet.AsyncContext; import javax.servlet.AsyncEvent; import javax.servlet.AsyncListener; @@ -57,13 +56,14 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponseWrapper; import javax.servlet.http.Part; - -/** +/** * Dump Servlet Request. */ @SuppressWarnings("serial") public class Dump extends HttpServlet { + /** Zero Width Space, to allow text to be wrapped at designated spots */ + private static final String ZWSP = "​"; boolean fixed; Timer _timer; @@ -647,7 +647,7 @@ public class Dump extends HttpServlet { name= (String)a.nextElement(); pout.write("\n"); - pout.write(""+name.replace("."," .")+": "); + pout.write("" + name.replace(".", ZWSP + ".") + ": "); Object value=request.getAttribute(name); if (value instanceof File) { @@ -678,7 +678,7 @@ public class Dump extends HttpServlet { name= (String)a.nextElement(); pout.write("\n"); - pout.write(""+name.replace("."," .")+": "); + pout.write("" + name.replace(".", ZWSP + ".") + ": "); pout.write(""+ toString(getServletContext().getInitParameter(name)) + ""); } @@ -689,7 +689,7 @@ public class Dump extends HttpServlet { name= (String)a.nextElement(); pout.write("\n"); - pout.write(""+name.replace("."," .")+": "); + pout.write("" + name.replace(".", ZWSP + ".") + ": "); pout.write(""+"
" + toString(getServletContext().getAttribute(name)) + "
"+""); } @@ -1055,9 +1055,9 @@ public class Dump extends HttpServlet { if (s==null) return "null"; - s=s.replaceAll("&","&"); - s=s.replaceAll("<","<"); - s=s.replaceAll(">",">"); + s = s.replace("&","&"); + s = s.replace("<","<"); + s = s.replace(">",">"); return s; } } From 245388210311e3fce7c4300ec9c848ca01fc9637 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 11 Jun 2019 13:02:40 -0500 Subject: [PATCH 03/22] Issue #3708 - Reverting change to RewriteRegexRule + Moving away from Regex / Pattern isn't appropriate here, as the entire class is dedicated to Regex behaviors. Signed-off-by: Joakim Erdfelt --- .../jetty/rewrite/handler/RewriteRegexRule.java | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RewriteRegexRule.java b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RewriteRegexRule.java index f41cbdb9032..5423f88170c 100644 --- a/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RewriteRegexRule.java +++ b/jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RewriteRegexRule.java @@ -24,7 +24,6 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.annotation.Name; /** @@ -96,22 +95,15 @@ public class RewriteRegexRule extends RegexRule implements Rule.ApplyURI group=""; else group = Matcher.quoteReplacement(group); - String dollarGroup = "$" + g; - target = StringUtil.replace(target, dollarGroup, group); - if (query != null) - query = StringUtil.replace(query, dollarGroup, group); + target=target.replaceAll("\\$"+g,group); + if (query!=null) + query=query.replaceAll("\\$"+g,group); } if (query!=null) { if (_queryGroup) - { - String replacement = ""; - if (request.getQueryString() != null) - replacement = request.getQueryString(); - - query = StringUtil.replace(query, "$Q", replacement); - } + query=query.replace("$Q",request.getQueryString()==null?"":request.getQueryString()); request.setAttribute("org.eclipse.jetty.rewrite.handler.RewriteRegexRule.Q",query); } From 8f53d14e15aa27bc48024bfba1bbaada3630c20b Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 12 Jun 2019 10:51:15 +0200 Subject: [PATCH 04/22] Issue #3758 - Avoid sending empty trailer frames for http/2 requests. Modified the sender logic to allow specific subclasses to decide when to send the trailers, if any. This allows HTTP/2 to correctly compute the end_stream flag and avoid sending empty trailers frames with end_stream=true. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/client/HttpSender.java | 54 +------ .../jetty/client/http/HttpSenderOverHTTP.java | 122 ++------------- .../fcgi/client/http/HttpSenderOverFCGI.java | 6 - .../jetty/http2/client/TrailersTest.java | 22 +-- .../client/http/HttpSenderOverHTTP2.java | 129 +++++++++++----- .../client/http/RequestTrailersTest.java | 142 ++++++++++++++++++ 6 files changed, 259 insertions(+), 216 deletions(-) create mode 100644 jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/RequestTrailersTest.java diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java index 694ff157509..99b865bd70a 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java @@ -22,12 +22,10 @@ import java.nio.ByteBuffer; import java.util.concurrent.Executor; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Supplier; import org.eclipse.jetty.client.api.ContentProvider; import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.client.api.Result; -import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.util.BufferUtil; @@ -67,7 +65,6 @@ public abstract class HttpSender implements AsyncContentProvider.Listener private final AtomicReference senderState = new AtomicReference<>(SenderState.IDLE); private final Callback commitCallback = new CommitCallback(); private final IteratingCallback contentCallback = new ContentCallback(); - private final Callback trailersCallback = new TrailersCallback(); private final Callback lastCallback = new LastCallback(); private final HttpChannel channel; private HttpContent content; @@ -444,15 +441,6 @@ public abstract class HttpSender implements AsyncContentProvider.Listener */ protected abstract void sendContent(HttpExchange exchange, HttpContent content, Callback callback); - /** - * Implementations should send the HTTP trailers and notify the given {@code callback} of the - * result of this operation. - * - * @param exchange the exchange to send - * @param callback the callback to notify - */ - protected abstract void sendTrailers(HttpExchange exchange, Callback callback); - protected void reset() { HttpContent content = this.content; @@ -745,20 +733,10 @@ public abstract class HttpSender implements AsyncContentProvider.Listener if (content == null) return; - HttpRequest request = exchange.getRequest(); - Supplier trailers = request.getTrailers(); - boolean hasContent = content.hasContent(); - if (!hasContent) + if (!content.hasContent()) { - if (trailers == null) - { - // No trailers or content to send, we are done. - someToSuccess(exchange); - } - else - { - sendTrailers(exchange, lastCallback); - } + // No content to send, we are done. + someToSuccess(exchange); } else { @@ -859,9 +837,7 @@ public abstract class HttpSender implements AsyncContentProvider.Listener if (lastContent) { - HttpRequest request = exchange.getRequest(); - Supplier trailers = request.getTrailers(); - sendContent(exchange, content, trailers == null ? lastCallback : trailersCallback); + sendContent(exchange, content, lastCallback); return Action.IDLE; } @@ -925,28 +901,6 @@ public abstract class HttpSender implements AsyncContentProvider.Listener } } - private class TrailersCallback implements Callback - { - @Override - public void succeeded() - { - HttpExchange exchange = getHttpExchange(); - if (exchange == null) - return; - sendTrailers(exchange, lastCallback); - } - - @Override - public void failed(Throwable x) - { - HttpContent content = HttpSender.this.content; - if (content == null) - return; - content.failed(x); - anyToFailure(x); - } - } - private class LastCallback implements Callback { @Override diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpSenderOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpSenderOverHTTP.java index 4cab4008904..1f8d04397da 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpSenderOverHTTP.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpSenderOverHTTP.java @@ -59,7 +59,7 @@ public class HttpSenderOverHTTP extends HttpSender { try { - new HeadersCallback(exchange, content, callback, getHttpChannel().getHttpConnection()).iterate(); + new HeadersCallback(exchange, content, callback).iterate(); } catch (Throwable x) { @@ -83,8 +83,8 @@ public class HttpSenderOverHTTP extends HttpSender HttpGenerator.Result result = generator.generateRequest(null, null, chunk, contentBuffer, lastContent); if (LOG.isDebugEnabled()) LOG.debug("Generated content ({} bytes) - {}/{}", - contentBuffer == null ? -1 : contentBuffer.remaining(), - result, generator); + contentBuffer == null ? -1 : contentBuffer.remaining(), + result, generator); switch (result) { case NEED_CHUNK: @@ -138,21 +138,6 @@ public class HttpSenderOverHTTP extends HttpSender } } - @Override - protected void sendTrailers(HttpExchange exchange, Callback callback) - { - try - { - new TrailersCallback(callback).iterate(); - } - catch (Throwable x) - { - if (LOG.isDebugEnabled()) - LOG.debug(x); - callback.failed(x); - } - } - @Override protected void reset() { @@ -191,19 +176,17 @@ public class HttpSenderOverHTTP extends HttpSender private final HttpExchange exchange; private final Callback callback; private final MetaData.Request metaData; - private final HttpConnectionOverHTTP httpConnectionOverHTTP; private ByteBuffer headerBuffer; private ByteBuffer chunkBuffer; private ByteBuffer contentBuffer; private boolean lastContent; private boolean generated; - public HeadersCallback(HttpExchange exchange, HttpContent content, Callback callback, HttpConnectionOverHTTP httpConnectionOverHTTP) + public HeadersCallback(HttpExchange exchange, HttpContent content, Callback callback) { super(false); this.exchange = exchange; this.callback = callback; - this.httpConnectionOverHTTP = httpConnectionOverHTTP; HttpRequest request = exchange.getRequest(); ContentProvider requestContent = request.getContent(); @@ -231,10 +214,10 @@ public class HttpSenderOverHTTP extends HttpSender HttpGenerator.Result result = generator.generateRequest(metaData, headerBuffer, chunkBuffer, contentBuffer, lastContent); if (LOG.isDebugEnabled()) LOG.debug("Generated headers ({} bytes), chunk ({} bytes), content ({} bytes) - {}/{}", - headerBuffer == null ? -1 : headerBuffer.remaining(), - chunkBuffer == null ? -1 : chunkBuffer.remaining(), - contentBuffer == null ? -1 : contentBuffer.remaining(), - result, generator); + headerBuffer == null ? -1 : headerBuffer.remaining(), + chunkBuffer == null ? -1 : chunkBuffer.remaining(), + contentBuffer == null ? -1 : contentBuffer.remaining(), + result, generator); switch (result) { case NEED_HEADER: @@ -249,7 +232,8 @@ public class HttpSenderOverHTTP extends HttpSender } case NEED_CHUNK_TRAILER: { - return Action.SUCCEEDED; + chunkBuffer = httpClient.getByteBufferPool().acquire(httpClient.getRequestBufferSize(), false); + break; } case FLUSH: { @@ -260,11 +244,8 @@ public class HttpSenderOverHTTP extends HttpSender chunkBuffer = BufferUtil.EMPTY_BUFFER; if (contentBuffer == null) contentBuffer = BufferUtil.EMPTY_BUFFER; - - httpConnectionOverHTTP.addBytesOut( BufferUtil.length(headerBuffer) - + BufferUtil.length(chunkBuffer) - + BufferUtil.length(contentBuffer)); - + long bytes = headerBuffer.remaining() + chunkBuffer.remaining() + contentBuffer.remaining(); + getHttpChannel().getHttpConnection().addBytesOut(bytes); endPoint.write(this, headerBuffer, chunkBuffer, contentBuffer); generated = true; return Action.SCHEDULED; @@ -331,83 +312,6 @@ public class HttpSenderOverHTTP extends HttpSender } } - private class TrailersCallback extends IteratingCallback - { - private final Callback callback; - private ByteBuffer chunkBuffer; - - public TrailersCallback(Callback callback) - { - this.callback = callback; - } - - @Override - protected Action process() throws Throwable - { - while (true) - { - HttpGenerator.Result result = generator.generateRequest(null, null, chunkBuffer, null, true); - if (LOG.isDebugEnabled()) - LOG.debug("Generated trailers {}/{}", result, generator); - switch (result) - { - case NEED_CHUNK_TRAILER: - { - chunkBuffer = httpClient.getByteBufferPool().acquire(httpClient.getRequestBufferSize(), false); - break; - } - case FLUSH: - { - EndPoint endPoint = getHttpChannel().getHttpConnection().getEndPoint(); - endPoint.write(this, chunkBuffer); - return Action.SCHEDULED; - } - case SHUTDOWN_OUT: - { - shutdownOutput(); - return Action.SUCCEEDED; - } - case DONE: - { - return Action.SUCCEEDED; - } - default: - { - throw new IllegalStateException(result.toString()); - } - } - } - } - - @Override - public void succeeded() - { - release(); - super.succeeded(); - } - - @Override - public void failed(Throwable x) - { - release(); - callback.failed(x); - super.failed(x); - } - - @Override - protected void onCompleteSuccess() - { - super.onCompleteSuccess(); - callback.succeeded(); - } - - private void release() - { - httpClient.getByteBufferPool().release(chunkBuffer); - chunkBuffer = null; - } - } - private class ByteBufferRecyclerCallback extends Callback.Nested { private final ByteBufferPool pool; @@ -435,7 +339,9 @@ public class HttpSenderOverHTTP extends HttpSender public void failed(Throwable x) { for (ByteBuffer buffer : buffers) + { pool.release(buffer); + } super.failed(x); } } diff --git a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpSenderOverFCGI.java b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpSenderOverFCGI.java index 3a5a6b63a4d..93203f26d00 100644 --- a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpSenderOverFCGI.java +++ b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpSenderOverFCGI.java @@ -125,10 +125,4 @@ public class HttpSenderOverFCGI extends HttpSender getHttpChannel().flush(result); } } - - @Override - protected void sendTrailers(HttpExchange exchange, Callback callback) - { - callback.succeeded(); - } } diff --git a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/TrailersTest.java b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/TrailersTest.java index 5b81e21bf20..348bb41d02b 100644 --- a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/TrailersTest.java +++ b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/TrailersTest.java @@ -18,16 +18,6 @@ package org.eclipse.jetty.http2.client; -import static org.hamcrest.Matchers.is; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; - import java.io.IOException; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; @@ -35,7 +25,6 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; - import javax.servlet.ServletInputStream; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -58,10 +47,14 @@ import org.eclipse.jetty.server.Response; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.FuturePromise; import org.eclipse.jetty.util.Promise; - import org.eclipse.jetty.util.StringUtil; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; public class TrailersTest extends AbstractTest { @@ -289,7 +282,7 @@ public class TrailersTest extends AbstractTest assertTrue(latch.await(5, TimeUnit.SECONDS)); - assertTrue( frames.size()==3, frames.toString()); + assertEquals(3, frames.size(), frames.toString()); HeadersFrame headers = (HeadersFrame)frames.get(0); DataFrame data = (DataFrame)frames.get(1); @@ -298,7 +291,7 @@ public class TrailersTest extends AbstractTest assertFalse(headers.isEndStream()); assertFalse(data.isEndStream()); assertTrue(trailers.isEndStream()); - assertTrue(trailers.getMetaData().getFields().get(trailerName).equals(trailerValue)); + assertEquals(trailers.getMetaData().getFields().get(trailerName), trailerValue); } @Test @@ -358,6 +351,5 @@ public class TrailersTest extends AbstractTest assertTrue(serverLatch.await(5, TimeUnit.SECONDS)); assertTrue(clientLatch.await(5, TimeUnit.SECONDS)); - } } diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpSenderOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpSenderOverHTTP2.java index b37d77a8b94..e376a91eeac 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpSenderOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpSenderOverHTTP2.java @@ -56,42 +56,57 @@ public class HttpSenderOverHTTP2 extends HttpSender String path = relativize(request.getPath()); HttpURI uri = HttpURI.createHttpURI(request.getScheme(), request.getHost(), request.getPort(), path, null, request.getQuery(), null); MetaData.Request metaData = new MetaData.Request(request.getMethod(), uri, HttpVersion.HTTP_2, request.getHeaders()); - Supplier trailers = request.getTrailers(); - metaData.setTrailerSupplier(trailers); - HeadersFrame headersFrame = new HeadersFrame(metaData, null, trailers == null && !content.hasContent()); - HttpChannelOverHTTP2 channel = getHttpChannel(); - Promise promise = new Promise() - { - @Override - public void succeeded(Stream stream) - { - channel.setStream(stream); - ((IStream)stream).setAttachment(channel); - long idleTimeout = request.getIdleTimeout(); - if (idleTimeout >= 0) - stream.setIdleTimeout(idleTimeout); + Supplier trailerSupplier = request.getTrailers(); + metaData.setTrailerSupplier(trailerSupplier); - if (content.hasContent() && !expects100Continue(request)) + HeadersFrame headersFrame; + Promise promise; + if (content.hasContent()) + { + headersFrame = new HeadersFrame(metaData, null, false); + promise = new HeadersPromise(request, callback) + { + @Override + public void succeeded(Stream stream) { - boolean advanced = content.advance(); - boolean lastContent = trailers == null && content.isLast(); - if (advanced || lastContent) + super.succeeded(stream); + if (expects100Continue(request)) { - DataFrame dataFrame = new DataFrame(stream.getId(), content.getByteBuffer(), lastContent); - stream.data(dataFrame, callback); - return; + // Don't send the content yet. + callback.succeeded(); + } + else + { + boolean advanced = content.advance(); + boolean lastContent = content.isLast(); + if (advanced || lastContent) + sendContent(stream, content, trailerSupplier, callback); + else + callback.succeeded(); } } - callback.succeeded(); - } - - @Override - public void failed(Throwable failure) + }; + } + else + { + HttpFields trailers = trailerSupplier == null ? null : trailerSupplier.get(); + boolean endStream = trailers == null || trailers.size() == 0; + headersFrame = new HeadersFrame(metaData, null, endStream); + promise = new HeadersPromise(request, callback) { - callback.failed(failure); - } - }; + @Override + public void succeeded(Stream stream) + { + super.succeeded(stream); + if (endStream) + callback.succeeded(); + else + sendTrailers(stream, trailers, callback); + } + }; + } // TODO optimize the send of HEADERS and DATA frames. + HttpChannelOverHTTP2 channel = getHttpChannel(); channel.getSession().newStream(headersFrame, promise, channel.getStreamListener()); } @@ -123,19 +138,59 @@ public class HttpSenderOverHTTP2 extends HttpSender else { Stream stream = getHttpChannel().getStream(); - Supplier trailers = exchange.getRequest().getTrailers(); - DataFrame frame = new DataFrame(stream.getId(), content.getByteBuffer(), trailers == null && content.isLast()); - stream.data(frame, callback); + Supplier trailerSupplier = exchange.getRequest().getTrailers(); + sendContent(stream, content, trailerSupplier, callback); } } - @Override - protected void sendTrailers(HttpExchange exchange, Callback callback) + private void sendContent(Stream stream, HttpContent content, Supplier trailerSupplier, Callback callback) { - Supplier trailers = exchange.getRequest().getTrailers(); - MetaData metaData = new MetaData(HttpVersion.HTTP_2, trailers.get()); - Stream stream = getHttpChannel().getStream(); + boolean lastContent = content.isLast(); + HttpFields trailers = null; + boolean endStream = false; + if (lastContent) + { + trailers = trailerSupplier == null ? null : trailerSupplier.get(); + endStream = trailers == null || trailers.size() == 0; + } + DataFrame dataFrame = new DataFrame(stream.getId(), content.getByteBuffer(), endStream); + HttpFields fTrailers = trailers; + stream.data(dataFrame, endStream || !lastContent ? callback : Callback.from(() -> sendTrailers(stream, fTrailers, callback), callback::failed)); + } + + private void sendTrailers(Stream stream, HttpFields trailers, Callback callback) + { + MetaData metaData = new MetaData(HttpVersion.HTTP_2, trailers); HeadersFrame trailersFrame = new HeadersFrame(stream.getId(), metaData, null, true); stream.headers(trailersFrame, callback); } + + private class HeadersPromise implements Promise + { + private final HttpRequest request; + private final Callback callback; + + private HeadersPromise(HttpRequest request, Callback callback) + { + this.request = request; + this.callback = callback; + } + + @Override + public void succeeded(Stream stream) + { + HttpChannelOverHTTP2 channel = getHttpChannel(); + channel.setStream(stream); + ((IStream)stream).setAttachment(channel); + long idleTimeout = request.getIdleTimeout(); + if (idleTimeout >= 0) + stream.setIdleTimeout(idleTimeout); + } + + @Override + public void failed(Throwable x) + { + callback.failed(x); + } + } } diff --git a/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/RequestTrailersTest.java b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/RequestTrailersTest.java new file mode 100644 index 00000000000..e6d321632d3 --- /dev/null +++ b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/RequestTrailersTest.java @@ -0,0 +1,142 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.http2.client.http; + +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import org.eclipse.jetty.client.HttpRequest; +import org.eclipse.jetty.client.api.ContentResponse; +import org.eclipse.jetty.client.util.DeferredContentProvider; +import org.eclipse.jetty.client.util.StringContentProvider; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.HttpVersion; +import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.http2.api.Stream; +import org.eclipse.jetty.http2.api.server.ServerSessionListener; +import org.eclipse.jetty.http2.frames.DataFrame; +import org.eclipse.jetty.http2.frames.HeadersFrame; +import org.eclipse.jetty.util.Callback; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class RequestTrailersTest extends AbstractTest +{ + @Test + public void testEmptyTrailersWithoutContent() throws Exception + { + testEmptyTrailers(null); + } + + @Test + public void testEmptyTrailersWithEagerContent() throws Exception + { + testEmptyTrailers("eager_content"); + } + + private void testEmptyTrailers(String content) throws Exception + { + CountDownLatch trailersLatch = new CountDownLatch(1); + start(new ServerSessionListener.Adapter() + { + @Override + public Stream.Listener onNewStream(Stream stream, HeadersFrame frame) + { + MetaData.Response response = new MetaData.Response(HttpVersion.HTTP_2, HttpStatus.OK_200, new HttpFields()); + HeadersFrame responseFrame = new HeadersFrame(stream.getId(), response, null, true); + stream.headers(responseFrame, Callback.NOOP); + return new Stream.Listener.Adapter() + { + @Override + public void onHeaders(Stream stream, HeadersFrame frame) + { + trailersLatch.countDown(); + } + }; + } + }); + + HttpRequest request = (HttpRequest)client.newRequest("localhost", connector.getLocalPort()); + HttpFields trailers = new HttpFields(); + request.trailers(() -> trailers); + if (content != null) + request.content(new StringContentProvider(content)); + + ContentResponse response = request.send(); + assertEquals(HttpStatus.OK_200, response.getStatus()); + + // The client must not send the trailers. + assertFalse(trailersLatch.await(1, TimeUnit.SECONDS)); + } + + @Test + public void testEmptyTrailersWithDeferredContent() throws Exception + { + start(new ServerSessionListener.Adapter() + { + @Override + public Stream.Listener onNewStream(Stream stream, HeadersFrame frame) + { + return new Stream.Listener.Adapter() + { + @Override + public void onData(Stream stream, DataFrame dataFrame, Callback callback) + { + callback.succeeded(); + // We should not receive an empty HEADERS frame for the + // trailers, but instead a DATA frame with endStream=true. + if (dataFrame.isEndStream()) + { + MetaData.Response response = new MetaData.Response(HttpVersion.HTTP_2, HttpStatus.OK_200, new HttpFields()); + HeadersFrame responseFrame = new HeadersFrame(stream.getId(), response, null, true); + stream.headers(responseFrame, Callback.NOOP); + } + } + }; + } + }); + + HttpRequest request = (HttpRequest)client.newRequest("localhost", connector.getLocalPort()); + HttpFields trailers = new HttpFields(); + request.trailers(() -> trailers); + DeferredContentProvider content = new DeferredContentProvider(); + request.content(content); + + CountDownLatch latch = new CountDownLatch(1); + request.send(result -> + { + assertTrue(result.isSucceeded()); + assertEquals(HttpStatus.OK_200, result.getResponse().getStatus()); + latch.countDown(); + }); + + // Send deferred content after a while. + Thread.sleep(1000); + content.offer(ByteBuffer.wrap("deferred_content".getBytes(StandardCharsets.UTF_8))); + content.close(); + + assertTrue(latch.await(5, TimeUnit.SECONDS)); + } +} From 877815e1959963f051a1c425405b0a4360f8a9e7 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 12 Jun 2019 09:56:41 -0500 Subject: [PATCH 05/22] Issue #3708 - Adding new methods and converting codebase to use them + StringUtil.replace(String, char, char) + StringUtil.strip(String, String) + URIUtil.encodeSpecific(String, String) + URIUtil.decodeSpecific(String, String) + TypeUtil.toClassReference(Class) + TypeUtil.toClassReference(String) Signed-off-by: Joakim Erdfelt --- .../asyncrest/AbstractRestServlet.java | 22 +++- .../eclipse/jetty/embedded/Http2Server.java | 3 +- .../jetty/annotations/AnnotationParser.java | 18 +-- .../annotations/TestAnnotationParser.java | 28 ++-- .../fcgi/client/http/HttpSenderOverFCGI.java | 3 +- .../org/eclipse/jetty/http/QuotedCSVTest.java | 12 +- .../jetty/http2/client/ProxyProtocolTest.java | 3 +- .../eclipse/jetty/io/ssl/SslConnection.java | 6 +- .../org/eclipse/jetty/jmx/MBeanContainer.java | 12 +- .../jetty/maven/plugin/JettyRunMojo.java | 3 +- .../maven/plugin/JettyWebAppContext.java | 6 +- .../maven/plugin/SelectiveJarResource.java | 3 +- .../nosql/mongodb/MongoSessionDataStore.java | 24 ++-- .../jetty/nosql/mongodb/MongoUtils.java | 12 +- .../osgi/annotations/AnnotationParser.java | 3 +- .../jetty/osgi/boot/BundleWebAppProvider.java | 2 +- .../webapp/OSGiWebappClassLoader.java | 6 +- .../jetty/plus/jndi/NamingEntryUtil.java | 6 +- .../jetty/proxy/AbstractProxyServlet.java | 4 +- .../eclipse/jetty/proxy/ProxyServletTest.java | 3 +- .../jetty/security/PropertyUserStore.java | 2 +- .../org/eclipse/jetty/server/Response.java | 4 +- .../jetty/server/session/SessionContext.java | 4 +- .../eclipse/jetty/server/AsyncStressTest.java | 8 +- .../eclipse/jetty/servlet/ServletHolder.java | 6 +- .../jetty/servlet/ResponseHeadersTest.java | 3 +- .../java/org/eclipse/jetty/servlets/CGI.java | 3 +- .../jetty/servlets/CrossOriginFilterTest.java | 5 +- .../org/eclipse/jetty/util/StringUtil.java | 73 +++++++++-- .../java/org/eclipse/jetty/util/TypeUtil.java | 42 +++++- .../java/org/eclipse/jetty/util/URIUtil.java | 115 ++++++++++++++++ .../jetty/util/component/Dumpable.java | 10 +- .../java/org/eclipse/jetty/util/log/Log.java | 3 +- .../jetty/util/resource/JarFileResource.java | 5 +- .../jetty/util/resource/JarResource.java | 3 +- .../eclipse/jetty/util/PathWatcherTest.java | 20 +-- .../org/eclipse/jetty/util/URIUtilTest.java | 123 +++++++++++++++++- .../jetty/webapp/ClasspathPattern.java | 3 +- .../jetty/webapp/WebAppClassLoader.java | 13 +- .../jetty/webapp/WebInfConfiguration.java | 18 +-- .../websocket/jsr356/server/WSServer.java | 11 +- .../jetty/websocket/server/WSServer.java | 3 +- .../test/support/TestableJettyServer.java | 54 ++++---- .../com/acme/test/ClassLoaderServlet.java | 1 - 44 files changed, 512 insertions(+), 199 deletions(-) diff --git a/examples/async-rest/async-rest-jar/src/main/java/org/eclipse/jetty/example/asyncrest/AbstractRestServlet.java b/examples/async-rest/async-rest-jar/src/main/java/org/eclipse/jetty/example/asyncrest/AbstractRestServlet.java index a3373fee901..b4779b8d8a8 100644 --- a/examples/async-rest/async-rest-jar/src/main/java/org/eclipse/jetty/example/asyncrest/AbstractRestServlet.java +++ b/examples/async-rest/async-rest-jar/src/main/java/org/eclipse/jetty/example/asyncrest/AbstractRestServlet.java @@ -24,7 +24,6 @@ import java.math.RoundingMode; import java.net.URLEncoder; import java.util.Map; import java.util.Queue; - import javax.servlet.ServletConfig; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; @@ -67,12 +66,25 @@ public class AbstractRestServlet extends HttpServlet _appid = servletConfig.getInitParameter(APPID_PARAM); } - - public static String sanitize(String s) + // TODO: consider using StringUtil.sanitizeFileSystemName instead of this? + // might introduce jetty-util dependency though + public static String sanitize(String str) { - if (s==null) + if (str == null) return null; - return s.replace("<","?").replace("&","?").replace("\n","?"); + + char[] chars = str.toCharArray(); + int len = chars.length; + for (int i = 0; i < len; i++) + { + char c = chars[i]; + if ((c <= 0x1F) || // control characters + (c == '<') || (c == '&')) + { + chars[i] = '?'; + } + } + return String.valueOf(chars); } protected String restURL(String item) diff --git a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/Http2Server.java b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/Http2Server.java index 8f219520966..3f50d7ee953 100644 --- a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/Http2Server.java +++ b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/Http2Server.java @@ -24,7 +24,6 @@ import java.io.IOException; import java.lang.management.ManagementFactory; import java.util.Date; import java.util.EnumSet; - import javax.servlet.DispatcherType; import javax.servlet.Filter; import javax.servlet.FilterChain; @@ -158,7 +157,7 @@ public class Http2Server public void destroy() { } - }; + } static Servlet servlet = new HttpServlet() { diff --git a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java index 0e7597a1de5..5b9da0b87a3 100644 --- a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java +++ b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java @@ -38,6 +38,8 @@ import org.eclipse.jetty.util.Loader; import org.eclipse.jetty.util.ManifestUtils; import org.eclipse.jetty.util.MultiException; import org.eclipse.jetty.util.MultiReleaseJarFile; +import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.resource.Resource; @@ -151,7 +153,7 @@ public class AnnotationParser if (name.endsWith(".class")) name = name.substring(0, name.length()-".class".length()); - return name.replace('/', '.'); + return StringUtil.replace(name,'/', '.'); } /** @@ -600,7 +602,7 @@ public class AnnotationParser return; String tmp = className; - className = className.replace('.', '/')+".class"; + className = TypeUtil.toClassReference(className); URL resource = Loader.getResource(className); if (resource!= null) { @@ -626,7 +628,7 @@ public class AnnotationParser Class cz = clazz; while (cz != Object.class) { - String nameAsResource = cz.getName().replace('.', '/')+".class"; + String nameAsResource = TypeUtil.toClassReference(cz); URL resource = Loader.getResource(nameAsResource); if (resource!= null) { @@ -675,8 +677,7 @@ public class AnnotationParser { try { - String name = s; - s = s.replace('.', '/')+".class"; + String name = TypeUtil.toClassReference(s); URL resource = Loader.getResource(s); if (resource!= null) { @@ -727,8 +728,9 @@ public class AnnotationParser { Path classpath = rootFile.toPath().relativize(file.toPath()); String str = classpath.toString(); - str = str.substring(0, str.lastIndexOf(".class")).replace('/', '.').replace('\\', '.'); - + str = str.substring(0, str.lastIndexOf(".class")); + str = StringUtil.replace(str, File.separatorChar, '.'); + try { if (LOG.isDebugEnabled()) @@ -910,7 +912,7 @@ public class AnnotationParser //check file is a valid class file name if (isValidClassFileName(name) && isValidClassFilePath(name)) { - String shortName = name.replace('/', '.').substring(0,name.length()-6); + String shortName = StringUtil.replace(name, '/', '.').substring(0, name.length() - 6); addParsedClass(shortName, Resource.newResource("jar:"+jar.getURI()+"!/"+entry.getNameInJar())); if (LOG.isDebugEnabled()) LOG.debug("Scanning class from jar {}!/{}", jar, entry); diff --git a/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationParser.java b/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationParser.java index 0d7cf14ddb8..05e54e8823c 100644 --- a/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationParser.java +++ b/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationParser.java @@ -19,11 +19,12 @@ package org.eclipse.jetty.annotations; import java.io.File; -import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.net.URL; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; @@ -42,6 +43,7 @@ import org.eclipse.jetty.toolchain.test.IO; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; +import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.resource.PathResource; import org.eclipse.jetty.util.resource.Resource; import org.junit.jupiter.api.Test; @@ -298,25 +300,17 @@ public class TestAnnotationParser private void copyClass(Class clazz, File basedir) throws IOException { - String classname = clazz.getName().replace('.',File.separatorChar) + ".class"; - URL url = this.getClass().getResource('/'+classname); - assertThat("URL for: " + classname,url,notNullValue()); + String classRef = TypeUtil.toClassReference(clazz); + URL url = this.getClass().getResource('/' + classRef); + assertThat("URL for: " + classRef, url, notNullValue()); - String classpath = classname.substring(0,classname.lastIndexOf(File.separatorChar)); - FS.ensureDirExists(new File(basedir,classpath)); + Path outputFile = basedir.toPath().resolve(classRef); + FS.ensureDirExists(outputFile.getParent()); - InputStream in = null; - OutputStream out = null; - try + try (InputStream in = url.openStream(); + OutputStream out = Files.newOutputStream(outputFile)) { - in = url.openStream(); - out = new FileOutputStream(new File(basedir,classname)); - IO.copy(in,out); - } - finally - { - IO.close(out); - IO.close(in); + IO.copy(in, out); } } } diff --git a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpSenderOverFCGI.java b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpSenderOverFCGI.java index 4f836fddfe2..2671b025c09 100644 --- a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpSenderOverFCGI.java +++ b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpSenderOverFCGI.java @@ -35,6 +35,7 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.Jetty; +import org.eclipse.jetty.util.StringUtil; public class HttpSenderOverFCGI extends HttpSender { @@ -88,7 +89,7 @@ public class HttpSenderOverFCGI extends HttpSender for (HttpField field : headers) { String name = field.getName(); - String fcgiName = "HTTP_" + name.replace('-', '_').toUpperCase(Locale.ENGLISH); + String fcgiName = "HTTP_" + StringUtil.replace(name, '-', '_').toUpperCase(Locale.ENGLISH); fcgiHeaders.add(fcgiName, field.getValue()); } diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/QuotedCSVTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/QuotedCSVTest.java index 954fb562dfc..9a719a08583 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/QuotedCSVTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/QuotedCSVTest.java @@ -18,13 +18,13 @@ package org.eclipse.jetty.http; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.MatcherAssert.assertThat; - +import org.eclipse.jetty.util.StringUtil; import org.hamcrest.Matchers; - import org.junit.jupiter.api.Test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + public class QuotedCSVTest { @Test @@ -111,13 +111,13 @@ public class QuotedCSVTest { if (buffer.toString().contains("DELETE")) { - String s = buffer.toString().replace("DELETE",""); + String s = StringUtil.strip(buffer.toString(), "DELETE"); buffer.setLength(0); buffer.append(s); } if (buffer.toString().contains("APPEND")) { - String s = buffer.toString().replace("APPEND","Append")+"!"; + String s = StringUtil.replace(buffer.toString(), "APPEND", "Append") + "!"; buffer.setLength(0); buffer.append(s); } diff --git a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/ProxyProtocolTest.java b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/ProxyProtocolTest.java index c06a555aa21..d63ab045dd5 100644 --- a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/ProxyProtocolTest.java +++ b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/ProxyProtocolTest.java @@ -48,6 +48,7 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.util.FuturePromise; import org.eclipse.jetty.util.Promise; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.TypeUtil; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; @@ -167,7 +168,7 @@ public class ProxyProtocolTest // String is: "MAGIC VER|CMD FAM|PROT LEN SRC_ADDR DST_ADDR SRC_PORT DST_PORT PP2_TYPE_SSL LEN CLIENT VERIFY PP2_SUBTYPE_SSL_VERSION LEN 1.2" String request1 = "0D0A0D0A000D0A515549540A 21 11 001A 0A000004 0A000005 8420 22B8 20 000B 01 00000000 21 0003 312E32"; - request1 = request1.replace(" ", ""); + request1 = StringUtil.strip(request1, " "); SocketChannel channel = SocketChannel.open(); channel.connect(new InetSocketAddress("localhost", connector.getLocalPort())); channel.write(ByteBuffer.wrap(TypeUtil.fromHexString(request1))); diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java index a2c1fdc732e..94485855edb 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java @@ -25,7 +25,6 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicReference; - import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLEngineResult; import javax.net.ssl.SSLEngineResult.HandshakeStatus; @@ -41,6 +40,7 @@ import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.io.WriteFlusher; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.thread.Invocable; @@ -584,7 +584,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr if (LOG.isDebugEnabled()) LOG.debug("unwrap net_filled={} {} encryptedBuffer={} unwrapBuffer={} appBuffer={}", net_filled, - unwrapResult.toString().replace('\n', ' '), + StringUtil.replace(unwrapResult.toString(), '\n', ' '), BufferUtil.toSummaryString(_encryptedInput), BufferUtil.toDetailString(app_in), BufferUtil.toDetailString(buffer)); @@ -895,7 +895,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr } if (LOG.isDebugEnabled()) LOG.debug("wrap {} {} ioDone={}/{}", - wrapResult.toString().replace('\n', ' '), + StringUtil.replace(wrapResult.toString(), '\n', ' '), BufferUtil.toSummaryString(_encryptedOutput), _sslEngine.isInboundDone(), _sslEngine.isOutboundDone()); diff --git a/jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MBeanContainer.java b/jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MBeanContainer.java index 42955478c5c..8b75436f0c1 100644 --- a/jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MBeanContainer.java +++ b/jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MBeanContainer.java @@ -30,7 +30,6 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; - import javax.management.InstanceNotFoundException; import javax.management.MBeanInfo; import javax.management.MBeanRegistrationException; @@ -39,6 +38,7 @@ import javax.management.ObjectName; import javax.management.modelmbean.ModelMBean; import org.eclipse.jetty.util.Loader; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.component.Container; @@ -394,15 +394,7 @@ public class MBeanContainer implements Container.InheritedListener, Dumpable, De */ public String makeName(String basis) { - if (basis == null) - return null; - return basis - .replace(':', '_') - .replace('*', '_') - .replace('?', '_') - .replace('=', '_') - .replace(',', '_') - .replace(' ', '_'); + return StringUtil.sanitizeFileSystemName(basis); } @Override diff --git a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunMojo.java b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunMojo.java index 56b50cf11cf..c90dd62e3f6 100644 --- a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunMojo.java +++ b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyRunMojo.java @@ -43,6 +43,7 @@ import org.apache.maven.project.MavenProject; import org.eclipse.jetty.maven.plugin.utils.MavenProjectHelper; import org.eclipse.jetty.util.PathWatcher; import org.eclipse.jetty.util.PathWatcher.PathWatchEvent; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceCollection; import org.eclipse.jetty.webapp.WebAppContext; @@ -661,7 +662,7 @@ public class JettyRunMojo extends AbstractJettyMojo int i = name.lastIndexOf('/'); if (i>0) name = name.substring(i+1,name.length()); - name = name.replace('.', '_'); + name = StringUtil.replace(name, '.', '_'); //name = name+(++COUNTER); //add some digits to ensure uniqueness File overlaysDir = new File (project.getBuild().getDirectory(), "jetty_overlays"); File dir = new File(overlaysDir, name); diff --git a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyWebAppContext.java b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyWebAppContext.java index 98abd11fa47..0c3d250ae00 100644 --- a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyWebAppContext.java +++ b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyWebAppContext.java @@ -514,7 +514,7 @@ public class JettyWebAppContext extends WebAppContext int i=0; while (res == null && (i < _webInfClasses.size())) { - String newPath = uri.replace(WEB_INF_CLASSES_PREFIX, _webInfClasses.get(i).getPath()); + String newPath = StringUtil.replace(uri, WEB_INF_CLASSES_PREFIX, _webInfClasses.get(i).getPath()); res = Resource.newResource(newPath); if (!res.exists()) { @@ -529,7 +529,7 @@ public class JettyWebAppContext extends WebAppContext { // Return the real jar file for all accesses to // /WEB-INF/lib/*.jar - String jarName = uri.replace(WEB_INF_LIB_PREFIX, ""); + String jarName = StringUtil.strip(uri, WEB_INF_LIB_PREFIX); if (jarName.startsWith("/") || jarName.startsWith("\\")) jarName = jarName.substring(1); if (jarName.length()==0) @@ -581,7 +581,7 @@ public class JettyWebAppContext extends WebAppContext while (i < _webInfClasses.size()) { - String newPath = path.replace(WEB_INF_CLASSES_PREFIX, _webInfClasses.get(i).getPath()); + String newPath = StringUtil.replace(path, WEB_INF_CLASSES_PREFIX, _webInfClasses.get(i).getPath()); allPaths.addAll(super.getResourcePaths(newPath)); i++; } diff --git a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/SelectiveJarResource.java b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/SelectiveJarResource.java index 91a9d4b9623..2830d5a1469 100644 --- a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/SelectiveJarResource.java +++ b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/SelectiveJarResource.java @@ -33,6 +33,7 @@ import java.util.jar.Manifest; import org.codehaus.plexus.util.SelectorUtils; import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -146,7 +147,7 @@ public class SelectiveJarResource extends JarResource String entryName = entry.getName(); LOG.debug("Looking at "+entryName); - String dotCheck = entryName.replace('\\', '/'); + String dotCheck = StringUtil.replace(entryName, '\\', '/'); dotCheck = URIUtil.canonicalPath(dotCheck); if (dotCheck == null) { diff --git a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStore.java b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStore.java index ab0b3fa6579..81630dff3d1 100644 --- a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStore.java +++ b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStore.java @@ -27,16 +27,6 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; -import org.eclipse.jetty.nosql.NoSqlSessionDataStore; -import org.eclipse.jetty.server.session.SessionContext; -import org.eclipse.jetty.server.session.SessionData; -import org.eclipse.jetty.server.session.UnreadableSessionDataException; -import org.eclipse.jetty.util.ClassLoadingObjectInputStream; -import org.eclipse.jetty.util.annotation.ManagedAttribute; -import org.eclipse.jetty.util.annotation.ManagedObject; -import org.eclipse.jetty.util.log.Log; -import org.eclipse.jetty.util.log.Logger; - import com.mongodb.BasicDBList; import com.mongodb.BasicDBObject; import com.mongodb.BasicDBObjectBuilder; @@ -46,6 +36,16 @@ import com.mongodb.DBObject; import com.mongodb.MongoException; import com.mongodb.WriteConcern; import com.mongodb.WriteResult; +import org.eclipse.jetty.nosql.NoSqlSessionDataStore; +import org.eclipse.jetty.server.session.SessionContext; +import org.eclipse.jetty.server.session.SessionData; +import org.eclipse.jetty.server.session.UnreadableSessionDataException; +import org.eclipse.jetty.util.ClassLoadingObjectInputStream; +import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.annotation.ManagedAttribute; +import org.eclipse.jetty.util.annotation.ManagedObject; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; /** * MongoSessionDataStore @@ -574,8 +574,8 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore { if (vhost == null) return ""; - - return vhost.replace('.', '_'); + + return StringUtil.replace(vhost, '.', '_'); } diff --git a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoUtils.java b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoUtils.java index a66ca44cc54..afe3c76f984 100644 --- a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoUtils.java +++ b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoUtils.java @@ -30,7 +30,7 @@ import java.util.Map; import com.mongodb.BasicDBObject; import com.mongodb.DBObject; import org.eclipse.jetty.util.ClassLoadingObjectInputStream; -import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.URIUtil; /** * MongoUtils @@ -72,18 +72,12 @@ public class MongoUtils public static String decodeName(String name) { - String cleaned = name; - cleaned = StringUtil.replace(cleaned, "%2E", "."); - cleaned = StringUtil.replace(cleaned, "%25", "%"); - return cleaned; + return URIUtil.decodeSpecific(name, ".%"); } public static String encodeName(String name) { - String cleaned = name; - cleaned = StringUtil.replace(cleaned, "%", "%25"); - cleaned = StringUtil.replace(cleaned, ".", "%2E"); - return cleaned; + return URIUtil.encodeSpecific(name, ".%"); } public static Object encodeName(Object value) throws IOException diff --git a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/annotations/AnnotationParser.java b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/annotations/AnnotationParser.java index 5b7be2e2510..d520cb8316e 100644 --- a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/annotations/AnnotationParser.java +++ b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/annotations/AnnotationParser.java @@ -30,6 +30,7 @@ import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; import org.eclipse.jetty.osgi.boot.utils.BundleFileLocatorHelperFactory; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.resource.Resource; import org.objectweb.asm.Opcodes; import org.osgi.framework.Bundle; @@ -209,7 +210,7 @@ public class AnnotationParser extends org.eclipse.jetty.annotations.AnnotationPa continue; } //transform into a classname to pass to the resolver - String shortName = name.replace('/', '.').substring(0,name.length()-6); + String shortName = StringUtil.replace(name, '/', '.').substring(0, name.length() - 6); addParsedClass(shortName, getResource(bundle)); try (InputStream classInputStream = classUrl.openStream()) diff --git a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/BundleWebAppProvider.java b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/BundleWebAppProvider.java index a3768471d2b..fb3fdea4c94 100644 --- a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/BundleWebAppProvider.java +++ b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/BundleWebAppProvider.java @@ -280,7 +280,7 @@ public class BundleWebAppProvider extends AbstractWebAppProvider implements Bund // the location will often reflect the version. // maybe this is relevant when the file is a war) String location = bundle.getLocation(); - String toks[] = location.replace('\\', '/').split("/"); + String toks[] = StringUtil.replace(location, '\\', '/').split("/"); contextPath = toks[toks.length - 1]; // remove .jar, .war etc: int lastDot = contextPath.lastIndexOf('.'); diff --git a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/webapp/OSGiWebappClassLoader.java b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/webapp/OSGiWebappClassLoader.java index 5e4212ff8c0..9afb705ad34 100644 --- a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/webapp/OSGiWebappClassLoader.java +++ b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/internal/webapp/OSGiWebappClassLoader.java @@ -30,10 +30,10 @@ import java.util.List; import java.util.Set; import java.util.StringTokenizer; import java.util.jar.JarFile; - import javax.servlet.http.HttpServlet; import org.eclipse.jetty.osgi.boot.utils.BundleClassLoaderHelperFactory; +import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.resource.Resource; @@ -61,12 +61,12 @@ public class OSGiWebappClassLoader extends WebAppClassLoader implements BundleRe public static void addClassThatIdentifiesAJarThatMustBeRejected(Class zclass) { - JAR_WITH_SUCH_CLASS_MUST_BE_EXCLUDED.add(zclass.getName().replace('.', '/') + ".class"); + JAR_WITH_SUCH_CLASS_MUST_BE_EXCLUDED.add(TypeUtil.toClassReference(zclass.getName())); } public static void addClassThatIdentifiesAJarThatMustBeRejected(String zclassName) { - JAR_WITH_SUCH_CLASS_MUST_BE_EXCLUDED.add(zclassName.replace('.', '/') + ".class"); + JAR_WITH_SUCH_CLASS_MUST_BE_EXCLUDED.add(TypeUtil.toClassReference(zclassName)); } static diff --git a/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingEntryUtil.java b/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingEntryUtil.java index f753bcfa218..956488be3a0 100644 --- a/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingEntryUtil.java +++ b/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingEntryUtil.java @@ -30,6 +30,7 @@ import javax.naming.NameParser; import javax.naming.NamingEnumeration; import javax.naming.NamingException; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -238,8 +239,9 @@ public class NamingEntryUtil if (scope==null) return ""; - String str = scope.getClass().getName()+"@"+Long.toHexString(scope.hashCode()); - str=str.replace('/', '_').replace(' ', '_'); + String str = scope.getClass().getName() + "@" + Long.toHexString(scope.hashCode()); + // TODO: Is this sanitize step still needed? (this canonicalizeScope method has been reduced in functionality a lot over the years) + str = StringUtil.sanitizeFileSystemName(str); return str; } } diff --git a/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/AbstractProxyServlet.java b/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/AbstractProxyServlet.java index 0badfed54c7..cadc3cf5b62 100644 --- a/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/AbstractProxyServlet.java +++ b/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/AbstractProxyServlet.java @@ -29,7 +29,6 @@ import java.util.Locale; import java.util.Set; import java.util.concurrent.Executor; import java.util.concurrent.TimeoutException; - import javax.servlet.AsyncContext; import javax.servlet.ServletConfig; import javax.servlet.ServletContext; @@ -51,6 +50,7 @@ import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.util.HttpCookieStore; import org.eclipse.jetty.util.ProcessorUtils; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.thread.QueuedThreadPool; @@ -207,7 +207,7 @@ public abstract class AbstractProxyServlet extends HttpServlet protected Logger createLogger() { String servletName = getServletConfig().getServletName(); - servletName = servletName.replace('-', '.'); + servletName = StringUtil.replace(servletName, '-', '.'); if ((getClass().getPackage() != null) && !servletName.startsWith(getClass().getPackage().getName())) { servletName = getClass().getName() + "." + servletName; diff --git a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyServletTest.java b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyServletTest.java index 71afb0e0904..762c0ab33bb 100644 --- a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyServletTest.java +++ b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyServletTest.java @@ -85,6 +85,7 @@ import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; @@ -669,7 +670,7 @@ public class ProxyServletTest // Make the request to the proxy, it should transparently forward to the server ContentResponse response = client.newRequest("localhost", proxyConnector.getLocalPort()) - .path((prefix + target).replace("//", "/")) + .path(StringUtil.replace((prefix + target), "//", "/")) .timeout(5, TimeUnit.SECONDS) .send(); assertEquals(200, response.getStatus()); diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/PropertyUserStore.java b/jetty-security/src/main/java/org/eclipse/jetty/security/PropertyUserStore.java index 9d0ea1004b4..b7e9a63269e 100644 --- a/jetty-security/src/main/java/org/eclipse/jetty/security/PropertyUserStore.java +++ b/jetty-security/src/main/java/org/eclipse/jetty/security/PropertyUserStore.java @@ -141,7 +141,7 @@ public class PropertyUserStore extends UserStore implements PathWatcher.Listener if (colon < 0 || bang_slash < 0 || colon > bang_slash) throw new IllegalArgumentException("Not resolved JarFile resource: " + uri); - String entry_path = StringUtil.sanitizeFileSystemPath(uri.substring(colon + 2)); + String entry_path = StringUtil.sanitizeFileSystemName(uri.substring(colon + 2)); Path tmpDirectory = Files.createTempDirectory("users_store"); tmpDirectory.toFile().deleteOnExit(); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java index 1809d5b1cea..e03c00711b9 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java @@ -235,7 +235,7 @@ public class Response implements HttpServletResponse if (i >= 0) { httpOnly = true; - comment = StringUtil.replace(comment.trim(), HTTP_ONLY_COMMENT, ""); + comment = StringUtil.strip(comment.trim(), HTTP_ONLY_COMMENT); if (comment.length() == 0) comment = null; } @@ -1144,7 +1144,7 @@ public class Response implements HttpServletResponse return; _locale = locale; - _fields.put(HttpHeader.CONTENT_LANGUAGE, locale.toString().replace('_', '-')); + _fields.put(HttpHeader.CONTENT_LANGUAGE, StringUtil.replace(locale.toString(), '_', '-')); if (_outputType != OutputType.NONE) return; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionContext.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionContext.java index 70e9ef8cdf3..97235ecac05 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionContext.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionContext.java @@ -21,6 +21,7 @@ package org.eclipse.jetty.server.session; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.handler.ContextHandler.Context; +import org.eclipse.jetty.util.StringUtil; /** * SessionContext @@ -138,7 +139,6 @@ public class SessionContext if (path==null) return ""; - return path.replace('/', '_').replace('.','_').replace('\\','_'); + return StringUtil.sanitizeFileSystemName(path); } - } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/AsyncStressTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/AsyncStressTest.java index 5982de96d0a..4b74de2bc02 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/AsyncStressTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/AsyncStressTest.java @@ -18,8 +18,6 @@ package org.eclipse.jetty.server; -import static org.junit.jupiter.api.Assertions.assertEquals; - import java.io.IOException; import java.io.InputStream; import java.net.InetAddress; @@ -29,7 +27,6 @@ import java.util.Random; import java.util.Timer; import java.util.TimerTask; import java.util.concurrent.TimeUnit; - import javax.servlet.AsyncContext; import javax.servlet.AsyncEvent; import javax.servlet.AsyncListener; @@ -40,6 +37,7 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.server.handler.HandlerWrapper; import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.thread.QueuedThreadPool; @@ -49,6 +47,8 @@ import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; + @Disabled @Tag("stress") public class AsyncStressTest @@ -123,7 +123,7 @@ public class AsyncStressTest int p=path[i][l]=_random.nextInt(__paths.length); int period = _random.nextInt(290)+10; - String uri=__paths[p][0].replace("",Integer.toString(period)); + String uri = StringUtil.replace(__paths[p][0], "", Integer.toString(period)); long start=TimeUnit.NANOSECONDS.toMillis(System.nanoTime()); String request = diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java index 8ed3d020e8a..1d0352c0307 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java @@ -966,7 +966,7 @@ public class ServletHolder extends Holder implements UserIdentity.Scope } catch (Exception e) { - String tmp = jsp.replace('.','_'); + String tmp = StringUtil.replace(jsp, '.', '_'); if (LOG.isDebugEnabled()) { LOG.warn("JspUtil.makeJavaIdentifier failed for jsp "+jsp +" using "+tmp+" instead"); @@ -1003,9 +1003,9 @@ public class ServletHolder extends Holder implements UserIdentity.Scope s = 1; //remove the element after last slash, which should be name of jsp - tmp = tmp.substring(s,i); + tmp = tmp.substring(s,i).trim(); - tmp = tmp.replace('/','.').trim(); + tmp = StringUtil.replace(tmp, '/', '.'); tmp = (".".equals(tmp)? "": tmp); if (LOG.isDebugEnabled()) { diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ResponseHeadersTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ResponseHeadersTest.java index eb801824bea..f9d1caa5612 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ResponseHeadersTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ResponseHeadersTest.java @@ -30,6 +30,7 @@ import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.util.StringUtil; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -143,7 +144,7 @@ public class ResponseHeadersTest assertThat("Response Code",response.getStatus(),is(200)); assertThat("Response Header Content-Type",response.get("Content-Type"),is("text/plain;charset=UTF-8")); - String expected = actualPathInfo.replace("%0A", " "); // replace OBS fold with space + String expected = StringUtil.replace(actualPathInfo, "%0A", " "); // replace OBS fold with space expected = URLDecoder.decode(expected, "utf-8"); // decode the rest expected = expected.trim(); // trim whitespace at start/end assertThat("Response Header X-example", response.get("X-Example"), is(expected)); diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/CGI.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/CGI.java index 63222907c08..39929a87a56 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/CGI.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/CGI.java @@ -29,7 +29,6 @@ import java.util.Enumeration; import java.util.HashMap; import java.util.Locale; import java.util.Map; - import javax.servlet.AsyncContext; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; @@ -324,7 +323,7 @@ public class CGI extends HttpServlet if (name.equalsIgnoreCase("Proxy")) continue; String value = req.getHeader(name); - env.set("HTTP_" + name.toUpperCase(Locale.ENGLISH).replace('-', '_'), value); + env.set("HTTP_" + StringUtil.replace(name.toUpperCase(Locale.ENGLISH), '-', '_'), value); } // these extra ones were from printenv on www.dev.nomura.co.uk diff --git a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/CrossOriginFilterTest.java b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/CrossOriginFilterTest.java index 5687334d40a..598c40beceb 100644 --- a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/CrossOriginFilterTest.java +++ b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/CrossOriginFilterTest.java @@ -34,6 +34,7 @@ import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.servlet.FilterHolder; import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.servlet.ServletTester; +import org.eclipse.jetty.util.StringUtil; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -94,7 +95,7 @@ public class CrossOriginFilterTest CountDownLatch latch = new CountDownLatch(1); tester.getContext().addServlet(new ServletHolder(new ResourceServlet(latch)), "/*"); - String otherOrigin = origin.replace("localhost", "127.0.0.1"); + String otherOrigin = StringUtil.replace(origin, "localhost", "127.0.0.1"); String request = "" + "GET / HTTP/1.1\r\n" + "Host: localhost\r\n" + @@ -284,7 +285,7 @@ public class CrossOriginFilterTest { FilterHolder filterHolder = new FilterHolder(new CrossOriginFilter()); String origin = "http://localhost"; - String otherOrigin = origin.replace("localhost", "127.0.0.1"); + String otherOrigin = StringUtil.replace(origin, "localhost", "127.0.0.1"); filterHolder.setInitParameter(CrossOriginFilter.ALLOWED_ORIGINS_PARAM, origin + "," + otherOrigin); tester.getContext().addFilter(filterHolder, "/*", EnumSet.of(DispatcherType.REQUEST)); diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java index 0bf5a431194..b2e148ea97a 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java @@ -142,7 +142,7 @@ public class StringUtil /** * Replace all characters from input string that are known to have - * special meaning in various filesytems. + * special meaning in various filesystems. * *

* This will replace all of the following characters @@ -151,14 +151,18 @@ public class StringUtil *

    *
  • Control Characters
    • *
  • Anything not 7-bit printable ASCII
    • - *
  • Special characters "{@code !|/.,:&><\?*"}"
    • + *
  • Special characters: pipe, redirect, combine, slash, equivalence, bang, glob, selection, etc...
    • + *
  • Space
    • *
* * @param str the raw input string - * @return the sanitized output string. + * @return the sanitized output string. or null if {@code str} is null. */ - public static String sanitizeFileSystemPath(String str) + public static String sanitizeFileSystemName(String str) { + if (str == null) + return null; + char[] chars = str.toCharArray(); int len = chars.length; for (int i = 0; i < len; i++) @@ -166,10 +170,18 @@ public class StringUtil char c = chars[i]; if ((c <= 0x1F) || // control characters (c >= 0x7F) || // over 7-bit printable ASCII - (c == '!') || (c == '|') || (c == '.') || - (c == ':') || (c == '>') || (c == '<') || - (c == '?') || (c == '/') || (c == '\\') || - (c == '*') || (c == '"') || (c == '&')) + // piping : special meaning on unix / osx / windows + (c == '|') || (c == '>') || (c == '<') || (c == '/') || (c == '&') || + // special characters on windows + (c == '\\') || (c == '.') || (c == ':') || + // special characters on osx + (c == '=') || (c == '"') || (c == ',') || + // glob / selection characters on most OS's + (c == '*') || (c == '?') || + // bang execution on unix / osx + (c == '!') || + // spaces are just generally difficult to work with + (c == ' ')) { chars[i] = '_'; } @@ -246,6 +258,41 @@ public class StringUtil return -1; } + /** + * Replace chars within string. + *

+ * Fast replacement for {@code java.lang.String#}{@link String#replace(char, char)} + *

+ * + * @param str the input string + * @param find the char to look for + * @param with the char to replace with + * @return the now replaced string + */ + public static String replace(String str, char find, char with) + { + if (str == null) + return null; + + if (find == with) + return str; + + int c = 0; + int idx = str.indexOf(find, c); + if (idx == -1) + { + return str; + } + char[] chars = str.toCharArray(); + int len = chars.length; + for (int i = idx; i < len; i++) + { + if (chars[i] == find) + chars[i] = with; + } + return String.valueOf(chars); + } + /** * Replace substrings within string. *

@@ -259,6 +306,9 @@ public class StringUtil */ public static String replace(String s, String sub, String with) { + if (s == null) + return null; + int c = 0; int i = s.indexOf(sub, c); if (i == -1) @@ -277,7 +327,7 @@ public class StringUtil { buf.append(s.substring(c)); } - return buf.toString(); + return buf.toString(); } /** @@ -1082,6 +1132,11 @@ public class StringUtil return out.toString(); } + public static String strip(String str, String find) + { + return StringUtil.replace(str, find, ""); + } + /** The String value of an Object *

This method calls {@link String#valueOf(Object)} unless the object is null, * in which case null is returned

diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java index cb675f97cf5..93c705b1ca3 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java @@ -206,6 +206,46 @@ public class TypeUtil return class2Name.get(type); } + /** + * Return the Classpath / Classloader reference for the + * provided class file. + * + *

+ * Convenience method for the code + *

+ * + * 
+     * String ref = myObject.getClass().getName().replace('.','/') + ".class";
+     *
+ * + * @param clazz the class to reference + * @return the classpath reference syntax for the class file + */ + public static String toClassReference(Class clazz) + { + return TypeUtil.toClassReference(clazz.getName()); + } + + /** + * Return the Classpath / Classloader reference for the + * provided class file. + * + *

+ * Convenience method for the code + *

+ * + * 
+     * String ref = myClassName.replace('.','/') + ".class";
+     *
+ * + * @param className the class to reference + * @return the classpath reference syntax for the class file + */ + public static String toClassReference(String className) + { + return StringUtil.replace(className, '.', '/').concat(".class"); + } + /* ------------------------------------------------------------ */ /** Convert String value to instance. * @param type The class of the instance, which may be a primitive TYPE field. @@ -560,7 +600,7 @@ public class TypeUtil } } - String resourceName = clazz.getName().replace('.', '/') + ".class"; + String resourceName = TypeUtil.toClassReference(clazz); ClassLoader loader = clazz.getClassLoader(); URL url = (loader == null ? ClassLoader.getSystemClassLoader() : loader).getResource(resourceName); if (url != null) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java index 17da05adf80..a0123e8c39b 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java @@ -281,6 +281,121 @@ public class URIUtil return StringUtil.replace(str, " ", "%20"); } + /** + * Encode a raw String and convert any specific characters to their URI encoded equivalent. + * + * @param str input raw string + * @param charsToEncode the list of raw characters that need to be encoded (if encountered) + * @return output with specified characters encoded. + */ + @SuppressWarnings("Duplicates") + public static String encodeSpecific(String str, String charsToEncode) + { + if ((str == null) || (str.length() == 0)) + return null; + + if ((charsToEncode == null) || (charsToEncode.length() == 0)) + return str; + + char[] find = charsToEncode.toCharArray(); + int len = str.length(); + StringBuilder ret = new StringBuilder((int)(len * 0.20d)); + for (int i = 0; i < len; i++) + { + char c = str.charAt(i); + boolean escaped = false; + for (char f : find) + { + if (c == f) + { + escaped = true; + ret.append('%'); + int d = 0xf & ((0xF0 & c) >> 4); + ret.append((char)((d > 9 ? ('A' - 10) : '0') + d)); + d = 0xf & c; + ret.append((char)((d > 9 ? ('A' - 10) : '0') + d)); + break; + } + } + if (!escaped) + { + ret.append(c); + } + } + return ret.toString(); + } + + /** + * Decode a raw String and convert any specific URI encoded sequences into characters. + * + * @param str input raw string + * @param charsToDecode the list of raw characters that need to be decoded (if encountered), leaving all other encoded sequences alone. + * @return output with specified characters decoded. + */ + @SuppressWarnings("Duplicates") + public static String decodeSpecific(String str, String charsToDecode) + { + if ((str == null) || (str.length() == 0)) + return null; + + if ((charsToDecode == null) || (charsToDecode.length() == 0)) + return str; + + int idx = str.indexOf('%'); + if (idx == -1) + { + // no hits + return str; + } + + char[] find = charsToDecode.toCharArray(); + int len = str.length(); + Utf8StringBuilder ret = new Utf8StringBuilder(len); + ret.append(str, 0, idx); + + for (int i = idx; i < len; i++) + { + char c = str.charAt(i); + switch (c) + { + case '%': + if ((i + 2) < len) + { + char u = str.charAt(i + 1); + char l = str.charAt(i + 2); + char result = (char)(0xff & (TypeUtil.convertHexDigit(u) * 16 + TypeUtil.convertHexDigit(l))); + boolean decoded = false; + for (char f : find) + { + if (f == result) + { + ret.append(result); + decoded = true; + break; + } + } + if (decoded) + { + i += 2; + } + else + { + ret.append(c); + } + } + else + { + throw new IllegalArgumentException("Bad URI % encoding"); + } + break; + default: + ret.append(c); + break; + } + } + return ret.toString(); + } + /* ------------------------------------------------------------ */ /** Encode a URI path. * @param path The path the encode diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Dumpable.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Dumpable.java index ee9fc9df110..1e973f3f5c6 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Dumpable.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Dumpable.java @@ -101,13 +101,15 @@ public interface Dumpable s = String.format("%s@%x{size=%d}",o.getClass().getName(),o.hashCode(),((Map)o).size()); else if (o instanceof Dumpable) { - s = StringUtil.replace(((Dumpable)o).dumpSelf(), "\r\n", "|") - .replace('\n', '|'); + s = ((Dumpable)o).dumpSelf(); + s = StringUtil.replace(s, "\r\n", "|"); + s = StringUtil.replace(s, '\n', '|'); } else { - s = StringUtil.replace(String.valueOf(o), "\r\n", "|") - .replace('\n', '|'); + s = String.valueOf(o); + s = StringUtil.replace(s, "\r\n", "|"); + s = StringUtil.replace(s, '\n', '|'); } if (o instanceof LifeCycle) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/log/Log.java b/jetty-util/src/main/java/org/eclipse/jetty/util/log/Log.java index 5a8e6c49dd1..71d6861216a 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/log/Log.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/log/Log.java @@ -33,6 +33,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import org.eclipse.jetty.util.Loader; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.Uptime; import org.eclipse.jetty.util.annotation.ManagedAttribute; @@ -102,7 +103,7 @@ public class Log // NOTE: cannot use jetty-util's StringUtil as that initializes logging itself. if (osName != null && osName.length() > 0) { - osName = osName.toLowerCase(Locale.ENGLISH).replace(' ','-'); + osName = StringUtil.replace(osName.toLowerCase(Locale.ENGLISH), ' ', '-'); loadProperties("jetty-logging-" + osName + ".properties",__props); } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarFileResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarFileResource.java index 406781fa8ca..d7861a37baf 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarFileResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarFileResource.java @@ -29,6 +29,7 @@ import java.util.List; import java.util.jar.JarEntry; import java.util.jar.JarFile; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -326,8 +327,8 @@ public class JarFileResource extends JarResource String dir=_urlString.substring(_urlString.lastIndexOf("!/")+2); while(e.hasMoreElements()) { - JarEntry entry = e.nextElement(); - String name=entry.getName().replace('\\','/'); + JarEntry entry = e.nextElement(); + String name = StringUtil.replace(entry.getName(), '\\', '/'); if(!name.startsWith(dir) || name.length()==dir.length()) { continue; diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarResource.java index 199141c57c7..099f15276e9 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarResource.java @@ -32,6 +32,7 @@ import java.util.jar.JarInputStream; import java.util.jar.Manifest; import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -212,7 +213,7 @@ public class JarResource extends URLResource continue; } - String dotCheck = entryName.replace('\\', '/'); + String dotCheck = StringUtil.replace(entryName, '\\', '/'); dotCheck = URIUtil.canonicalPath(dotCheck); if (dotCheck == null) { diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/PathWatcherTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/PathWatcherTest.java index 774103192d3..72eb0a5aecf 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/PathWatcherTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/PathWatcherTest.java @@ -18,15 +18,6 @@ package org.eclipse.jetty.util; -import static org.eclipse.jetty.util.PathWatcher.PathWatchEventType.ADDED; -import static org.eclipse.jetty.util.PathWatcher.PathWatchEventType.DELETED; -import static org.eclipse.jetty.util.PathWatcher.PathWatchEventType.MODIFIED; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.is; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.io.File; import java.io.FileOutputStream; import java.io.IOException; @@ -52,6 +43,15 @@ import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import static org.eclipse.jetty.util.PathWatcher.PathWatchEventType.ADDED; +import static org.eclipse.jetty.util.PathWatcher.PathWatchEventType.DELETED; +import static org.eclipse.jetty.util.PathWatcher.PathWatchEventType.MODIFIED; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertTrue; + @Disabled @ExtendWith(WorkDirExtension.class) public class PathWatcherTest @@ -111,7 +111,7 @@ public class PathWatcherTest synchronized (events) { Path relativePath = this.baseDir.relativize(event.getPath()); - String key = relativePath.toString().replace(File.separatorChar,'/'); + String key = StringUtil.replace(relativePath.toString(), File.separatorChar, '/'); List types = this.events.get(key); if (types == null) diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilTest.java index 92838feb54c..45f474aef9a 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilTest.java @@ -18,18 +18,22 @@ package org.eclipse.jetty.util; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Stream; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.net.URI; -import java.nio.charset.StandardCharsets; - -import org.junit.jupiter.api.Test; - - /** * URIUtil Tests. */ @@ -314,4 +318,109 @@ public class URIUtilTest assertThat(URIUtil.getJarSource(new URI("jar:file:///tmp/foo.jar")),is(new URI("file:///tmp/foo.jar"))); assertThat(URIUtil.getJarSource(new URI("jar:file:///tmp/foo.jar!/some/path")),is(new URI("file:///tmp/foo.jar"))); } + + public static Stream encodeSpaces() + { + List data = new ArrayList<>(); + + // [raw, expected] + + // null + data.add(new String[]{null, null}); + + // no spaces + data.add(new String[]{"abc", "abc"}); + + // match + data.add(new String[]{"a c", "a%20c"}); + data.add(new String[]{" ", "%20%20%20"}); + data.add(new String[]{"a%20space", "a%20space"}); + + return data.stream(); + } + + @ParameterizedTest + @MethodSource(value = "encodeSpaces") + public void testEncodeSpaces(String raw, String expected) + { + assertThat(URIUtil.encodeSpaces(raw), is(expected)); + } + + public static Stream encodeSpecific() + { + List data = new ArrayList<>(); + + // [raw, chars, expected] + + // null input + data.add(new String[]{null, null, null}); + + // null chars + data.add(new String[]{"abc", null, "abc"}); + + // empty chars + data.add(new String[]{"abc", "", "abc"}); + + // no matches + data.add(new String[]{"abc", ".;", "abc"}); + data.add(new String[]{"xyz", ".;", "xyz"}); + data.add(new String[]{":::", ".;", ":::"}); + + // matches + data.add(new String[]{"a c", " ", "a%20c"}); + data.add(new String[]{"name=value", "=", "name%3Dvalue"}); + data.add(new String[]{"This has fewer then 10% hits.", ".%", "This has fewer then 10%25 hits%2E"}); + + // partially encoded already + data.add(new String[]{"a%20name=value%20pair", "=", "a%20name%3Dvalue%20pair"}); + data.add(new String[]{"a%20name=value%20pair", "=%", "a%2520name%3Dvalue%2520pair"}); + + return data.stream(); + } + + @ParameterizedTest + @MethodSource(value = "encodeSpecific") + public void testEncodeSpecific(String raw, String chars, String expected) + { + assertThat(URIUtil.encodeSpecific(raw, chars), is(expected)); + } + + public static Stream decodeSpecific() + { + List data = new ArrayList<>(); + + // [raw, chars, expected] + + // null input + data.add(new String[]{null, null, null}); + + // null chars + data.add(new String[]{"abc", null, "abc"}); + + // empty chars + data.add(new String[]{"abc", "", "abc"}); + + // no matches + data.add(new String[]{"abc", ".;", "abc"}); + data.add(new String[]{"xyz", ".;", "xyz"}); + data.add(new String[]{":::", ".;", ":::"}); + + // matches + data.add(new String[]{"a%20c", " ", "a c"}); + data.add(new String[]{"name%3Dvalue", "=", "name=value"}); + data.add(new String[]{"This has fewer then 10%25 hits%2E", ".%", "This has fewer then 10% hits."}); + + // partially decode + data.add(new String[]{"a%20name%3Dvalue%20pair", "=", "a%20name=value%20pair"}); + data.add(new String[]{"a%2520name%3Dvalue%2520pair", "=%", "a%20name=value%20pair"}); + + return data.stream(); + } + + @ParameterizedTest + @MethodSource(value = "decodeSpecific") + public void testDecodeSpecific(String raw, String chars, String expected) + { + assertThat(URIUtil.decodeSpecific(raw, chars), is(expected)); + } } 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 1ca05559cc9..db395405695 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 @@ -41,6 +41,7 @@ import java.util.function.Supplier; import org.eclipse.jetty.io.RuntimeIOException; import org.eclipse.jetty.util.ArrayTernaryTrie; import org.eclipse.jetty.util.IncludeExcludeSet; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.log.Log; @@ -703,7 +704,7 @@ public class ClasspathPattern extends AbstractSet name=name.substring(0,name.length()-6); // Treat path elements as packages for name matching - name = name.replace('/', '.'); + name = StringUtil.replace(name, '/', '.'); return combine(_packageOrNamePatterns, name, _locations, ()-> { diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java index 967b9e5feaa..18231f5650a 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java @@ -41,7 +41,8 @@ import java.util.concurrent.CopyOnWriteArrayList; import org.eclipse.jetty.util.ClassVisibilityChecker; import org.eclipse.jetty.util.IO; -import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.TypeUtil; +import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.resource.Resource; @@ -338,12 +339,10 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility if(LOG.isDebugEnabled()) LOG.debug("addJar - {}", fn); String fnlc=fn.getName().toLowerCase(Locale.ENGLISH); - // don't check if this is a directory, see Bug 353165 + // don't check if this is a directory (prevents use of symlinks), see Bug 353165 if (isFileSupported(fnlc)) { - String jar=fn.toString(); - jar=StringUtil.replace(jar, ",", "%2C"); - jar=StringUtil.replace(jar, ";", "%3B"); + String jar = URIUtil.encodeSpecific(fn.toString(), ",;"); addClassPath(jar); } } @@ -630,7 +629,7 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility // Look in the webapp classloader as a resource, to avoid // loading a system class. Class webapp_class = null; - String path = name.replace('.', '/').concat(".class"); + String path = TypeUtil.toClassReference(name); URL webapp_url = findResource(path); if (webapp_url!=null && (!checkSystemResource || !_context.isSystemResource(name,webapp_url))) @@ -656,7 +655,7 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility return super.findClass(name); } - String path = name.replace('.', '/').concat(".class"); + String path = TypeUtil.toClassReference(name); URL url = findResource(path); if (url==null) throw new ClassNotFoundException(name); diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java index 20aec0b824d..d7339989fb1 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebInfConfiguration.java @@ -211,8 +211,7 @@ public class WebInfConfiguration extends AbstractConfiguration } catch (URISyntaxException e) { - String fixedUriStr = StringUtil.replace(u.toString(), " ", "%20"); - containerUris.add(new URI(fixedUriStr)); + containerUris.add(new URI(URIUtil.encodeSpaces(u.toString()))); } } } @@ -813,10 +812,7 @@ public class WebInfConfiguration extends AbstractConfiguration } //Context name - String contextPath = context.getContextPath(); - contextPath=contextPath.replace('/','_'); - contextPath=contextPath.replace('\\','_'); - canonicalName.append(contextPath); + canonicalName.append(context.getContextPath()); //Virtual host (if there is one) canonicalName.append("-"); @@ -826,17 +822,9 @@ public class WebInfConfiguration extends AbstractConfiguration else canonicalName.append(vhosts[0]); - // sanitize - for (int i=0;i clazz) throws Exception { ClassLoader cl = Thread.currentThread().getContextClassLoader(); - String endpointPath = clazz.getName().replace('.','/') + ".class"; + String endpointPath = TypeUtil.toClassReference(clazz); URL classUrl = cl.getResource(endpointPath); assertThat("Class URL for: " + clazz,classUrl,notNullValue()); File destFile = Paths.get( classesDir.toString(), FS.separators( endpointPath)).toFile(); diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WSServer.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WSServer.java index 3bdc4bf257a..f5627707216 100644 --- a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WSServer.java +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/WSServer.java @@ -37,6 +37,7 @@ import org.eclipse.jetty.toolchain.test.IO; import org.eclipse.jetty.toolchain.test.JAR; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; +import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.resource.Resource; @@ -82,7 +83,7 @@ public class WSServer public void copyClass(Class clazz) throws Exception { ClassLoader cl = Thread.currentThread().getContextClassLoader(); - String endpointPath = clazz.getName().replace('.','/') + ".class"; + String endpointPath = TypeUtil.toClassReference(clazz); URL classUrl = cl.getResource(endpointPath); assertThat("Class URL for: " + clazz,classUrl,notNullValue()); File destFile = new File(classesDir,FS.separators(endpointPath)); diff --git a/tests/test-integration/src/test/java/org/eclipse/jetty/test/support/TestableJettyServer.java b/tests/test-integration/src/test/java/org/eclipse/jetty/test/support/TestableJettyServer.java index a612e6592e7..a59f93d7a72 100644 --- a/tests/test-integration/src/test/java/org/eclipse/jetty/test/support/TestableJettyServer.java +++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/support/TestableJettyServer.java @@ -18,18 +18,16 @@ package org.eclipse.jetty.test.support; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.io.File; -import java.io.FileOutputStream; import java.io.IOException; +import java.io.OutputStream; import java.net.InetAddress; import java.net.MalformedURLException; import java.net.URI; import java.net.URL; import java.net.UnknownHostException; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -39,9 +37,14 @@ import java.util.Properties; import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.server.NetworkConnector; import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.xml.XmlConfiguration; import org.junit.jupiter.api.Disabled; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + /** * Allows for setting up a Jetty server for testing based on XML configuration files. */ @@ -55,8 +58,8 @@ public class TestableJettyServer private String _scheme = HttpScheme.HTTP.asString(); /* Popular Directories */ - private File baseDir; - private File testResourcesDir; + private Path baseDir; + private Path testResourcesDir; public TestableJettyServer() throws IOException { @@ -64,31 +67,28 @@ public class TestableJettyServer Properties properties = new Properties(); /* Establish Popular Directories */ - String baseDirPath = System.getProperty("basedir"); - if (baseDirPath == null) - { - baseDirPath = System.getProperty("user.dir","."); - } - baseDir = new File(baseDirPath); - properties.setProperty("test.basedir",baseDir.getAbsolutePath()); + baseDir = MavenTestingUtils.getBasePath(); + properties.setProperty("test.basedir",baseDir.toString()); - testResourcesDir = new File(baseDirPath,"src/test/resources".replace('/',File.separatorChar)); - properties.setProperty("test.resourcesdir",testResourcesDir.getAbsolutePath()); + testResourcesDir = MavenTestingUtils.getTestResourcesPath(); + properties.setProperty("test.resourcesdir",testResourcesDir.toString()); - File testDocRoot = new File(testResourcesDir,"docroots"); - properties.setProperty("test.docroot.base",testDocRoot.getAbsolutePath()); + Path testDocRoot = MavenTestingUtils.getTestResourcePathDir("docroots"); + properties.setProperty("test.docroot.base",testDocRoot.toString()); - File targetDir = new File(baseDir,"target"); - properties.setProperty("test.targetdir",targetDir.getAbsolutePath()); + Path targetDir = MavenTestingUtils.getTargetPath(); + properties.setProperty("test.targetdir",targetDir.toString()); - File webappsDir = new File(targetDir,"webapps"); - properties.setProperty("test.webapps",webappsDir.getAbsolutePath()); + Path webappsDir = MavenTestingUtils.getTargetPath("webapps"); + properties.setProperty("test.webapps",webappsDir.toString()); // Write out configuration for use by ConfigurationManager. - File testConfig = new File(targetDir,"testable-jetty-server-config.properties"); - FileOutputStream out = new FileOutputStream(testConfig); - properties.store(out,"Generated by " + TestableJettyServer.class.getName()); - + Path testConfig = targetDir.resolve("testable-jetty-server-config.properties"); + try(OutputStream out = Files.newOutputStream(testConfig)) + { + properties.store(out,"Generated by " + TestableJettyServer.class.getName()); + } + for (Object key:properties.keySet()) _properties.put(String.valueOf(key),String.valueOf(properties.get(key))); } @@ -105,7 +105,7 @@ public class TestableJettyServer public void addConfiguration(String testConfigName) throws MalformedURLException { - addConfiguration(new File(testResourcesDir,testConfigName)); + addConfiguration(MavenTestingUtils.getTestResourceFile(testConfigName)); } public void setProperty(String key, String value) diff --git a/tests/test-webapps/test-servlet-spec/test-spec-webapp/src/main/java/com/acme/test/ClassLoaderServlet.java b/tests/test-webapps/test-servlet-spec/test-spec-webapp/src/main/java/com/acme/test/ClassLoaderServlet.java index ad348ff2a0d..59f1d8eed90 100644 --- a/tests/test-webapps/test-servlet-spec/test-spec-webapp/src/main/java/com/acme/test/ClassLoaderServlet.java +++ b/tests/test-webapps/test-servlet-spec/test-spec-webapp/src/main/java/com/acme/test/ClassLoaderServlet.java @@ -24,7 +24,6 @@ import java.net.URISyntaxException; import java.net.URL; import java.security.CodeSource; import java.security.ProtectionDomain; - import javax.servlet.ServletException; import javax.servlet.annotation.WebServlet; import javax.servlet.http.HttpServlet; From adb6e7c8d2702d52a4ce0af34aec51dead0b88c9 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 12 Jun 2019 10:39:05 -0500 Subject: [PATCH 06/22] Issue #3708 - Correcting for testcases Signed-off-by: Joakim Erdfelt --- .../jetty/annotations/AnnotationParser.java | 17 ++++++++--------- .../jetty/plus/jndi/NamingEntryUtil.java | 4 ++-- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java index 5b9da0b87a3..786d7c468c6 100644 --- a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java +++ b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java @@ -601,13 +601,12 @@ public class AnnotationParser if (className == null) return; - String tmp = className; - className = TypeUtil.toClassReference(className); - URL resource = Loader.getResource(className); + String classRef = TypeUtil.toClassReference(className); + URL resource = Loader.getResource(classRef); if (resource!= null) { Resource r = Resource.newResource(resource); - addParsedClass(tmp, r); + addParsedClass(className, r); try (InputStream is = r.getInputStream()) { scanClass(handlers, null, is); @@ -673,16 +672,16 @@ public class AnnotationParser { MultiException me = new MultiException(); - for (String s:classNames) + for (String className : classNames) { try { - String name = TypeUtil.toClassReference(s); - URL resource = Loader.getResource(s); + String classRef = TypeUtil.toClassReference(className); + URL resource = Loader.getResource(classRef); if (resource!= null) { Resource r = Resource.newResource(resource); - addParsedClass(name, r); + addParsedClass(className, r); try (InputStream is = r.getInputStream()) { scanClass(handlers, null, is); @@ -691,7 +690,7 @@ public class AnnotationParser } catch (Exception e) { - me.add(new RuntimeException("Error scanning class "+s, e)); + me.add(new RuntimeException("Error scanning class "+className, e)); } } me.ifExceptionThrow(); diff --git a/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingEntryUtil.java b/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingEntryUtil.java index 956488be3a0..e005d4337b2 100644 --- a/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingEntryUtil.java +++ b/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingEntryUtil.java @@ -240,8 +240,8 @@ public class NamingEntryUtil return ""; String str = scope.getClass().getName() + "@" + Long.toHexString(scope.hashCode()); - // TODO: Is this sanitize step still needed? (this canonicalizeScope method has been reduced in functionality a lot over the years) - str = StringUtil.sanitizeFileSystemName(str); + str = StringUtil.replace(str, '/', '_'); + str = StringUtil.replace(str, ' ', '_'); return str; } } From 82f7647629022e9152087012db672ea54008d6c8 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 12 Jun 2019 19:15:15 +0200 Subject: [PATCH 07/22] Issue #3758 - Avoid sending empty trailer frames for http/2 requests. Added one more test case and comments about handling of `content.isConsumed()` in HTTP/2. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/client/HttpSender.java | 2 +- .../jetty/client/http/HttpSenderOverHTTP.java | 4 +- .../client/http/HttpSenderOverHTTP2.java | 3 ++ .../client/http/RequestTrailersTest.java | 48 +++++++++++++++++++ 4 files changed, 54 insertions(+), 3 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java index 99b865bd70a..d29c09663b8 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java @@ -740,7 +740,7 @@ public abstract class HttpSender implements AsyncContentProvider.Listener } else { - // Was any content sent while committing ? + // Was any content sent while committing? ByteBuffer contentBuffer = content.getContent(); if (contentBuffer != null) { diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpSenderOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpSenderOverHTTP.java index 1f8d04397da..1edae133609 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpSenderOverHTTP.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpSenderOverHTTP.java @@ -94,8 +94,8 @@ public class HttpSenderOverHTTP extends HttpSender } case NEED_CHUNK_TRAILER: { - callback.succeeded(); - return; + chunk = bufferPool.acquire(httpClient.getRequestBufferSize(), false); + break; } case FLUSH: { diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpSenderOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpSenderOverHTTP2.java index e376a91eeac..e1078ad0976 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpSenderOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpSenderOverHTTP2.java @@ -133,6 +133,9 @@ public class HttpSenderOverHTTP2 extends HttpSender { if (content.isConsumed()) { + // The superclass calls sendContent() one more time after the last content. + // This is necessary for HTTP/1.1 to generate the terminal chunk (with trailers), + // but it's not necessary for HTTP/2 so we just succeed the callback. callback.succeeded(); } else diff --git a/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/RequestTrailersTest.java b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/RequestTrailersTest.java index e6d321632d3..46778f30d9a 100644 --- a/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/RequestTrailersTest.java +++ b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/RequestTrailersTest.java @@ -139,4 +139,52 @@ public class RequestTrailersTest extends AbstractTest assertTrue(latch.await(5, TimeUnit.SECONDS)); } + + @Test + public void testEmptyTrailersWithEmptyDeferredContent() throws Exception + { + start(new ServerSessionListener.Adapter() + { + @Override + public Stream.Listener onNewStream(Stream stream, HeadersFrame frame) + { + return new Stream.Listener.Adapter() + { + @Override + public void onData(Stream stream, DataFrame dataFrame, Callback callback) + { + callback.succeeded(); + // We should not receive an empty HEADERS frame for the + // trailers, but instead a DATA frame with endStream=true. + if (dataFrame.isEndStream()) + { + MetaData.Response response = new MetaData.Response(HttpVersion.HTTP_2, HttpStatus.OK_200, new HttpFields()); + HeadersFrame responseFrame = new HeadersFrame(stream.getId(), response, null, true); + stream.headers(responseFrame, Callback.NOOP); + } + } + }; + } + }); + + HttpRequest request = (HttpRequest)client.newRequest("localhost", connector.getLocalPort()); + HttpFields trailers = new HttpFields(); + request.trailers(() -> trailers); + DeferredContentProvider content = new DeferredContentProvider(); + request.content(content); + + CountDownLatch latch = new CountDownLatch(1); + request.send(result -> + { + assertTrue(result.isSucceeded()); + assertEquals(HttpStatus.OK_200, result.getResponse().getStatus()); + latch.countDown(); + }); + + // Send deferred content after a while. + Thread.sleep(1000); + content.close(); + + assertTrue(latch.await(5, TimeUnit.SECONDS)); + } } From e81fdbd7c7b7388b079edc8c2ec170fa1e7aba18 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 12 Jun 2019 13:38:45 -0500 Subject: [PATCH 08/22] Issue #3736 - Give better exception when using bad classloader impl Signed-off-by: Joakim Erdfelt --- .../main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java index 967b9e5feaa..e043e6fb797 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java @@ -498,7 +498,9 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility // Try the parent loader try { - parent_class = _parent.loadClass(name); + parent_class = _parent.loadClass(name); + if (parent_class == null) + throw new ClassNotFoundException("Bad ClassLoader: returned null for loadClass(" + name + ")"); // If the webapp is allowed to see this class if (Boolean.TRUE.equals(__loadServerClasses.get()) || !_context.isServerClass(parent_class)) From 9e368726d76802e5e256250c4bc5e557249a1f97 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 12 Jun 2019 15:10:22 -0500 Subject: [PATCH 09/22] Attempting to address stats difference --- .../websocket/tests/WebSocketConnectionStatsTest.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketConnectionStatsTest.java b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketConnectionStatsTest.java index 97ca787eba5..68b16d1d909 100644 --- a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketConnectionStatsTest.java +++ b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketConnectionStatsTest.java @@ -33,7 +33,9 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.websocket.api.CloseStatus; import org.eclipse.jetty.websocket.api.Session; +import org.eclipse.jetty.websocket.api.StatusCode; import org.eclipse.jetty.websocket.api.WebSocketPolicy; import org.eclipse.jetty.websocket.api.annotations.OnWebSocketClose; import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect; @@ -193,9 +195,11 @@ public class WebSocketConnectionStatsTest { session.getRemote().sendString(msgText); } + session.close(StatusCode.NORMAL, null); + + assertTrue(socket.closed.await(5, TimeUnit.SECONDS)); + assertTrue(wsConnectionClosed.await(5, TimeUnit.SECONDS)); } - assertTrue(socket.closed.await(5, TimeUnit.SECONDS)); - assertTrue(wsConnectionClosed.await(5, TimeUnit.SECONDS)); assertThat(statistics.getConnectionsMax(), is(1L)); assertThat(statistics.getConnections(), is(0L)); From 370a85b921ecbeb244c506cb3fdb5c7d8cd5ed09 Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Thu, 13 Jun 2019 11:28:22 +1000 Subject: [PATCH 10/22] Jetty 9.4.x aggregate javadoc fix (#3780) * fix javadoc plugin configuration fixing searchbox when navigating to class found, as jetty 10 has module defined we probably do not need to merge this to 10 except other parameters Signed-off-by: olivier lamy --- pom.xml | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/pom.xml b/pom.xml index 0c6dd675e32..ad9030f57c0 100644 --- a/pom.xml +++ b/pom.xml @@ -283,7 +283,7 @@ true false javadoc:aggregate-jar deploy - -Peclipse-release -Paggregate-javadoc-jar + -Peclipse-release clean install forked-path @@ -534,20 +534,18 @@ org.apache.maven.plugins maven-javadoc-plugin - 3.0.1 + 3.1.0 8 - 8 UTF-8 UTF-8 UTF-8 - -html5 true false false protected true - com.acme,org.slf4j*,org.mortbay*,*.jmh*,org.eclipse.jetty.embedded*,org.eclipse.jetty.example.asyncrest*,org.eclipse.jetty.test* + com.*:org.slf4j*:org.mortbay*:*.jmh*:org.eclipse.jetty.embedded*:org.eclipse.jetty.example.asyncrest*:org.eclipse.jetty.test* @@ -1128,6 +1126,27 @@ + + jdk11 + + [11,) + + + + + + org.apache.maven.plugins + maven-javadoc-plugin + + --no-module-directories + -html5 + com.*:org.slf4j*:org.mortbay*:*.jmh*:org.eclipse.jetty.embedded*:org.eclipse.jetty.example.asyncrest*:org.eclipse.jetty.test* + + + + + + config From c94753ece3f79715c5d0707c60dcdfed17d678c9 Mon Sep 17 00:00:00 2001 From: Lachlan Date: Thu, 13 Jun 2019 15:51:22 +1000 Subject: [PATCH 11/22] Issue #3746 - fix WriteFlusher ClassCastException if Callback throws (#3778) Signed-off-by: Lachlan Roberts --- .../org/eclipse/jetty/io/WriteFlusher.java | 31 ++++++++++- .../eclipse/jetty/io/WriteFlusherTest.java | 55 ++++++++++++++++--- 2 files changed, 75 insertions(+), 11 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/WriteFlusher.java b/jetty-io/src/main/java/org/eclipse/jetty/io/WriteFlusher.java index 9359e38f7d2..e55263c1954 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/WriteFlusher.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/WriteFlusher.java @@ -307,9 +307,36 @@ abstract public class WriteFlusher private void fail(Callback callback, Throwable... suppressed) { - FailedState failed = (FailedState)_state.get(); + Throwable cause; + loop: + while (true) + { + State state = _state.get(); + + switch (state.getType()) + { + case FAILED: + { + FailedState failed = (FailedState)state; + cause = failed.getCause(); + break loop; + } + + case IDLE: + for (Throwable t : suppressed) + LOG.warn(t); + return; + + default: + Throwable t = new IllegalStateException(); + if (!_state.compareAndSet(state, new FailedState(t))) + continue; + + cause = t; + break loop; + } + } - Throwable cause = failed.getCause(); for (Throwable t : suppressed) { if (t != cause) diff --git a/jetty-io/src/test/java/org/eclipse/jetty/io/WriteFlusherTest.java b/jetty-io/src/test/java/org/eclipse/jetty/io/WriteFlusherTest.java index 185bc0a783e..f1d6aeb2b32 100644 --- a/jetty-io/src/test/java/org/eclipse/jetty/io/WriteFlusherTest.java +++ b/jetty-io/src/test/java/org/eclipse/jetty/io/WriteFlusherTest.java @@ -18,14 +18,6 @@ package org.eclipse.jetty.io; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.instanceOf; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.io.IOException; import java.io.InterruptedIOException; import java.nio.ByteBuffer; @@ -43,10 +35,18 @@ import java.util.concurrent.atomic.AtomicBoolean; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.FutureCallback; +import org.eclipse.jetty.util.log.StacklessLogging; import org.hamcrest.Matchers; - import org.junit.jupiter.api.Test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.instanceOf; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + public class WriteFlusherTest { @Test @@ -159,6 +159,43 @@ public class WriteFlusherTest assertTrue(flusher.isIdle()); } + @Test + public void testCallbackThrows() throws Exception + { + ByteArrayEndPoint endPoint = new ByteArrayEndPoint(new byte[0], 100); + + AtomicBoolean incompleteFlush = new AtomicBoolean(false); + WriteFlusher flusher = new WriteFlusher(endPoint) + { + @Override + protected void onIncompleteFlush() + { + incompleteFlush.set(true); + } + }; + + FutureCallback callback = new FutureCallback() + { + @Override + public void succeeded() + { + super.succeeded(); + throw new IllegalStateException(); + } + }; + + try (StacklessLogging stacklessLogging = new StacklessLogging(WriteFlusher.class)) + { + flusher.write(callback, BufferUtil.toBuffer("How now brown cow!")); + callback.get(100, TimeUnit.MILLISECONDS); + } + + assertEquals("How now brown cow!", endPoint.takeOutputString()); + assertTrue(callback.isDone()); + assertFalse(incompleteFlush.get()); + assertTrue(flusher.isIdle()); + } + @Test public void testCloseWhileBlocking() throws Exception { From f54b6825283468ddbef6e9eaff2082e3511b180b Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Sun, 16 Jun 2019 11:12:40 +0200 Subject: [PATCH 12/22] Fixes #3786 - ALPN support for Java 14. Signed-off-by: Simone Bordet --- .../src/main/config/modules/alpn-impl/alpn-14.mod | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 jetty-alpn/jetty-alpn-server/src/main/config/modules/alpn-impl/alpn-14.mod diff --git a/jetty-alpn/jetty-alpn-server/src/main/config/modules/alpn-impl/alpn-14.mod b/jetty-alpn/jetty-alpn-server/src/main/config/modules/alpn-impl/alpn-14.mod new file mode 100644 index 00000000000..689601a4197 --- /dev/null +++ b/jetty-alpn/jetty-alpn-server/src/main/config/modules/alpn-impl/alpn-14.mod @@ -0,0 +1,4 @@ +DO NOT EDIT - See: https://www.eclipse.org/jetty/documentation/current/startup-modules.html + +[depend] +alpn-impl/alpn-9 From 1550d4f59a7d36ba9f18cf8191eec614e2312efe Mon Sep 17 00:00:00 2001 From: olivier lamy Date: Sat, 15 Jun 2019 11:09:36 +1000 Subject: [PATCH 13/22] avoid depending on a range dependency from a transitive dependency Signed-off-by: olivier lamy --- jetty-gcloud/jetty-gcloud-session-manager/pom.xml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/jetty-gcloud/jetty-gcloud-session-manager/pom.xml b/jetty-gcloud/jetty-gcloud-session-manager/pom.xml index 15c103fd57e..f9088835f35 100644 --- a/jetty-gcloud/jetty-gcloud-session-manager/pom.xml +++ b/jetty-gcloud/jetty-gcloud-session-manager/pom.xml @@ -33,6 +33,13 @@ + + + io.grpc + grpc-core + 1.0.1 + compile + com.google.auto.value auto-value From a18bd12ada2aabbb72cd142b05fffb0af6cabd6d Mon Sep 17 00:00:00 2001 From: Lachlan Date: Thu, 13 Jun 2019 15:43:19 +1000 Subject: [PATCH 14/22] Issue #3762 - use the default port of 0 for WebSocket tests Signed-off-by: Lachlan Roberts --- .../tests/WebSocketConnectionStatsTest.java | 72 +++---------------- .../tests/client/BadNetworkTest.java | 1 - .../tests/client/ClientCloseTest.java | 1 - .../tests/client/ClientSessionsTest.java | 1 - .../tests/client/SlowClientTest.java | 1 - .../tests/server/ServerCloseTest.java | 1 - .../tests/server/SlowServerTest.java | 1 - 7 files changed, 10 insertions(+), 68 deletions(-) diff --git a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketConnectionStatsTest.java b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketConnectionStatsTest.java index 68b16d1d909..a17eb1a7089 100644 --- a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketConnectionStatsTest.java +++ b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketConnectionStatsTest.java @@ -33,15 +33,9 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.util.BufferUtil; -import org.eclipse.jetty.websocket.api.CloseStatus; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.api.StatusCode; import org.eclipse.jetty.websocket.api.WebSocketPolicy; -import org.eclipse.jetty.websocket.api.annotations.OnWebSocketClose; -import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect; -import org.eclipse.jetty.websocket.api.annotations.OnWebSocketError; -import org.eclipse.jetty.websocket.api.annotations.OnWebSocketMessage; -import org.eclipse.jetty.websocket.api.annotations.WebSocket; import org.eclipse.jetty.websocket.client.WebSocketClient; import org.eclipse.jetty.websocket.common.CloseInfo; import org.eclipse.jetty.websocket.common.Generator; @@ -60,52 +54,6 @@ import static org.junit.jupiter.api.Assertions.assertTrue; public class WebSocketConnectionStatsTest { - - @WebSocket - public static class ClientSocket - { - CountDownLatch closed = new CountDownLatch(1); - int closeStatus; - String closeReason; - String behavior; - - @OnWebSocketConnect - public void onOpen(Session session) - { - behavior = session.getPolicy().getBehavior().name(); - } - - @OnWebSocketClose - public void onClose(int statusCode, String reason) - { - closeStatus = statusCode; - closeReason = reason; - closed.countDown(); - } - - @OnWebSocketError - public void onError(Throwable cause) - { - cause.printStackTrace(System.err); - } - - @Override - public String toString() - { - return String.format("[%s@%s]", behavior, Integer.toHexString(hashCode())); - } - } - - @WebSocket - public static class EchoSocket extends ClientSocket - { - @OnWebSocketMessage - public void onMessage(Session session, String message) - { - session.getRemote().sendString(message, null); - } - } - public static class MyWebSocketServlet extends WebSocketServlet { @Override @@ -115,11 +63,12 @@ public class WebSocketConnectionStatsTest } } - Server server; - WebSocketClient client; - ConnectionStatistics statistics; - CountDownLatch wsUpgradeComplete = new CountDownLatch(1); - CountDownLatch wsConnectionClosed = new CountDownLatch(1); + private Server server; + private ServerConnector connector; + private WebSocketClient client; + private ConnectionStatistics statistics; + private CountDownLatch wsUpgradeComplete = new CountDownLatch(1); + private CountDownLatch wsConnectionClosed = new CountDownLatch(1); @BeforeEach public void start() throws Exception @@ -139,8 +88,7 @@ public class WebSocketConnectionStatsTest }; server = new Server(); - ServerConnector connector = new ServerConnector(server); - connector.setPort(8080); + connector = new ServerConnector(server); connector.addBean(statistics); server.addConnector(connector); @@ -175,8 +123,8 @@ public class WebSocketConnectionStatsTest @Test public void echoStatsTest() throws Exception { - URI uri = URI.create("ws://localhost:8080/testPath"); - ClientSocket socket = new ClientSocket(); + URI uri = URI.create("ws://localhost:"+connector.getLocalPort()+"/testPath"); + EventSocket socket = new EventSocket(); Future connect = client.connect(socket, uri); final long numMessages = 10000; @@ -208,7 +156,7 @@ public class WebSocketConnectionStatsTest assertThat(statistics.getReceivedMessages(), is(numMessages + 2L)); WebSocketFrame textFrame = new TextFrame().setPayload(msgText); - WebSocketFrame closeFrame = new CloseInfo(socket.closeStatus, socket.closeReason).asFrame(); + WebSocketFrame closeFrame = new CloseInfo(socket.closeCode, socket.closeReason).asFrame(); final long textFrameSize = getFrameByteSize(textFrame); final long closeFrameSize = getFrameByteSize(closeFrame); diff --git a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/BadNetworkTest.java b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/BadNetworkTest.java index 2b51caf95d6..fe6df99bec0 100644 --- a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/BadNetworkTest.java +++ b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/BadNetworkTest.java @@ -80,7 +80,6 @@ public class BadNetworkTest server = new Server(); connector = new ServerConnector(server); - connector.setPort(0); server.addConnector(connector); context = new ServletContextHandler(); diff --git a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientCloseTest.java b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientCloseTest.java index c28d5aadae1..cd858295826 100644 --- a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientCloseTest.java +++ b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientCloseTest.java @@ -119,7 +119,6 @@ public class ClientCloseTest server = new Server(); ServerConnector connector = new ServerConnector(server); - connector.setPort(0); server.addConnector(connector); ServletContextHandler context = new ServletContextHandler(); diff --git a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientSessionsTest.java b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientSessionsTest.java index 8ce7e34a8da..dda19b04604 100644 --- a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientSessionsTest.java +++ b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientSessionsTest.java @@ -63,7 +63,6 @@ public class ClientSessionsTest server = new Server(); ServerConnector connector = new ServerConnector(server); - connector.setPort(0); server.addConnector(connector); ServletContextHandler context = new ServletContextHandler(); diff --git a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/SlowClientTest.java b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/SlowClientTest.java index 27045eb17aa..f2bbde10ef5 100644 --- a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/SlowClientTest.java +++ b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/SlowClientTest.java @@ -65,7 +65,6 @@ public class SlowClientTest server = new Server(); ServerConnector connector = new ServerConnector(server); - connector.setPort(0); server.addConnector(connector); ServletContextHandler context = new ServletContextHandler(); diff --git a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/server/ServerCloseTest.java b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/server/ServerCloseTest.java index 017482187ce..66260942988 100644 --- a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/server/ServerCloseTest.java +++ b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/server/ServerCloseTest.java @@ -65,7 +65,6 @@ public class ServerCloseTest server = new Server(); ServerConnector connector = new ServerConnector(server); - connector.setPort(0); server.addConnector(connector); ServletContextHandler context = new ServletContextHandler(); diff --git a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/server/SlowServerTest.java b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/server/SlowServerTest.java index 19fb2cae723..2e8000be152 100644 --- a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/server/SlowServerTest.java +++ b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/server/SlowServerTest.java @@ -64,7 +64,6 @@ public class SlowServerTest server = new Server(); ServerConnector connector = new ServerConnector(server); - connector.setPort(0); server.addConnector(connector); ServletContextHandler context = new ServletContextHandler(); From 2f3eed86049600b9da385112168f7f8b658fd582 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 14 Jun 2019 11:20:08 -0500 Subject: [PATCH 15/22] Issue #3782 - reducing noise during testing Signed-off-by: Joakim Erdfelt --- jetty-server/src/test/resources/jetty-logging.properties | 1 + 1 file changed, 1 insertion(+) diff --git a/jetty-server/src/test/resources/jetty-logging.properties b/jetty-server/src/test/resources/jetty-logging.properties index 21db0759fe3..1ba30af5c5f 100644 --- a/jetty-server/src/test/resources/jetty-logging.properties +++ b/jetty-server/src/test/resources/jetty-logging.properties @@ -1,4 +1,5 @@ org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog +org.eclipse.jetty.LEVEL=WARN #org.eclipse.jetty.LEVEL=DEBUG #org.eclipse.jetty.server.LEVEL=DEBUG #org.eclipse.jetty.server.ConnectionLimit.LEVEL=DEBUG From a1fe57a654db6a44449c0c7e40acb8217cc7eb28 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 14 Jun 2019 11:20:31 -0500 Subject: [PATCH 16/22] Issue #3782 - updating existing testcases Signed-off-by: Joakim Erdfelt --- .../ForwardedRequestCustomizerTest.java | 682 ++++++++++-------- 1 file changed, 377 insertions(+), 305 deletions(-) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java index db8a9ad64e0..5c38e601d52 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java @@ -19,25 +19,20 @@ package org.eclipse.jetty.server; import java.io.IOException; -import java.util.ArrayDeque; -import java.util.Deque; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; - import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.eclipse.jetty.http.MimeTypes; +import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.server.handler.AbstractHandler; -import org.eclipse.jetty.util.IO; -import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -46,11 +41,15 @@ public class ForwardedRequestCustomizerTest private Server _server; private LocalConnector _connector; private RequestHandler _handler; - final Deque _results = new ArrayDeque<>(); final AtomicBoolean _wasSecure = new AtomicBoolean(false); final AtomicReference _sslSession = new AtomicReference<>(); final AtomicReference _sslCertificate = new AtomicReference<>(); - + final AtomicReference _scheme = new AtomicReference<>(); + final AtomicReference _serverName = new AtomicReference<>(); + final AtomicReference _serverPort = new AtomicReference<>(); + final AtomicReference _remoteAddr = new AtomicReference<>(); + final AtomicReference _remotePort = new AtomicReference<>(); + ForwardedRequestCustomizer _customizer; @BeforeEach @@ -62,29 +61,25 @@ public class ForwardedRequestCustomizerTest http.getHttpConfiguration().setRequestHeaderSize(512); http.getHttpConfiguration().setResponseHeaderSize(512); http.getHttpConfiguration().setOutputBufferSize(2048); - http.getHttpConfiguration().addCustomizer(_customizer=new ForwardedRequestCustomizer()); - _connector = new LocalConnector(_server,http); + http.getHttpConfiguration().addCustomizer(_customizer = new ForwardedRequestCustomizer()); + _connector = new LocalConnector(_server, http); _server.addConnector(_connector); _handler = new RequestHandler(); _server.setHandler(_handler); - _handler._checker = new RequestTester() + _handler._checker = (request, response) -> { - @Override - public boolean check(HttpServletRequest request,HttpServletResponse response) - { - _wasSecure.set(request.isSecure()); - _sslSession.set(String.valueOf(request.getAttribute("javax.servlet.request.ssl_session_id"))); - _sslCertificate.set(String.valueOf(request.getAttribute("javax.servlet.request.cipher_suite"))); - _results.add(request.getScheme()); - _results.add(request.getServerName()); - _results.add(Integer.toString(request.getServerPort())); - _results.add(request.getRemoteAddr()); - _results.add(Integer.toString(request.getRemotePort())); - return true; - } + _wasSecure.set(request.isSecure()); + _sslSession.set(String.valueOf(request.getAttribute("javax.servlet.request.ssl_session_id"))); + _sslCertificate.set(String.valueOf(request.getAttribute("javax.servlet.request.cipher_suite"))); + _scheme.set(request.getScheme()); + _serverName.set(request.getServerName()); + _serverPort.set(request.getServerPort()); + _remoteAddr.set(request.getRemoteAddr()); + _remotePort.set(request.getRemotePort()); + return true; }; - + _server.start(); } @@ -92,391 +87,468 @@ public class ForwardedRequestCustomizerTest public void destroy() throws Exception { _server.stop(); - _server.join(); } @Test public void testHostIpv4() throws Exception { - String response=_connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: 1.2.3.4:2222\n"+ - "\n"); - assertThat(response, Matchers.containsString("200 OK")); - assertEquals("http",_results.poll()); - assertEquals("1.2.3.4",_results.poll()); - assertEquals("2222",_results.poll()); - assertEquals("0.0.0.0",_results.poll()); - assertEquals("0",_results.poll()); + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: 1.2.3.4:2222\n" + + "\n")); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("1.2.3.4")); + assertThat("serverPort", _serverPort.get(), is(2222)); } @Test public void testHostIpv6() throws Exception { - String response=_connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: [::1]:2222\n"+ - "\n"); - assertThat(response, Matchers.containsString("200 OK")); - assertEquals("http",_results.poll()); - assertEquals("[::1]",_results.poll()); - assertEquals("2222",_results.poll()); - assertEquals("0.0.0.0",_results.poll()); - assertEquals("0",_results.poll()); + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: [::1]:2222\n" + + "\n")); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("[::1]")); + assertThat("serverPort", _serverPort.get(), is(2222)); } - - @Test public void testURIIpv4() throws Exception { - String response=_connector.getResponse( - "GET http://1.2.3.4:2222/ HTTP/1.1\n"+ - "Host: wrong\n"+ - "\n"); - assertThat(response, Matchers.containsString("200 OK")); - assertEquals("http",_results.poll()); - assertEquals("1.2.3.4",_results.poll()); - assertEquals("2222",_results.poll()); - assertEquals("0.0.0.0",_results.poll()); - assertEquals("0",_results.poll()); + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET http://1.2.3.4:2222/ HTTP/1.1\n" + + "Host: wrong\n" + + "\n")); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("1.2.3.4")); + assertThat("serverPort", _serverPort.get(), is(2222)); } @Test public void testURIIpv6() throws Exception { - String response=_connector.getResponse( - "GET http://[::1]:2222/ HTTP/1.1\n"+ - "Host: wrong\n"+ - "\n"); - assertThat(response, Matchers.containsString("200 OK")); - assertEquals("http",_results.poll()); - assertEquals("[::1]",_results.poll()); - assertEquals("2222",_results.poll()); - assertEquals("0.0.0.0",_results.poll()); - assertEquals("0",_results.poll()); + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET http://[::1]:2222/ HTTP/1.1\n" + + "Host: wrong\n" + + "\n")); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("[::1]")); + assertThat("serverPort", _serverPort.get(), is(2222)); } - + /** + * RFC 7239: Section 4 + * + * Examples of syntax. + */ @Test public void testRFC7239_Examples_4() throws Exception { - String response=_connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: myhost\n"+ - "Forwarded: for=\"_gazonk\"\n"+ - "Forwarded: For=\"[2001:db8:cafe::17]:4711\"\n"+ - "Forwarded: for=192.0.2.60;proto=http;by=203.0.113.43\n"+ - "Forwarded: for=192.0.2.43, for=198.51.100.17\n"+ - "\n"); - assertThat(response, Matchers.containsString("200 OK")); - assertEquals("http",_results.poll()); - assertEquals("myhost",_results.poll()); - assertEquals("80",_results.poll()); - assertEquals("[2001:db8:cafe::17]",_results.poll()); - assertEquals("4711",_results.poll()); + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "Forwarded: for=\"_gazonk\"\n" + + "Forwarded: For=\"[2001:db8:cafe::17]:4711\"\n" + + "Forwarded: for=192.0.2.60;proto=http;by=203.0.113.43\n" + + "Forwarded: for=192.0.2.43, for=198.51.100.17\n" + + "\n")); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("remoteAddr", _remoteAddr.get(), is("[2001:db8:cafe::17]")); + assertThat("remotePort", _remotePort.get(), is(4711)); } - + + /** + * RFC 7239: Section 7.1 + * + * Examples of syntax with regards to HTTP header fields + */ @Test public void testRFC7239_Examples_7_1() throws Exception { - _connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: myhost\n"+ - "Forwarded: for=192.0.2.43,for=\"[2001:db8:cafe::17]\",for=unknown\n"+ - "\n"); - _connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: myhost\n"+ - "Forwarded: for=192.0.2.43, for=\"[2001:db8:cafe::17]\", for=unknown\n"+ - "\n"); - _connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: myhost\n"+ - "Forwarded: for=192.0.2.43\n"+ - "Forwarded: for=\"[2001:db8:cafe::17]\", for=unknown\n"+ - "\n"); + // Without spaces + HttpTester.Response response1 = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "Forwarded: for=192.0.2.43,for=\"[2001:db8:cafe::17]\",for=unknown\n" + + "\n")); - assertEquals("http",_results.poll()); - assertEquals("myhost",_results.poll()); - assertEquals("80",_results.poll()); - assertEquals("192.0.2.43",_results.poll()); - assertEquals("0",_results.poll()); - assertEquals("http",_results.poll()); - assertEquals("myhost",_results.poll()); - assertEquals("80",_results.poll()); - assertEquals("192.0.2.43",_results.poll()); - assertEquals("0",_results.poll()); - assertEquals("http",_results.poll()); - assertEquals("myhost",_results.poll()); - assertEquals("80",_results.poll()); - assertEquals("192.0.2.43",_results.poll()); - assertEquals("0",_results.poll()); + assertThat("status", response1.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); + assertThat("remotePort", _remotePort.get(), is(0)); + + // With spaces + HttpTester.Response response2 = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "Forwarded: for=192.0.2.43, for=\"[2001:db8:cafe::17]\", for=unknown\n" + + "\n")); + assertThat("status", response2.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); + assertThat("remotePort", _remotePort.get(), is(0)); + + // As multiple headers + HttpTester.Response response3 = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "Forwarded: for=192.0.2.43\n" + + "Forwarded: for=\"[2001:db8:cafe::17]\", for=unknown\n" + + "\n")); + + assertThat("status", response3.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); + assertThat("remotePort", _remotePort.get(), is(0)); } + /** + * RFC 7239: Section 7.4 + * + * Transition + */ @Test public void testRFC7239_Examples_7_4() throws Exception { - _connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: myhost\n"+ - "Forwarded: for=192.0.2.43, for=\"[2001:db8:cafe::17]\"\n"+ - "\n"); + // Old syntax + HttpTester.Response response1 = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "X-Forwarded-For: 192.0.2.43, 2001:db8:cafe::17\n" + + "\n")); - assertEquals("http",_results.poll()); - assertEquals("myhost",_results.poll()); - assertEquals("80",_results.poll()); - assertEquals("192.0.2.43",_results.poll()); - assertEquals("0",_results.poll()); + assertThat("status", response1.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); + assertThat("remotePort", _remotePort.get(), is(0)); + + // New syntax + HttpTester.Response response2 = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "Forwarded: for=192.0.2.43, for=\"[2001:db8:cafe::17]\"\n" + + "\n")); + + assertThat("status", response2.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); + assertThat("remotePort", _remotePort.get(), is(0)); } + /** + * RFC 7239: Section 7.5 + * + * Example Usage + */ @Test public void testRFC7239_Examples_7_5() throws Exception { - _connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: myhost\n"+ - "Forwarded: for=192.0.2.43,for=198.51.100.17;by=203.0.113.60;proto=http;host=example.com\n"+ - "\n"); + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "Forwarded: for=192.0.2.43,for=198.51.100.17;by=203.0.113.60;proto=http;host=example.com\n" + + "\n")); - assertEquals("http",_results.poll()); - assertEquals("example.com",_results.poll()); - assertEquals("80",_results.poll()); - assertEquals("192.0.2.43",_results.poll()); - assertEquals("0",_results.poll()); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("example.com")); + assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); + assertThat("remotePort", _remotePort.get(), is(0)); } @Test - public void testProto() throws Exception + public void testProto_OldSyntax() throws Exception { - String response=_connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: myhost\n"+ - "X-Forwarded-Proto: foobar\n"+ - "Forwarded: proto=https\n"+ - "\n"); - assertThat(response, Matchers.containsString("200 OK")); - assertEquals("https",_results.poll()); - assertEquals("myhost",_results.poll()); - assertEquals("443",_results.poll()); - assertEquals("0.0.0.0",_results.poll()); - assertEquals("0",_results.poll()); + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "X-Forwarded-Proto: https\n" + + "\n")); + + assertTrue(_wasSecure.get(), "wasSecure"); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("https")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(443)); + assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); + assertThat("remotePort", _remotePort.get(), is(0)); + } + + @Test + public void testRFC7239_Proto() throws Exception + { + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "Forwarded: proto=https\n" + + "\n")); + + assertTrue(_wasSecure.get(), "wasSecure"); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("https")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(443)); + assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); + assertThat("remotePort", _remotePort.get(), is(0)); } @Test public void testFor() throws Exception { - String response=_connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: myhost\n"+ - "X-Forwarded-For: 10.9.8.7,6.5.4.3\n"+ - "X-Forwarded-For: 8.9.8.7,7.5.4.3\n"+ - "\n"); - assertThat(response, Matchers.containsString("200 OK")); - assertEquals("http",_results.poll()); - assertEquals("myhost",_results.poll()); - assertEquals("80",_results.poll()); - assertEquals("10.9.8.7",_results.poll()); - assertEquals("0",_results.poll()); + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "X-Forwarded-For: 10.9.8.7,6.5.4.3\n" + + "X-Forwarded-For: 8.9.8.7,7.5.4.3\n" + + "\n")); + + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("remoteAddr", _remoteAddr.get(), is("10.9.8.7")); + assertThat("remotePort", _remotePort.get(), is(0)); } @Test public void testForIpv4WithPort() throws Exception { - String response=_connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: myhost\n"+ - "X-Forwarded-For: 10.9.8.7:1111,6.5.4.3:2222\n"+ - "\n"); - assertThat(response, Matchers.containsString("200 OK")); - assertEquals("http",_results.poll()); - assertEquals("myhost",_results.poll()); - assertEquals("80",_results.poll()); - assertEquals("10.9.8.7",_results.poll()); - assertEquals("1111",_results.poll()); + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "X-Forwarded-For: 10.9.8.7:1111,6.5.4.3:2222\n" + + "\n")); + + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("remoteAddr", _remoteAddr.get(), is("10.9.8.7")); + assertThat("remotePort", _remotePort.get(), is(1111)); } @Test public void testForIpv6WithPort() throws Exception { - String response=_connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: myhost\n"+ - "X-Forwarded-For: [2001:db8:cafe::17]:1111,6.5.4.3:2222\n"+ - "\n"); - assertThat(response, Matchers.containsString("200 OK")); - assertEquals("http",_results.poll()); - assertEquals("myhost",_results.poll()); - assertEquals("80",_results.poll()); - assertEquals("[2001:db8:cafe::17]",_results.poll()); - assertEquals("1111",_results.poll()); + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "X-Forwarded-For: [2001:db8:cafe::17]:1111,6.5.4.3:2222\n" + + "\n")); + + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("remoteAddr", _remoteAddr.get(), is("[2001:db8:cafe::17]")); + assertThat("remotePort", _remotePort.get(), is(1111)); } @Test public void testForIpv6AndPort() throws Exception { - String response=_connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: myhost\n"+ - "X-Forwarded-For: 1:2:3:4:5:6:7:8\n"+ - "X-Forwarded-Port: 2222\n"+ - "\n"); - assertThat(response, Matchers.containsString("200 OK")); - assertEquals("http",_results.poll()); - assertEquals("myhost",_results.poll()); - assertEquals("80",_results.poll()); - assertEquals("[1:2:3:4:5:6:7:8]",_results.poll()); - assertEquals("2222",_results.poll()); + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "X-Forwarded-For: 1:2:3:4:5:6:7:8\n" + + "X-Forwarded-Port: 2222\n" + + "\n")); - response=_connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: myhost\n"+ - "X-Forwarded-Port: 2222\n"+ - "X-Forwarded-For: 1:2:3:4:5:6:7:8\n"+ - "X-Forwarded-For: 7:7:7:7:7:7:7:7\n"+ - "X-Forwarded-Port: 3333\n"+ - "\n"); - assertThat(response, Matchers.containsString("200 OK")); - assertEquals("http",_results.poll()); - assertEquals("myhost",_results.poll()); - assertEquals("80",_results.poll()); - assertEquals("[1:2:3:4:5:6:7:8]",_results.poll()); - assertEquals("2222",_results.poll()); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("remoteAddr", _remoteAddr.get(), is("[1:2:3:4:5:6:7:8]")); + assertThat("remotePort", _remotePort.get(), is(2222)); + } + + @Test + public void testForIpv6AndPort_MultiField() throws Exception + { + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "X-Forwarded-Port: 2222\n" + + "X-Forwarded-For: 1:2:3:4:5:6:7:8\n" + + "X-Forwarded-For: 7:7:7:7:7:7:7:7\n" + + "X-Forwarded-Port: 3333\n" + + "\n")); + + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("remoteAddr", _remoteAddr.get(), is("[1:2:3:4:5:6:7:8]")); + assertThat("remotePort", _remotePort.get(), is(2222)); } @Test public void testLegacyProto() throws Exception { - String response=_connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: myhost\n"+ - "X-Proxied-Https: on\n"+ - "\n"); - assertThat(response, Matchers.containsString("200 OK")); - assertEquals("https",_results.poll()); - assertEquals("myhost",_results.poll()); - assertEquals("443",_results.poll()); - assertEquals("0.0.0.0",_results.poll()); - assertEquals("0",_results.poll()); - assertTrue(_wasSecure.get()); + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "X-Proxied-Https: on\n" + + "\n")); + assertTrue(_wasSecure.get(), "wasSecure"); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("https")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(443)); + assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); + assertThat("remotePort", _remotePort.get(), is(0)); } @Test public void testSslSession() throws Exception { _customizer.setSslIsSecure(false); - String response=_connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: myhost\n"+ - "Proxy-Ssl-Id: Wibble\n"+ - "\n"); - - assertThat(response, Matchers.containsString("200 OK")); - assertEquals("http",_results.poll()); - assertEquals("myhost",_results.poll()); - assertEquals("80",_results.poll()); - assertEquals("0.0.0.0",_results.poll()); - assertEquals("0",_results.poll()); - assertFalse(_wasSecure.get()); - assertEquals("Wibble",_sslSession.get()); - - _customizer.setSslIsSecure(true); - response=_connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: myhost\n"+ - "Proxy-Ssl-Id: 0123456789abcdef\n"+ - "\n"); - - assertThat(response, Matchers.containsString("200 OK")); - assertEquals("https",_results.poll()); - assertEquals("myhost",_results.poll()); - assertEquals("443",_results.poll()); - assertEquals("0.0.0.0",_results.poll()); - assertEquals("0",_results.poll()); - assertTrue(_wasSecure.get()); - assertEquals("0123456789abcdef",_sslSession.get()); + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "Proxy-Ssl-Id: Wibble\n" + + "\n")); + + assertFalse(_wasSecure.get(), "wasSecure"); + assertThat("sslSession", _sslSession.get(), is("Wibble")); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); + assertThat("remotePort", _remotePort.get(), is(0)); + + _customizer.setSslIsSecure(true); + response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "Proxy-Ssl-Id: 0123456789abcdef\n" + + "\n")); + + assertTrue(_wasSecure.get(), "wasSecure"); + assertThat("sslSession", _sslSession.get(), is("0123456789abcdef")); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("https")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(443)); + assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); + assertThat("remotePort", _remotePort.get(), is(0)); } - + @Test public void testSslCertificate() throws Exception { _customizer.setSslIsSecure(false); - String response=_connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: myhost\n"+ - "Proxy-auth-cert: Wibble\n"+ - "\n"); - - assertThat(response, Matchers.containsString("200 OK")); - assertEquals("http",_results.poll()); - assertEquals("myhost",_results.poll()); - assertEquals("80",_results.poll()); - assertEquals("0.0.0.0",_results.poll()); - assertEquals("0",_results.poll()); - assertFalse(_wasSecure.get()); - assertEquals("Wibble",_sslCertificate.get()); - - - _customizer.setSslIsSecure(true); - response=_connector.getResponse( - "GET / HTTP/1.1\n"+ - "Host: myhost\n"+ - "Proxy-auth-cert: 0123456789abcdef\n"+ - "\n"); - - assertThat(response, Matchers.containsString("200 OK")); - assertEquals("https",_results.poll()); - assertEquals("myhost",_results.poll()); - assertEquals("443",_results.poll()); - assertEquals("0.0.0.0",_results.poll()); - assertEquals("0",_results.poll()); - assertTrue(_wasSecure.get()); - assertEquals("0123456789abcdef",_sslCertificate.get()); + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "Proxy-auth-cert: Wibble\n" + + "\n")); + + assertFalse(_wasSecure.get(), "wasSecure"); + assertThat("sslCertificate", _sslCertificate.get(), is("Wibble")); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); + assertThat("remotePort", _remotePort.get(), is(0)); + + _customizer.setSslIsSecure(true); + response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "Proxy-auth-cert: 0123456789abcdef\n" + + "\n")); + + assertTrue(_wasSecure.get(), "wasSecure"); + assertThat("sslCertificate", _sslCertificate.get(), is("0123456789abcdef")); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("https")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(443)); + assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); + assertThat("remotePort", _remotePort.get(), is(0)); } - @Test - public void testMixed() throws Exception + public void testMixed_For_Port_RFC_For() throws Exception { - String response = _connector.getResponse( + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( "GET / HTTP/1.1\n" + "Host: myhost\n" + "X-Forwarded-For: 11.9.8.7:1111,8.5.4.3:2222\n" + "X-Forwarded-Port: 3333\n" + - "Forwarded: for=192.0.2.43,for=198.51.100.17;by=203.0.113.60;proto=http;host=example.com\n"+ + "Forwarded: for=192.0.2.43,for=198.51.100.17;by=203.0.113.60;proto=http;host=example.com\n" + "X-Forwarded-For: 11.9.8.7:1111,8.5.4.3:2222\n" + - "\n"); + "\n")); - assertEquals("http",_results.poll()); - assertEquals("example.com",_results.poll()); - assertEquals("80",_results.poll()); - assertEquals("192.0.2.43",_results.poll()); - assertEquals("0",_results.poll()); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("example.com")); + assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); + assertThat("remotePort", _remotePort.get(), is(0)); } - - + interface RequestTester { - boolean check(HttpServletRequest request,HttpServletResponse response) throws IOException; + boolean check(HttpServletRequest request, HttpServletResponse response) throws IOException; } private class RequestHandler extends AbstractHandler { private RequestTester _checker; - @SuppressWarnings("unused") - private String _content; @Override public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { - ((Request)request).setHandled(true); + baseRequest.setHandled(true); - if (request.getContentLength()>0 - && !MimeTypes.Type.FORM_ENCODED.asString().equals(request.getContentType()) - && !request.getContentType().startsWith("multipart/form-data")) - _content=IO.toString(request.getInputStream()); - - if (_checker!=null && _checker.check(request,response)) + if (_checker != null && _checker.check(request, response)) response.setStatus(200); else response.sendError(500); From 345750f9fdc6673fdbf01545d4b0cad074e4d694 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 14 Jun 2019 12:28:58 -0500 Subject: [PATCH 17/22] Issue #3782 - X-Forwarded-Port refers to node listening port + Updating testcases to test requestURL as well. + Adding new testcases for X-Forwarded-Port modifying only the port of an existing `Host:` header. Signed-off-by: Joakim Erdfelt --- .../server/ForwardedRequestCustomizer.java | 24 ++++-- .../ForwardedRequestCustomizerTest.java | 78 ++++++++++++++++++- 2 files changed, 93 insertions(+), 9 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java index e098a68e635..e9320300153 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java @@ -22,7 +22,6 @@ import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import java.net.InetSocketAddress; - import javax.servlet.ServletRequest; import org.eclipse.jetty.http.HostPortHttpField; @@ -448,6 +447,8 @@ public class ForwardedRequestCustomizer implements Customizer size += 128; _handles = new ArrayTrie<>(size); + _handles.put(HttpHeader.HOST.toString(), lookup.findVirtual(Forwarded.class, "handleHttpHost", type)); + if (_forwardedCipherSuiteHeader != null && !_handles.put(_forwardedCipherSuiteHeader, lookup.findVirtual(Forwarded.class, "handleCipherSuite", type))) continue; if (_forwardedSslSessionIdHeader != null && !_handles.put(_forwardedSslSessionIdHeader, lookup.findVirtual(Forwarded.class, "handleSslSessionId", type))) @@ -513,6 +514,7 @@ public class ForwardedRequestCustomizer implements Customizer String _proto; HostPort _for; HostPort _host; + HostPort _httpHost; public Forwarded(Request request, HttpConfiguration config) { @@ -523,6 +525,13 @@ public class ForwardedRequestCustomizer implements Customizer _host = _forcedHost.getHostPort(); } + public void handleHttpHost(HttpField field) + { + _httpHost = new HostPort(field.getValue()); + if (_host != null && _host instanceof XHostPort && "unknown".equals(_host.getHost())) + _host = new XHostPort(_httpHost.getHost(), _host.getPort()); + } + public void handleCipherSuite(HttpField field) { _request.setAttribute("javax.servlet.request.cipher_suite", field.getValue()); @@ -547,6 +556,8 @@ public class ForwardedRequestCustomizer implements Customizer { if (_host==null) _host = new XHostPort(getLeftMost(field.getValue())); + else if (_host instanceof XHostPort && "unknown".equals(_host.getHost())) + _host = new XHostPort(getLeftMost(field.getValue()), _host.getPort()); } public void handleServer(HttpField field) @@ -571,10 +582,13 @@ public class ForwardedRequestCustomizer implements Customizer public void handlePort(HttpField field) { - if (_for == null) - _for = new XHostPort("unknown", field.getIntValue()); - else if (_for instanceof XHostPort && _for.getPort()<=0) - _for = new XHostPort(HostPort.normalizeHost(_for.getHost()), field.getIntValue()); + if (_host == null) + { + String hostname = _httpHost != null ? _httpHost.getHost() : "unknown"; + _host = new XHostPort(hostname, field.getIntValue()); + } + else if (_host instanceof XHostPort && _host.getPort()<=0) + _host = new XHostPort(HostPort.normalizeHost(_host.getHost()), field.getIntValue()); } public void handleHttps(HttpField field) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java index 5c38e601d52..8f5ef3b0f81 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java @@ -49,6 +49,7 @@ public class ForwardedRequestCustomizerTest final AtomicReference _serverPort = new AtomicReference<>(); final AtomicReference _remoteAddr = new AtomicReference<>(); final AtomicReference _remotePort = new AtomicReference<>(); + final AtomicReference _requestURL = new AtomicReference<>(); ForwardedRequestCustomizer _customizer; @@ -77,6 +78,7 @@ public class ForwardedRequestCustomizerTest _serverPort.set(request.getServerPort()); _remoteAddr.set(request.getRemoteAddr()); _remotePort.set(request.getRemotePort()); + _requestURL.set(request.getRequestURL().toString()); return true; }; @@ -101,6 +103,7 @@ public class ForwardedRequestCustomizerTest assertThat("scheme", _scheme.get(), is("http")); assertThat("serverName", _serverName.get(), is("1.2.3.4")); assertThat("serverPort", _serverPort.get(), is(2222)); + assertThat("requestURL", _requestURL.get(), is("http://1.2.3.4:2222/")); } @Test @@ -115,6 +118,7 @@ public class ForwardedRequestCustomizerTest assertThat("scheme", _scheme.get(), is("http")); assertThat("serverName", _serverName.get(), is("[::1]")); assertThat("serverPort", _serverPort.get(), is(2222)); + assertThat("requestURL", _requestURL.get(), is("http://[::1]:2222/")); } @Test @@ -129,6 +133,7 @@ public class ForwardedRequestCustomizerTest assertThat("scheme", _scheme.get(), is("http")); assertThat("serverName", _serverName.get(), is("1.2.3.4")); assertThat("serverPort", _serverPort.get(), is(2222)); + assertThat("requestURL", _requestURL.get(), is("http://1.2.3.4:2222/")); } @Test @@ -143,6 +148,7 @@ public class ForwardedRequestCustomizerTest assertThat("scheme", _scheme.get(), is("http")); assertThat("serverName", _serverName.get(), is("[::1]")); assertThat("serverPort", _serverPort.get(), is(2222)); + assertThat("requestURL", _requestURL.get(), is("http://[::1]:2222/")); } /** @@ -168,6 +174,7 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(80)); assertThat("remoteAddr", _remoteAddr.get(), is("[2001:db8:cafe::17]")); assertThat("remotePort", _remotePort.get(), is(4711)); + assertThat("requestURL", _requestURL.get(), is("http://myhost/")); } /** @@ -192,6 +199,7 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(80)); assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("http://myhost/")); // With spaces HttpTester.Response response2 = HttpTester.parseResponse( @@ -206,6 +214,7 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(80)); assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("http://myhost/")); // As multiple headers HttpTester.Response response3 = HttpTester.parseResponse( @@ -222,6 +231,7 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(80)); assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("http://myhost/")); } /** @@ -246,6 +256,7 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(80)); assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("http://myhost/")); // New syntax HttpTester.Response response2 = HttpTester.parseResponse( @@ -261,6 +272,7 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(80)); assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("http://myhost/")); } /** @@ -284,6 +296,7 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(80)); assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("http://example.com/")); } @Test @@ -303,6 +316,7 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(443)); assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("https://myhost/")); } @Test @@ -322,6 +336,7 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(443)); assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("https://myhost/")); } @Test @@ -341,6 +356,7 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(80)); assertThat("remoteAddr", _remoteAddr.get(), is("10.9.8.7")); assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("http://myhost/")); } @Test @@ -359,6 +375,7 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(80)); assertThat("remoteAddr", _remoteAddr.get(), is("10.9.8.7")); assertThat("remotePort", _remotePort.get(), is(1111)); + assertThat("requestURL", _requestURL.get(), is("http://myhost/")); } @Test @@ -377,6 +394,7 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(80)); assertThat("remoteAddr", _remoteAddr.get(), is("[2001:db8:cafe::17]")); assertThat("remotePort", _remotePort.get(), is(1111)); + assertThat("requestURL", _requestURL.get(), is("http://myhost/")); } @Test @@ -393,9 +411,10 @@ public class ForwardedRequestCustomizerTest assertThat("status", response.getStatus(), is(200)); assertThat("scheme", _scheme.get(), is("http")); assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("serverPort", _serverPort.get(), is(2222)); assertThat("remoteAddr", _remoteAddr.get(), is("[1:2:3:4:5:6:7:8]")); - assertThat("remotePort", _remotePort.get(), is(2222)); + assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("http://myhost:2222/")); } @Test @@ -414,9 +433,10 @@ public class ForwardedRequestCustomizerTest assertThat("status", response.getStatus(), is(200)); assertThat("scheme", _scheme.get(), is("http")); assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("serverPort", _serverPort.get(), is(2222)); assertThat("remoteAddr", _remoteAddr.get(), is("[1:2:3:4:5:6:7:8]")); - assertThat("remotePort", _remotePort.get(), is(2222)); + assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("http://myhost:2222/")); } @Test @@ -435,6 +455,7 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(443)); assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("https://myhost/")); } @Test @@ -456,6 +477,7 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(80)); assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("http://myhost/")); _customizer.setSslIsSecure(true); response = HttpTester.parseResponse( @@ -473,6 +495,7 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(443)); assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("https://myhost/")); } @Test @@ -494,6 +517,7 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(80)); assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("http://myhost/")); _customizer.setSslIsSecure(true); response = HttpTester.parseResponse( @@ -511,6 +535,51 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(443)); assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("https://myhost/")); + } + + /** + * Resetting the server port via a forwarding header + */ + @Test + public void testPort_For() throws Exception + { + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "X-Forwarded-Port: 4444\n" + + "X-Forwarded-For: 192.168.1.200\n" + + "\n")); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(4444)); + assertThat("remoteAddr", _remoteAddr.get(), is("192.168.1.200")); + assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("http://myhost:4444/")); + } + + /** + * Test setting the server Port before the "Host" header has been seen. + */ + @Test + public void testPort_For_LateHost() throws Exception + { + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "X-Forwarded-Port: 4444\n" + // this order is intentional + "X-Forwarded-For: 192.168.1.200\n" + + "Host: myhost\n" + // leave this as the last header + "\n")); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(4444)); + assertThat("remoteAddr", _remoteAddr.get(), is("192.168.1.200")); + assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("http://myhost:4444/")); } @Test @@ -532,6 +601,7 @@ public class ForwardedRequestCustomizerTest assertThat("serverPort", _serverPort.get(), is(80)); assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); assertThat("remotePort", _remotePort.get(), is(0)); + assertThat("requestURL", _requestURL.get(), is("http://example.com/")); } interface RequestTester From 9cf8cf3c0d60c7e92cf0239730183168e90a663b Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 17 Jun 2019 11:35:40 +0200 Subject: [PATCH 18/22] Issue #3782 X-Forwarded-Port can be remote or local Added option for X-Forwarded-Port to be remote or local with default being local. Signed-off-by: Greg Wilkins --- .../main/config/etc/jetty-http-forwarded.xml | 1 + .../main/config/modules/http-forwarded.mod | 1 + .../server/ForwardedRequestCustomizer.java | 93 ++++++++++++------- .../ForwardedRequestCustomizerTest.java | 25 +++++ 4 files changed, 89 insertions(+), 31 deletions(-) diff --git a/jetty-server/src/main/config/etc/jetty-http-forwarded.xml b/jetty-server/src/main/config/etc/jetty-http-forwarded.xml index 648d6c6a94f..8d0047cc54d 100644 --- a/jetty-server/src/main/config/etc/jetty-http-forwarded.xml +++ b/jetty-server/src/main/config/etc/jetty-http-forwarded.xml @@ -12,6 +12,7 @@ + diff --git a/jetty-server/src/main/config/modules/http-forwarded.mod b/jetty-server/src/main/config/modules/http-forwarded.mod index f67822065a4..7e6545a5902 100644 --- a/jetty-server/src/main/config/modules/http-forwarded.mod +++ b/jetty-server/src/main/config/modules/http-forwarded.mod @@ -24,6 +24,7 @@ etc/jetty-http-forwarded.xml # jetty.httpConfig.forwardedProtoHeader=X-Forwarded-Proto # jetty.httpConfig.forwardedForHeader=X-Forwarded-For # jetty.httpConfig.forwardedPortHeader=X-Forwarded-Port +# jetty.httpConfig.forwardedPortHeaderRemote=false # jetty.httpConfig.forwardedHttpsHeader=X-Proxied-Https # jetty.httpConfig.forwardedSslSessionIdHeader=Proxy-ssl-id # jetty.httpConfig.forwardedCipherSuiteHeader=Proxy-auth-cert diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java index e9320300153..171384bd672 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java @@ -73,6 +73,7 @@ public class ForwardedRequestCustomizer implements Customizer private String _forwardedProtoHeader = HttpHeader.X_FORWARDED_PROTO.toString(); private String _forwardedForHeader = HttpHeader.X_FORWARDED_FOR.toString(); private String _forwardedPortHeader = HttpHeader.X_FORWARDED_PORT.toString(); + private boolean _forwardedPortHeaderRemote = false; private String _forwardedHttpsHeader = "X-Proxied-Https"; private String _forwardedCipherSuiteHeader = "Proxy-auth-cert"; private String _forwardedSslSessionIdHeader = "Proxy-ssl-id"; @@ -251,6 +252,16 @@ public class ForwardedRequestCustomizer implements Customizer } } + public boolean isForwardedPortHeaderRemote() + { + return _forwardedPortHeaderRemote; + } + + public void setForwardedPortHeaderRemote(boolean forwardedPortHeaderRemote) + { + _forwardedPortHeaderRemote = forwardedPortHeaderRemote; + } + /** * Get the forwardedProtoHeader. * @@ -447,8 +458,6 @@ public class ForwardedRequestCustomizer implements Customizer size += 128; _handles = new ArrayTrie<>(size); - _handles.put(HttpHeader.HOST.toString(), lookup.findVirtual(Forwarded.class, "handleHttpHost", type)); - if (_forwardedCipherSuiteHeader != null && !_handles.put(_forwardedCipherSuiteHeader, lookup.findVirtual(Forwarded.class, "handleCipherSuite", type))) continue; if (_forwardedSslSessionIdHeader != null && !_handles.put(_forwardedSslSessionIdHeader, lookup.findVirtual(Forwarded.class, "handleSslSessionId", type))) @@ -484,14 +493,22 @@ public class ForwardedRequestCustomizer implements Customizer } } - private static class XHostPort extends HostPort + private static class PossiblyPartialHostPort extends HostPort { - XHostPort(String authority) + PossiblyPartialHostPort(String authority) { super(authority); } - XHostPort(String host, int port) + protected PossiblyPartialHostPort(String host, int port) + { + super(host, port); + } + } + + private static class PortSetHostPort extends PossiblyPartialHostPort + { + PortSetHostPort(String host, int port) { super(host, port); } @@ -514,7 +531,6 @@ public class ForwardedRequestCustomizer implements Customizer String _proto; HostPort _for; HostPort _host; - HostPort _httpHost; public Forwarded(Request request, HttpConfiguration config) { @@ -525,13 +541,6 @@ public class ForwardedRequestCustomizer implements Customizer _host = _forcedHost.getHostPort(); } - public void handleHttpHost(HttpField field) - { - _httpHost = new HostPort(field.getValue()); - if (_host != null && _host instanceof XHostPort && "unknown".equals(_host.getHost())) - _host = new XHostPort(_httpHost.getHost(), _host.getPort()); - } - public void handleCipherSuite(HttpField field) { _request.setAttribute("javax.servlet.request.cipher_suite", field.getValue()); @@ -554,16 +563,24 @@ public class ForwardedRequestCustomizer implements Customizer public void handleHost(HttpField field) { - if (_host==null) - _host = new XHostPort(getLeftMost(field.getValue())); - else if (_host instanceof XHostPort && "unknown".equals(_host.getHost())) - _host = new XHostPort(getLeftMost(field.getValue()), _host.getPort()); + if (!_forwardedPortHeaderRemote && !StringUtil.isEmpty(_forwardedPortHeader)) + { + if (_host == null) + _host = new PossiblyPartialHostPort(getLeftMost(field.getValue())); + else if (_for instanceof PortSetHostPort) + _host = new HostPort(HostPort.normalizeHost(getLeftMost(field.getValue())), _host.getPort()); + } + else if (_host==null) + { + _host = new HostPort(getLeftMost(field.getValue())); + } } public void handleServer(HttpField field) { - if (_proxyAsAuthority && _host==null) - _host = new XHostPort(getLeftMost(field.getValue())); + if (_proxyAsAuthority) + return; + handleHost(field); } public void handleProto(HttpField field) @@ -574,21 +591,35 @@ public class ForwardedRequestCustomizer implements Customizer public void handleFor(HttpField field) { - if (_for==null) - _for = new XHostPort(getLeftMost(field.getValue())); - else if (_for instanceof XHostPort && "unknown".equals(_for.getHost())) - _for = new XHostPort(HostPort.normalizeHost(getLeftMost(field.getValue())),_for.getPort()); + if (_forwardedPortHeaderRemote && !StringUtil.isEmpty(_forwardedPortHeader)) + { + if (_for == null) + _for = new PossiblyPartialHostPort(getLeftMost(field.getValue())); + else if (_for instanceof PortSetHostPort) + _for = new HostPort(HostPort.normalizeHost(getLeftMost(field.getValue())), _for.getPort()); + } + else if (_for == null) + { + _for = new HostPort(getLeftMost(field.getValue())); + } } public void handlePort(HttpField field) { - if (_host == null) + if (_forwardedPortHeaderRemote) { - String hostname = _httpHost != null ? _httpHost.getHost() : "unknown"; - _host = new XHostPort(hostname, field.getIntValue()); + if (_for == null) + _for = new PortSetHostPort(_request.getRemoteHost(), field.getIntValue()); + else if (_for instanceof PossiblyPartialHostPort && _for.getPort() <= 0) + _for = new HostPort(HostPort.normalizeHost(_for.getHost()), field.getIntValue()); + } + else + { + if (_host == null) + _host = new PortSetHostPort(_request.getServerName(), field.getIntValue()); + else if (_host instanceof PossiblyPartialHostPort && _host.getPort() <= 0) + _host = new HostPort(HostPort.normalizeHost(_host.getHost()), field.getIntValue()); } - else if (_host instanceof XHostPort && _host.getPort()<=0) - _host = new XHostPort(HostPort.normalizeHost(_host.getHost()), field.getIntValue()); } public void handleHttps(HttpField field) @@ -616,19 +647,19 @@ public class ForwardedRequestCustomizer implements Customizer break; if (value.startsWith("_") || "unknown".equals(value)) break; - if (_host == null || _host instanceof XHostPort) + if (_host == null || !(_host instanceof Rfc7239HostPort)) _host = new Rfc7239HostPort(value); break; case "for": if (value.startsWith("_") || "unknown".equals(value)) break; - if (_for == null || _for instanceof XHostPort) + if (_for == null || !(_for instanceof Rfc7239HostPort)) _for = new Rfc7239HostPort(value); break; case "host": if (value.startsWith("_") || "unknown".equals(value)) break; - if (_host == null || _host instanceof XHostPort) + if (_host == null || !(_host instanceof Rfc7239HostPort)) _host = new Rfc7239HostPort(value); break; case "proto": diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java index 8f5ef3b0f81..61181da2efc 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java @@ -560,6 +560,31 @@ public class ForwardedRequestCustomizerTest assertThat("requestURL", _requestURL.get(), is("http://myhost:4444/")); } + + + /** + * Resetting the server port via a forwarding header + */ + @Test + public void testRemote_Port_For() throws Exception + { + _customizer.setForwardedPortHeaderRemote(true); + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "X-Forwarded-Port: 4444\n" + + "X-Forwarded-For: 192.168.1.200\n" + + "\n")); + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("http")); + assertThat("serverName", _serverName.get(), is("myhost")); + assertThat("serverPort", _serverPort.get(), is(80)); + assertThat("remoteAddr", _remoteAddr.get(), is("192.168.1.200")); + assertThat("remotePort", _remotePort.get(), is(4444)); + assertThat("requestURL", _requestURL.get(), is("http://myhost/")); + } + /** * Test setting the server Port before the "Host" header has been seen. */ From e7e63a5617ffca484a1b1349f8c90eb826605023 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 17 Jun 2019 14:00:30 +0200 Subject: [PATCH 19/22] Issue #3782 X-Forwarded-Port can be remote or local Improved names and javadoc after review. Signed-off-by: Greg Wilkins --- .../main/config/etc/jetty-http-forwarded.xml | 2 +- .../main/config/modules/http-forwarded.mod | 8 ++++- .../server/ForwardedRequestCustomizer.java | 29 ++++++++++++------- .../ForwardedRequestCustomizerTest.java | 2 +- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/jetty-server/src/main/config/etc/jetty-http-forwarded.xml b/jetty-server/src/main/config/etc/jetty-http-forwarded.xml index 8d0047cc54d..2c843733c2a 100644 --- a/jetty-server/src/main/config/etc/jetty-http-forwarded.xml +++ b/jetty-server/src/main/config/etc/jetty-http-forwarded.xml @@ -6,13 +6,13 @@ + - diff --git a/jetty-server/src/main/config/modules/http-forwarded.mod b/jetty-server/src/main/config/modules/http-forwarded.mod index 7e6545a5902..e6303cd992e 100644 --- a/jetty-server/src/main/config/modules/http-forwarded.mod +++ b/jetty-server/src/main/config/modules/http-forwarded.mod @@ -16,15 +16,21 @@ etc/jetty-http-forwarded.xml [ini-template] ### ForwardedRequestCustomizer Configuration +## If true, only the RFC7239 Forwarded header is accepted # jetty.httpConfig.forwardedOnly=false + +## if true, the proxy address obtained from X-Forwarded-Server or RFC7239 is used as the request authority. # jetty.httpConfig.forwardedProxyAsAuthority=false + +## if true, the X-Forwarded-Port header applies to the authority, else it applies to the remote client address +# jetty.httpConfig.forwardedPortAsAuthority=true + # jetty.httpConfig.forwardedHeader=Forwarded # jetty.httpConfig.forwardedHostHeader=X-Forwarded-Host # jetty.httpConfig.forwardedServerHeader=X-Forwarded-Server # jetty.httpConfig.forwardedProtoHeader=X-Forwarded-Proto # jetty.httpConfig.forwardedForHeader=X-Forwarded-For # jetty.httpConfig.forwardedPortHeader=X-Forwarded-Port -# jetty.httpConfig.forwardedPortHeaderRemote=false # jetty.httpConfig.forwardedHttpsHeader=X-Proxied-Https # jetty.httpConfig.forwardedSslSessionIdHeader=Proxy-ssl-id # jetty.httpConfig.forwardedCipherSuiteHeader=Proxy-auth-cert diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java index 171384bd672..141af6eaaf4 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java @@ -67,17 +67,17 @@ public class ForwardedRequestCustomizer implements Customizer private static final Logger LOG = Log.getLogger(ForwardedRequestCustomizer.class); private HostPortHttpField _forcedHost; + private boolean _proxyAsAuthority = false; + private boolean _forwardedPortAsAuthority = true; private String _forwardedHeader = HttpHeader.FORWARDED.toString(); private String _forwardedHostHeader = HttpHeader.X_FORWARDED_HOST.toString(); private String _forwardedServerHeader = HttpHeader.X_FORWARDED_SERVER.toString(); private String _forwardedProtoHeader = HttpHeader.X_FORWARDED_PROTO.toString(); private String _forwardedForHeader = HttpHeader.X_FORWARDED_FOR.toString(); private String _forwardedPortHeader = HttpHeader.X_FORWARDED_PORT.toString(); - private boolean _forwardedPortHeaderRemote = false; private String _forwardedHttpsHeader = "X-Proxied-Https"; private String _forwardedCipherSuiteHeader = "Proxy-auth-cert"; private String _forwardedSslSessionIdHeader = "Proxy-ssl-id"; - private boolean _proxyAsAuthority = false; private boolean _sslIsSecure = true; private Trie _handles; @@ -252,14 +252,23 @@ public class ForwardedRequestCustomizer implements Customizer } } - public boolean isForwardedPortHeaderRemote() + /** + * @return if true, the X-Forwarded-Port header applies to the authority, + * else it applies to the remote client address + */ + public boolean getForwardedPortAsAuthority() { - return _forwardedPortHeaderRemote; + return _forwardedPortAsAuthority; } - public void setForwardedPortHeaderRemote(boolean forwardedPortHeaderRemote) + /** + * Set if the X-Forwarded-Port header will be used for Authority + * @param forwardedPortAsAuthority if true, the X-Forwarded-Port header applies to the authority, + * else it applies to the remote client address + */ + public void setForwardedPortAsAuthority(boolean forwardedPortAsAuthority) { - _forwardedPortHeaderRemote = forwardedPortHeaderRemote; + _forwardedPortAsAuthority = forwardedPortAsAuthority; } /** @@ -563,7 +572,7 @@ public class ForwardedRequestCustomizer implements Customizer public void handleHost(HttpField field) { - if (!_forwardedPortHeaderRemote && !StringUtil.isEmpty(_forwardedPortHeader)) + if (_forwardedPortAsAuthority && !StringUtil.isEmpty(_forwardedPortHeader)) { if (_host == null) _host = new PossiblyPartialHostPort(getLeftMost(field.getValue())); @@ -591,7 +600,7 @@ public class ForwardedRequestCustomizer implements Customizer public void handleFor(HttpField field) { - if (_forwardedPortHeaderRemote && !StringUtil.isEmpty(_forwardedPortHeader)) + if (!_forwardedPortAsAuthority && !StringUtil.isEmpty(_forwardedPortHeader)) { if (_for == null) _for = new PossiblyPartialHostPort(getLeftMost(field.getValue())); @@ -606,7 +615,7 @@ public class ForwardedRequestCustomizer implements Customizer public void handlePort(HttpField field) { - if (_forwardedPortHeaderRemote) + if (!_forwardedPortAsAuthority) { if (_for == null) _for = new PortSetHostPort(_request.getRemoteHost(), field.getIntValue()); @@ -647,7 +656,7 @@ public class ForwardedRequestCustomizer implements Customizer break; if (value.startsWith("_") || "unknown".equals(value)) break; - if (_host == null || !(_host instanceof Rfc7239HostPort)) + if (_proxyAsAuthority && (_host == null || !(_host instanceof Rfc7239HostPort))) _host = new Rfc7239HostPort(value); break; case "for": diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java index 61181da2efc..c5537eaa2e0 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java @@ -568,7 +568,7 @@ public class ForwardedRequestCustomizerTest @Test public void testRemote_Port_For() throws Exception { - _customizer.setForwardedPortHeaderRemote(true); + _customizer.setForwardedPortAsAuthority(false); HttpTester.Response response = HttpTester.parseResponse( _connector.getResponse( "GET / HTTP/1.1\n" + From 275f83c1d0503b12e2b308fead5f18ded4d4e952 Mon Sep 17 00:00:00 2001 From: Lachlan Date: Tue, 18 Jun 2019 16:01:09 +1000 Subject: [PATCH 20/22] Issue #3785 - fix failures in QTP testLifeCycleStop (#3788) make sure the jobs are actually run before calling QTP.stop() Signed-off-by: Lachlan Roberts --- .../util/thread/QueuedThreadPoolTest.java | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/thread/QueuedThreadPoolTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/thread/QueuedThreadPoolTest.java index e03322d7b74..0c11f81163c 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/thread/QueuedThreadPoolTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/thread/QueuedThreadPoolTest.java @@ -48,11 +48,12 @@ public class QueuedThreadPoolTest extends AbstractThreadPoolTest private class RunningJob implements Runnable { - private final CountDownLatch _run = new CountDownLatch(1); - private final CountDownLatch _stopping = new CountDownLatch(1); - private final CountDownLatch _stopped = new CountDownLatch(1); - private final String _name; - private final boolean _fail; + final CountDownLatch _run = new CountDownLatch(1); + final CountDownLatch _stopping = new CountDownLatch(1); + final CountDownLatch _stopped = new CountDownLatch(1); + final String _name; + final boolean _fail; + RunningJob() { this(null, false); @@ -118,7 +119,7 @@ public class QueuedThreadPoolTest extends AbstractThreadPoolTest private class CloseableJob extends RunningJob implements Closeable { - private final CountDownLatch _closed = new CountDownLatch(1); + final CountDownLatch _closed = new CountDownLatch(1); @Override public void close() throws IOException @@ -382,24 +383,24 @@ public class QueuedThreadPoolTest extends AbstractThreadPoolTest // Wait until the first 2 start running waitForThreads(tp,2); waitForIdle(tp,0); + assertTrue(job0._run.await(200, TimeUnit.MILLISECONDS)); + assertTrue(job1._run.await(200, TimeUnit.MILLISECONDS)); // Queue should be empty after thread pool is stopped tp.stop(); assertThat(tp.getQueue().size(), is(0)); // First 2 jobs closed by InterruptedException - assertThat(job0._stopped.await(200, TimeUnit.MILLISECONDS), is(true)); - assertThat(job1._stopped.await(200, TimeUnit.MILLISECONDS), is(true)); + assertTrue(job0._stopped.await(200, TimeUnit.MILLISECONDS)); + assertTrue(job1._stopped.await(200, TimeUnit.MILLISECONDS)); // Verify RunningJobs in the queue have not been run - assertThat(job2._run.await(200, TimeUnit.MILLISECONDS), is(false)); - assertThat(job4._run.await(200, TimeUnit.MILLISECONDS), is(false)); + assertFalse(job2._run.await(200, TimeUnit.MILLISECONDS)); + assertFalse(job4._run.await(200, TimeUnit.MILLISECONDS)); // Verify ClosableJobs have not been run but have been closed - assertThat(job4._run.await(200, TimeUnit.MILLISECONDS), is(false)); - assertThat(job3._closed.await(200, TimeUnit.MILLISECONDS), is(true)); - - tp.stop(); + assertFalse(job3._run.await(200, TimeUnit.MILLISECONDS)); + assertTrue(job3._closed.await(200, TimeUnit.MILLISECONDS)); } From 862e6d008e99223ad048860dd69c19914732a4f7 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 18 Jun 2019 09:50:18 +0200 Subject: [PATCH 21/22] Jetty 9.4.x 3755 annotation/jndi example cleanup (#3763) * Issue #3755 Annotation example cleanup + Created JettyDistribution class as common utility to locate a jetty distribution for examples. + Fixed ServerWithAnnotations to correctly use the test-spec-webapp + Added AttributeContainerMap as a better way to treat attribute values as beans. This avoids them appearing twice in a dump and always associates them with their key. + Added NamingDump and use it in EnvConfiguration and jetty-plus.xml so that a server dump will contain dumps of the server local tree and each contexts java:comp/env tree + Improved the dump format of NamingContext and WebAppContext + Improved the toString format of several associated classes Signed-off-by: Greg Wilkins --- .../jetty/embedded/JettyDistribution.java | 109 +++++++++++ .../eclipse/jetty/embedded/LikeJettyXml.java | 20 +- .../org/eclipse/jetty/embedded/OneWebApp.java | 3 +- .../jetty/embedded/ServerWithAnnotations.java | 18 +- .../org/eclipse/jetty/jndi/NamingContext.java | 27 ++- .../jetty/jndi/local/localContextRoot.java | 4 +- .../org/eclipse/jetty/plus/jndi/EnvEntry.java | 6 + .../org/eclipse/jetty/plus/jndi/Link.java | 6 + .../eclipse/jetty/plus/jndi/NamingDump.java | 71 +++++++ .../eclipse/jetty/plus/jndi/NamingEntry.java | 32 ++-- .../jetty/plus/jndi/NamingEntryUtil.java | 4 +- .../jetty/plus/webapp/EnvConfiguration.java | 47 ++--- .../src/main/plus-config/etc/jetty-plus.xml | 4 + .../jetty/plus/jndi/TestNamingEntryUtil.java | 21 +-- .../jetty/server/LowResourceMonitor.java | 20 +- .../java/org/eclipse/jetty/server/Server.java | 20 +- .../util/component/AttributeContainerMap.java | 82 +++++++++ .../jetty/util/component/Container.java | 2 +- .../util/component/ContainerLifeCycle.java | 12 +- .../jetty/util/component/Dumpable.java | 174 ++++++++++-------- .../jetty/webapp/WebAppClassLoader.java | 2 +- .../eclipse/jetty/webapp/WebAppContext.java | 48 +++-- 22 files changed, 520 insertions(+), 212 deletions(-) create mode 100644 examples/embedded/src/main/java/org/eclipse/jetty/embedded/JettyDistribution.java create mode 100644 jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingDump.java create mode 100644 jetty-util/src/main/java/org/eclipse/jetty/util/component/AttributeContainerMap.java diff --git a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/JettyDistribution.java b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/JettyDistribution.java new file mode 100644 index 00000000000..0d3c3810606 --- /dev/null +++ b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/JettyDistribution.java @@ -0,0 +1,109 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.embedded; + +import java.io.File; +import java.nio.file.Path; + +import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; + +/** + * A utility test class to locate a Jetty Distribution for testing purposes by searching: + *
    + *
  • The jetty.home system property
    • + *
  • The JETTY_HOME environment variable
    • + *
  • The working directory hierarchy with subdirectory jetty-distribution/target/home
    • + *
+ */ +public class JettyDistribution +{ + private final static Logger LOG = Log.getLogger(JettyDistribution.class); + public final static Path DISTRIBUTION; + + static + { + Path distro = asJettyDistribution(System.getProperty("jetty.home")); + if (distro==null) + distro = asJettyDistribution(System.getenv().get("JETTY_HOME")); + + if (distro==null) + { + try + { + Path working = new File(".").getAbsoluteFile().getCanonicalFile().toPath(); + while(distro == null && working !=null ) + { + distro = asJettyDistribution(working.resolve("jetty-distribution/target/distribution").toString()); + working = working.getParent(); + } + } + catch(Throwable th) + { + LOG.warn(th); + } + } + DISTRIBUTION = distro; + } + + private static Path asJettyDistribution(String test) + { + try + { + if (StringUtil.isBlank(test)) + { + LOG.info("asJettyDistribution {} is blank", test); + return null; + } + + File dir = new File(test); + if (!dir.exists() || !dir.isDirectory()) + { + LOG.info("asJettyDistribution {} is not a directory", test); + return null; + } + + File demoBase = new File(dir,"demo-base"); + if (!demoBase.exists() || !demoBase.isDirectory()) + { + LOG.info("asJettyDistribution {} has no demo-base", test); + return null; + } + + LOG.info("asJettyDistribution {}", dir); + return dir.getAbsoluteFile().getCanonicalFile().toPath(); + } + catch(Exception e) + { + LOG.ignore(e); + } + return null; + } + + public static Path resolve(String path) + { + return DISTRIBUTION.resolve(path); + } + + public static void main(String... arg) + { + System.err.println("Jetty Distribution is " + DISTRIBUTION); + } +} diff --git a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/LikeJettyXml.java b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/LikeJettyXml.java index 3a56eff6da9..1ad9f45f664 100644 --- a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/LikeJettyXml.java +++ b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/LikeJettyXml.java @@ -19,7 +19,6 @@ package org.eclipse.jetty.embedded; import java.io.File; -import java.io.FileNotFoundException; import java.lang.management.ManagementFactory; import org.eclipse.jetty.deploy.DeploymentManager; @@ -60,27 +59,14 @@ public class LikeJettyXml public static void main( String[] args ) throws Exception { // Path to as-built jetty-distribution directory - String jettyHomeBuild = "jetty-distribution/target/distribution"; - + String jettyHomeBuild = JettyDistribution.DISTRIBUTION.toString(); + // Find jetty home and base directories String homePath = System.getProperty("jetty.home", jettyHomeBuild); - File start_jar = new File(homePath,"start.jar"); - if (!start_jar.exists()) - { - homePath = jettyHomeBuild = "jetty-distribution/target/distribution"; - start_jar = new File(homePath,"start.jar"); - if (!start_jar.exists()) - throw new FileNotFoundException(start_jar.toString()); - } - File homeDir = new File(homePath); String basePath = System.getProperty("jetty.base", homeDir + "/demo-base"); File baseDir = new File(basePath); - if(!baseDir.exists()) - { - throw new FileNotFoundException(baseDir.getAbsolutePath()); - } // Configure jetty.home and jetty.base system properties String jetty_home = homeDir.getAbsolutePath(); @@ -88,7 +74,6 @@ public class LikeJettyXml System.setProperty("jetty.home", jetty_home); System.setProperty("jetty.base", jetty_base); - // === jetty.xml === // Setup Threadpool QueuedThreadPool threadPool = new QueuedThreadPool(); @@ -219,7 +204,6 @@ public class LikeJettyXml lowResourcesMonitor.setPeriod(1000); lowResourcesMonitor.setLowResourcesIdleTimeout(200); lowResourcesMonitor.setMonitorThreads(true); - lowResourcesMonitor.setMaxConnections(0); lowResourcesMonitor.setMaxMemory(0); lowResourcesMonitor.setMaxLowResourcesTime(5000); server.addBean(lowResourcesMonitor); diff --git a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/OneWebApp.java b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/OneWebApp.java index e6457c784c0..454d0555b4c 100644 --- a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/OneWebApp.java +++ b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/OneWebApp.java @@ -49,8 +49,7 @@ public class OneWebApp // PlusConfiguration) to choosing where the webapp will unpack itself. WebAppContext webapp = new WebAppContext(); webapp.setContextPath("/"); - File warFile = new File( - "../../tests/test-jmx/jmx-webapp/target/jmx-webapp"); + File warFile = JettyDistribution.resolve("demo-base/webapps/async-rest.war").toFile(); webapp.setWar(warFile.getAbsolutePath()); // A WebAppContext is a ContextHandler as well so it needs to be set to diff --git a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/ServerWithAnnotations.java b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/ServerWithAnnotations.java index d5ed0c922ca..41b611d2543 100644 --- a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/ServerWithAnnotations.java +++ b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/ServerWithAnnotations.java @@ -21,6 +21,7 @@ package org.eclipse.jetty.embedded; import java.io.File; import org.eclipse.jetty.plus.jndi.EnvEntry; +import org.eclipse.jetty.plus.jndi.NamingDump; import org.eclipse.jetty.plus.jndi.Resource; import org.eclipse.jetty.plus.jndi.Transaction; import org.eclipse.jetty.security.HashLoginService; @@ -47,16 +48,14 @@ public class ServerWithAnnotations classlist.addBefore( "org.eclipse.jetty.webapp.JettyWebXmlConfiguration", "org.eclipse.jetty.annotations.AnnotationConfiguration"); - // Create a WebApp WebAppContext webapp = new WebAppContext(); webapp.setContextPath("/"); - File warFile = new File( - "jetty-distribution/target/distribution/demo-base/webapps/test.war"); + File warFile = JettyDistribution.resolve("demo-base/webapps/test-spec.war").toFile(); webapp.setWar(warFile.getAbsolutePath()); webapp.setAttribute( - "org.eclipse.jetty.server.webapp.ContainerIncludeJarPattern", - ".*/javax.servlet-[^/]*\\.jar$|.*/servlet-api-[^/]*\\.jar$"); + "org.eclipse.jetty.server.webapp.ContainerIncludeJarPattern", + ".*/javax.servlet-[^/]*\\.jar$|.*/servlet-api-[^/]*\\.jar$"); server.setHandler(webapp); // Register new transaction manager in JNDI @@ -64,10 +63,14 @@ public class ServerWithAnnotations new Transaction(new com.acme.MockUserTransaction()); // Define an env entry with webapp scope. - new EnvEntry(webapp, "maxAmount", new Double(100), true); + // THIS ENTRY IS OVERRIDEN BY THE ENTRY IN jetty-env.xml + new EnvEntry(webapp, "maxAmount", 100d, true); // Register a mock DataSource scoped to the webapp - new Resource(webapp, "jdbc/mydatasource", new com.acme.MockDataSource()); + new Resource(server, "jdbc/mydatasource", new com.acme.MockDataSource()); + + // Add JNDI context to server for dump + server.addBean(new NamingDump()); // Configure a LoginService HashLoginService loginService = new HashLoginService(); @@ -75,6 +78,7 @@ public class ServerWithAnnotations loginService.setConfig("examples/embedded/src/test/resources/realm.properties"); server.addBean(loginService); + server.start(); server.dumpStdErr(); server.join(); diff --git a/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/NamingContext.java b/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/NamingContext.java index 1d147752c12..d0f7271f297 100644 --- a/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/NamingContext.java +++ b/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/NamingContext.java @@ -22,8 +22,10 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.Hashtable; import java.util.List; +import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import javax.naming.Binding; @@ -126,7 +128,7 @@ public class NamingContext implements Context, Dumpable this(env, name, parent, parser, null); } - private NamingContext(Hashtable env, + protected NamingContext(Hashtable env, String name, NamingContext parent, NameParser parser, @@ -143,14 +145,13 @@ public class NamingContext implements Context, Dumpable } /** - * @return A shallow copy of the Context with the same bindings, but a copy of the Env + * @return A shallow copy of the Context with the same bindings, but with the passed environment */ - public NamingContext shallowCopy() + public Context shallowCopy(Hashtable env) { - return new NamingContext(_env, _name, _parent, _parser, _bindings); + return new NamingContext(env, _name, _parent, _parser, _bindings); } - public boolean isDeepBindingSupported() { // look for deep binding support in _env @@ -457,7 +458,7 @@ public class NamingContext implements Context, Dumpable { if(LOG.isDebugEnabled()) LOG.debug("Null or empty name, returning shallowCopy of this context"); - return shallowCopy(); + return shallowCopy(_env); } if (cname.size() == 1) @@ -541,7 +542,7 @@ public class NamingContext implements Context, Dumpable if (cname == null || name.isEmpty()) { - return shallowCopy(); + return shallowCopy(_env); } if (cname.size() == 0) @@ -1118,7 +1119,17 @@ public class NamingContext implements Context, Dumpable @Override public void dump(Appendable out,String indent) throws IOException { - Dumpable.dumpObjects(out,indent,this, _bindings); + Map bindings = new HashMap<>(); + for (Map.Entry binding : _bindings.entrySet()) + bindings.put(binding.getKey(), binding.getValue().getObject()); + + Dumpable.dumpObject(out, this); + Dumpable.dumpMapEntries(out, indent, bindings, _env.isEmpty()); + if (!_env.isEmpty()) + { + out.append(indent).append("+> environment\n"); + Dumpable.dumpMapEntries(out, indent + " ", _env, true); + } } private Collection findListeners() diff --git a/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/local/localContextRoot.java b/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/local/localContextRoot.java index f712d329be6..3ad31582062 100644 --- a/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/local/localContextRoot.java +++ b/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/local/localContextRoot.java @@ -270,7 +270,7 @@ public class localContextRoot implements Context if (cname == null || cname.isEmpty()) { //If no name create copy of this context with same bindings, but with copy of the environment so it can be modified - return __root.shallowCopy(); + return __root.shallowCopy(_env); } if (cname.size() == 0) @@ -339,7 +339,7 @@ public class localContextRoot implements Context if ((cname == null) || cname.isEmpty()) { - return __root.shallowCopy(); + return __root.shallowCopy(_env); } if (cname.size() == 1) diff --git a/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/EnvEntry.java b/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/EnvEntry.java index 8a08ae902e4..a6881343ea2 100644 --- a/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/EnvEntry.java +++ b/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/EnvEntry.java @@ -56,4 +56,10 @@ public class EnvEntry extends NamingEntry { return this.overrideWebXml; } + + @Override + protected String toStringMetaData() + { + return "OverrideWebXml=" + overrideWebXml; + } } diff --git a/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/Link.java b/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/Link.java index aeb96e096e3..c0ee8a68563 100644 --- a/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/Link.java +++ b/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/Link.java @@ -52,4 +52,10 @@ public class Link extends NamingEntry { return _link; } + + @Override + protected String toStringMetaData() + { + return _link; + } } diff --git a/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingDump.java b/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingDump.java new file mode 100644 index 00000000000..7be17835011 --- /dev/null +++ b/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingDump.java @@ -0,0 +1,71 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.plus.jndi; + +import javax.naming.InitialContext; + +import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.component.Dumpable; + +/** + * A utility Dumpable to dump a JNDI naming context tree. + */ +public class NamingDump implements Dumpable +{ + private final ClassLoader _loader; + private final String _name; + + public NamingDump() + { + this(null,""); + } + + public NamingDump(ClassLoader loader, String name) + { + _loader = loader; + _name = name; + } + + @Override + public void dump(Appendable out, String indent) + { + ClassLoader loader = Thread.currentThread().getContextClassLoader(); + try + { + if (!StringUtil.isBlank(_name)) + out.append(_name).append(" "); + if (_loader!=null) + Thread.currentThread().setContextClassLoader(_loader); + Object context = new InitialContext().lookup(_name); + if (context instanceof Dumpable) + ((Dumpable)context).dump(out, indent); + else + Dumpable.dumpObjects(out, indent, context); + } + catch(Throwable th) + { + throw new RuntimeException(th); + } + finally + { + if (_loader!=null) + Thread.currentThread().setContextClassLoader(loader); + } + } +} diff --git a/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingEntry.java b/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingEntry.java index 765064524cd..92a3ff0c216 100644 --- a/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingEntry.java +++ b/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingEntry.java @@ -49,14 +49,7 @@ public abstract class NamingEntry protected String _namingEntryNameString; //the name of the NamingEntry relative to the context it is stored in protected String _objectNameString; //the name of the object relative to the context it is stored in - - @Override - public String toString() - { - return _jndiName; - } - - + /** * Create a naming entry. * @@ -173,21 +166,21 @@ public abstract class NamingEntry /** * Save the NamingEntry for later use. *

- * Saving is done by binding the NamingEntry + * Saving is done by binding both the NamingEntry * itself, and the value it represents into * JNDI. In this way, we can link to the * value it represents later, but also * still retrieve the NamingEntry itself too. *

- * The object is bound at the jndiName passed in. - * This NamingEntry is bound at __/jndiName. + * The object is bound at scope/jndiName and + * the NamingEntry is bound at scope/__/jndiName. *

* eg * 

      * jdbc/foo    : DataSource
      * __/jdbc/foo : NamingEntry
      *
- * + * @see NamingEntryUtil#getNameForScope(Object) * @param object the object to save * @throws NamingException if unable to save */ @@ -212,5 +205,18 @@ public abstract class NamingEntry _objectNameString = objectName.toString(); NamingUtil.bind(ic, _objectNameString, object); } - + + protected String toStringMetaData() + { + return null; + } + + @Override + public String toString() + { + String metadata = toStringMetaData(); + if (metadata == null) + return String.format("%s@%x{name=%s}", this.getClass().getName(), hashCode(), getJndiName()); + return String.format("%s@%x{name=%s,%s}", this.getClass().getName(), hashCode(), getJndiName(), metadata); + } } diff --git a/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingEntryUtil.java b/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingEntryUtil.java index e005d4337b2..b6050350dfe 100644 --- a/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingEntryUtil.java +++ b/jetty-plus/src/main/java/org/eclipse/jetty/plus/jndi/NamingEntryUtil.java @@ -118,7 +118,7 @@ public class NamingEntryUtil * @return all NameEntries of a certain type in the given naming environment scope (server-wide names or context-specific names) * @throws NamingException if unable to lookup the naming entries */ - public static List lookupNamingEntries (Object scope, Class clazz) + public static List lookupNamingEntries (Object scope, Class clazz) throws NamingException { try @@ -127,7 +127,7 @@ public class NamingEntryUtil Context namingEntriesContext = (Context)scopeContext.lookup(NamingEntry.__contextName); ArrayList list = new ArrayList(); lookupNamingEntries(list, namingEntriesContext, clazz); - return list; + return (List)list; } catch (NameNotFoundException e) { diff --git a/jetty-plus/src/main/java/org/eclipse/jetty/plus/webapp/EnvConfiguration.java b/jetty-plus/src/main/java/org/eclipse/jetty/plus/webapp/EnvConfiguration.java index 869d2d605d0..6441478bc17 100644 --- a/jetty-plus/src/main/java/org/eclipse/jetty/plus/webapp/EnvConfiguration.java +++ b/jetty-plus/src/main/java/org/eclipse/jetty/plus/webapp/EnvConfiguration.java @@ -21,9 +21,7 @@ package org.eclipse.jetty.plus.webapp; import java.net.URL; import java.util.ArrayList; import java.util.Collections; -import java.util.Iterator; import java.util.List; - import javax.naming.Binding; import javax.naming.Context; import javax.naming.InitialContext; @@ -36,6 +34,7 @@ import org.eclipse.jetty.jndi.NamingContext; import org.eclipse.jetty.jndi.NamingUtil; import org.eclipse.jetty.jndi.local.localContextRoot; import org.eclipse.jetty.plus.jndi.EnvEntry; +import org.eclipse.jetty.plus.jndi.NamingDump; import org.eclipse.jetty.plus.jndi.NamingEntryUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -54,6 +53,7 @@ public class EnvConfiguration extends AbstractConfiguration private static final String JETTY_ENV_BINDINGS = "org.eclipse.jetty.jndi.EnvConfiguration"; private URL jettyEnvXmlUrl; + private NamingDump _dumper; public void setJettyEnvXml (URL url) { @@ -128,6 +128,9 @@ public class EnvConfiguration extends AbstractConfiguration //add java:comp/env entries for any EnvEntries that have been defined so far bindEnvEntries(context); + + _dumper = new NamingDump(context.getClassLoader(),"java:comp"); + context.addBean(_dumper); } @@ -138,6 +141,9 @@ public class EnvConfiguration extends AbstractConfiguration @Override public void deconfigure (WebAppContext context) throws Exception { + context.removeBean(_dumper); + _dumper = null; + //get rid of any bindings for comp/env for webapp ClassLoader oldLoader = Thread.currentThread().getContextClassLoader(); Thread.currentThread().setContextClassLoader(context.getClassLoader()); @@ -206,40 +212,23 @@ public class EnvConfiguration extends AbstractConfiguration public void bindEnvEntries (WebAppContext context) throws NamingException { - LOG.debug("Binding env entries from the jvm scope"); InitialContext ic = new InitialContext(); Context envCtx = (Context)ic.lookup("java:comp/env"); - Object scope = null; - List list = NamingEntryUtil.lookupNamingEntries(scope, EnvEntry.class); - Iterator itor = list.iterator(); - while (itor.hasNext()) - { - EnvEntry ee = (EnvEntry)itor.next(); - ee.bindToENC(ee.getJndiName()); - Name namingEntryName = NamingEntryUtil.makeNamingEntryName(null, ee); - NamingUtil.bind(envCtx, namingEntryName.toString(), ee);//also save the EnvEntry in the context so we can check it later - } + + LOG.debug("Binding env entries from the jvm scope"); + doBindings(envCtx, null); LOG.debug("Binding env entries from the server scope"); - - scope = context.getServer(); - list = NamingEntryUtil.lookupNamingEntries(scope, EnvEntry.class); - itor = list.iterator(); - while (itor.hasNext()) - { - EnvEntry ee = (EnvEntry)itor.next(); - ee.bindToENC(ee.getJndiName()); - Name namingEntryName = NamingEntryUtil.makeNamingEntryName(null, ee); - NamingUtil.bind(envCtx, namingEntryName.toString(), ee);//also save the EnvEntry in the context so we can check it later - } + doBindings(envCtx, context.getServer()); LOG.debug("Binding env entries from the context scope"); - scope = context; - list = NamingEntryUtil.lookupNamingEntries(scope, EnvEntry.class); - itor = list.iterator(); - while (itor.hasNext()) + doBindings(envCtx, context); + } + + private void doBindings(Context envCtx, Object scope) throws NamingException + { + for (EnvEntry ee : NamingEntryUtil.lookupNamingEntries(scope, EnvEntry.class)) { - EnvEntry ee = (EnvEntry)itor.next(); ee.bindToENC(ee.getJndiName()); Name namingEntryName = NamingEntryUtil.makeNamingEntryName(null, ee); NamingUtil.bind(envCtx, namingEntryName.toString(), ee);//also save the EnvEntry in the context so we can check it later diff --git a/jetty-plus/src/main/plus-config/etc/jetty-plus.xml b/jetty-plus/src/main/plus-config/etc/jetty-plus.xml index ed3082408a4..fd869e72aa4 100644 --- a/jetty-plus/src/main/plus-config/etc/jetty-plus.xml +++ b/jetty-plus/src/main/plus-config/etc/jetty-plus.xml @@ -22,5 +22,9 @@ + + + + diff --git a/jetty-plus/src/test/java/org/eclipse/jetty/plus/jndi/TestNamingEntryUtil.java b/jetty-plus/src/test/java/org/eclipse/jetty/plus/jndi/TestNamingEntryUtil.java index b861ef308a9..d4a22cabe25 100644 --- a/jetty-plus/src/test/java/org/eclipse/jetty/plus/jndi/TestNamingEntryUtil.java +++ b/jetty-plus/src/test/java/org/eclipse/jetty/plus/jndi/TestNamingEntryUtil.java @@ -18,6 +18,15 @@ package org.eclipse.jetty.plus.jndi; +import java.util.List; +import javax.naming.Context; +import javax.naming.InitialContext; +import javax.naming.Name; +import javax.naming.NameNotFoundException; +import javax.naming.NamingException; + +import org.junit.jupiter.api.Test; + import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.empty; @@ -27,16 +36,6 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.fail; -import java.util.List; - -import javax.naming.Context; -import javax.naming.InitialContext; -import javax.naming.Name; -import javax.naming.NameNotFoundException; -import javax.naming.NamingException; - -import org.junit.jupiter.api.Test; - public class TestNamingEntryUtil { public class MyNamingEntry extends NamingEntry @@ -122,7 +121,7 @@ public class TestNamingEntryUtil public void testLookupNamingEntries() throws Exception { ScopeA scope = new ScopeA(); - List list = NamingEntryUtil.lookupNamingEntries(scope, MyNamingEntry.class); + List list = NamingEntryUtil.lookupNamingEntries(scope, MyNamingEntry.class); assertThat(list, is(empty())); MyNamingEntry mne1 = new MyNamingEntry(scope, "a/b", 1); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/LowResourceMonitor.java b/jetty-server/src/main/java/org/eclipse/jetty/server/LowResourceMonitor.java index 811c547307a..65994fced8d 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/LowResourceMonitor.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/LowResourceMonitor.java @@ -18,6 +18,15 @@ package org.eclipse.jetty.server; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.Executor; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; @@ -29,15 +38,6 @@ import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler; import org.eclipse.jetty.util.thread.Scheduler; import org.eclipse.jetty.util.thread.ThreadPool; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.HashSet; -import java.util.Set; -import java.util.concurrent.Executor; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; - /** * @@ -135,7 +135,7 @@ public class LowResourceMonitor extends ContainerLifeCycle /** * @param maxConnections The maximum connections before low resources state is triggered - * @deprecated Replaced by ConnectionLimit + * @deprecated Replaced by {@link ConnectionLimit} */ @Deprecated public void setMaxConnections(int maxConnections) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java index 808882a8576..b04ee643cf6 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java @@ -28,7 +28,6 @@ import java.util.Enumeration; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Future; - import javax.servlet.ServletContext; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -47,7 +46,6 @@ import org.eclipse.jetty.server.handler.ErrorHandler; import org.eclipse.jetty.server.handler.HandlerWrapper; import org.eclipse.jetty.server.handler.StatisticsHandler; import org.eclipse.jetty.util.Attributes; -import org.eclipse.jetty.util.AttributesMap; import org.eclipse.jetty.util.Jetty; import org.eclipse.jetty.util.MultiException; import org.eclipse.jetty.util.URIUtil; @@ -55,6 +53,7 @@ import org.eclipse.jetty.util.Uptime; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.annotation.Name; +import org.eclipse.jetty.util.component.AttributeContainerMap; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -75,7 +74,7 @@ public class Server extends HandlerWrapper implements Attributes { private static final Logger LOG = Log.getLogger(Server.class); - private final AttributesMap _attributes = new AttributesMap(); + private final AttributeContainerMap _attributes = new AttributeContainerMap(); private final ThreadPool _threadPool; private final List _connectors = new CopyOnWriteArrayList<>(); private SessionIdManager _sessionIdManager; @@ -107,6 +106,7 @@ public class Server extends HandlerWrapper implements Attributes ServerConnector connector=new ServerConnector(this); connector.setPort(port); setConnectors(new Connector[]{connector}); + addBean(_attributes); } /* ------------------------------------------------------------ */ @@ -584,9 +584,6 @@ public class Server extends HandlerWrapper implements Attributes @Override public void clearAttributes() { - Enumeration names = _attributes.getAttributeNames(); - while (names.hasMoreElements()) - removeBean(_attributes.getAttribute(names.nextElement())); _attributes.clearAttributes(); } @@ -607,7 +604,7 @@ public class Server extends HandlerWrapper implements Attributes @Override public Enumeration getAttributeNames() { - return AttributesMap.getAttributeNamesCopy(_attributes); + return _attributes.getAttributeNames(); } /* ------------------------------------------------------------ */ @@ -617,9 +614,6 @@ public class Server extends HandlerWrapper implements Attributes @Override public void removeAttribute(String name) { - Object bean=_attributes.getAttribute(name); - if (bean!=null) - removeBean(bean); _attributes.removeAttribute(name); } @@ -630,9 +624,6 @@ public class Server extends HandlerWrapper implements Attributes @Override public void setAttribute(String name, Object attribute) { - // TODO this is a crude way to get attribute values managed by JMX. - Object old=_attributes.getAttribute(name); - updateBean(old,attribute); _attributes.setAttribute(name, attribute); } @@ -693,7 +684,7 @@ public class Server extends HandlerWrapper implements Attributes @Override public void dump(Appendable out,String indent) throws IOException { - dumpObjects(out,indent,new ClassLoaderDump(this.getClass().getClassLoader()),_attributes); + dumpObjects(out,indent,new ClassLoaderDump(this.getClass().getClassLoader())); } /* ------------------------------------------------------------ */ @@ -714,6 +705,5 @@ public class Server extends HandlerWrapper implements Attributes _seconds = seconds; _dateField = dateField; } - } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/AttributeContainerMap.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/AttributeContainerMap.java new file mode 100644 index 00000000000..109af9e163f --- /dev/null +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/AttributeContainerMap.java @@ -0,0 +1,82 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.util.component; + +import java.io.IOException; +import java.util.Collections; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.Map; + +import org.eclipse.jetty.util.Attributes; + +/** + * An Attributes implementation that holds it's values in an immutable {@link ContainerLifeCycle} + */ +public class AttributeContainerMap extends ContainerLifeCycle implements Attributes +{ + private final Map _map = new HashMap<>(); + + @Override + public synchronized void setAttribute(String name, Object attribute) + { + Object old = _map.put(name, attribute); + updateBean(old, attribute); + } + + @Override + public synchronized void removeAttribute(String name) + { + Object removed = _map.remove(name); + if (removed != null) + removeBean(removed); + } + + @Override + public synchronized Object getAttribute(String name) + { + return _map.get(name); + } + + @Override + public synchronized Enumeration getAttributeNames() + { + return Collections.enumeration(_map.keySet()); + } + + @Override + public synchronized void clearAttributes() + { + _map.clear(); + this.removeBeans(); + } + + @Override + public void dump(Appendable out, String indent) throws IOException + { + Dumpable.dumpObject(out, this); + Dumpable.dumpMapEntries(out, indent, _map, true); + } + + @Override + public String toString() + { + return String.format("%s@%x{size=%d}",this.getClass().getSimpleName(),hashCode(),_map.size()); + } +} diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Container.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Container.java index 7ead27df1c5..805b0814215 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Container.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Container.java @@ -132,7 +132,7 @@ public interface Container /** * @param clazz the class of the beans - * @return the list of beans of the given class from the entire managed hierarchy + * @return the list of beans of the given class from the entire Container hierarchy * @param the Bean type */ public Collection getContainedBeans(Class clazz); diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java index 9f5483eac8a..8f3bcff06dd 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java @@ -550,13 +550,17 @@ public class ContainerLifeCycle extends AbstractLifeCycle implements Container, @Override public Collection getBeans(Class clazz) { - ArrayList beans = new ArrayList<>(); + ArrayList beans = null; for (Bean b : _beans) { if (clazz.isInstance(b._bean)) + { + if (beans == null) + beans = new ArrayList<>(); beans.add(clazz.cast(b._bean)); + } } - return beans; + return beans == null ? Collections.emptyList() : beans; } @Override @@ -879,7 +883,7 @@ public class ContainerLifeCycle extends AbstractLifeCycle implements Container, /** * @param clazz the class of the beans - * @return the list of beans of the given class from the entire managed hierarchy + * @return the list of beans of the given class from the entire Container hierarchy * @param the Bean type */ @Override @@ -893,7 +897,7 @@ public class ContainerLifeCycle extends AbstractLifeCycle implements Container, /** * @param clazz the class of the beans * @param the Bean type - * @param beans the collection to add beans of the given class from the entire managed hierarchy + * @param beans the collection to add beans of the given class from the entire Container hierarchy */ protected void getContainedBeans(Class clazz, Collection beans) { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Dumpable.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Dumpable.java index 1e973f3f5c6..7908bf48adf 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Dumpable.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Dumpable.java @@ -93,18 +93,19 @@ public interface Dumpable String s; if (o==null) s = "null"; - else if (o instanceof Collection) - s = String.format("%s@%x(size=%d)",o.getClass().getName(),o.hashCode(),((Collection)o).size()); - else if (o.getClass().isArray()) - s = String.format("%s@%x[size=%d]",o.getClass().getComponentType(),o.hashCode(), Array.getLength(o)); - else if (o instanceof Map) - s = String.format("%s@%x{size=%d}",o.getClass().getName(),o.hashCode(),((Map)o).size()); else if (o instanceof Dumpable) { s = ((Dumpable)o).dumpSelf(); s = StringUtil.replace(s, "\r\n", "|"); s = StringUtil.replace(s, '\n', '|'); } + else if (o instanceof Collection) + s = String.format("%s@%x(size=%d)",o.getClass().getName(),o.hashCode(),((Collection)o).size()); + else if (o.getClass().isArray()) + s = String.format("%s@%x[size=%d]",o.getClass().getComponentType(),o.hashCode(), Array.getLength(o)); + else if (o instanceof Map) + s = String.format("%s@%x{size=%d}",o.getClass().getName(),o.hashCode(),((Map)o).size()); + else { s = String.valueOf(o); @@ -139,7 +140,7 @@ public interface Dumpable { dumpObject(out,object); - int size = extraChildren==null?0:extraChildren.length; + int extras = extraChildren==null?0:extraChildren.length; if (object instanceof Stream) object = ((Stream)object).toArray(); @@ -148,87 +149,25 @@ public interface Dumpable if (object instanceof Container) { - Container container = (Container)object; - ContainerLifeCycle containerLifeCycle = container instanceof ContainerLifeCycle ? (ContainerLifeCycle)container : null; - for (Iterator i = container.getBeans().iterator(); i.hasNext();) - { - Object bean = i.next(); - String nextIndent = indent + ((i.hasNext() || size>0) ? "| " : " "); - if (bean instanceof LifeCycle) - { - if (container.isManaged(bean)) - { - out.append(indent).append("+= "); - if (bean instanceof Dumpable) - ((Dumpable)bean).dump(out,nextIndent); - else - dumpObjects(out, nextIndent, bean); - } - else if (containerLifeCycle != null && containerLifeCycle.isAuto(bean)) - { - out.append(indent).append("+? "); - if (bean instanceof Dumpable) - ((Dumpable)bean).dump(out,nextIndent); - else - dumpObjects(out, nextIndent, bean); - } - else - { - out.append(indent).append("+~ "); - dumpObject(out, bean); - } - } - else if (containerLifeCycle != null && containerLifeCycle.isUnmanaged(bean)) - { - out.append(indent).append("+~ "); - dumpObject(out, bean); - } - else - { - out.append(indent).append("+- "); - if (bean instanceof Dumpable) - ((Dumpable)bean).dump(out,nextIndent); - else - dumpObjects(out, nextIndent, bean); - } - } + dumpContainer(out, indent, (Container)object, extras==0); } if (object instanceof Iterable) { - for (Iterator i = ((Iterable)object).iterator(); i.hasNext();) - { - Object item = i.next(); - String nextIndent = indent + ((i.hasNext() || size>0) ? "| " : " "); - out.append(indent).append("+: "); - if (item instanceof Dumpable) - ((Dumpable)item).dump(out,nextIndent); - else - dumpObjects(out,nextIndent, item); - } + dumpIterable(out, indent, (Iterable)object, extras==0); } else if (object instanceof Map) { - for (Iterator> i = ((Map)object).entrySet().iterator(); i.hasNext();) - { - Map.Entry entry = i.next(); - String nextIndent = indent + ((i.hasNext() || size>0) ? "| " : " "); - out.append(indent).append("+@ ").append(String.valueOf(entry.getKey())).append('='); - Object item = entry.getValue(); - if (item instanceof Dumpable) - ((Dumpable)item).dump(out,nextIndent); - else - dumpObjects(out,nextIndent, item); - } + dumpMapEntries(out, indent, (Map)object, extras==0); } - if (size==0) + if (extras==0) return; int i = 0; for (Object item : extraChildren) { i++; - String nextIndent = indent + (i "); if (item instanceof Dumpable) ((Dumpable)item).dump(out,nextIndent); @@ -236,4 +175,91 @@ public interface Dumpable dumpObjects(out, nextIndent, item); } } + + static void dumpContainer(Appendable out, String indent, Container object, boolean last) throws IOException + { + Container container = object; + ContainerLifeCycle containerLifeCycle = container instanceof ContainerLifeCycle ? (ContainerLifeCycle)container : null; + for (Iterator i = container.getBeans().iterator(); i.hasNext();) + { + Object bean = i.next(); + String nextIndent = indent + ((i.hasNext() || !last) ? "| " : " "); + if (bean instanceof LifeCycle) + { + if (container.isManaged(bean)) + { + out.append(indent).append("+= "); + if (bean instanceof Dumpable) + ((Dumpable)bean).dump(out,nextIndent); + else + dumpObjects(out, nextIndent, bean); + } + else if (containerLifeCycle != null && containerLifeCycle.isAuto(bean)) + { + out.append(indent).append("+? "); + if (bean instanceof Dumpable) + ((Dumpable)bean).dump(out,nextIndent); + else + dumpObjects(out, nextIndent, bean); + } + else + { + out.append(indent).append("+~ "); + dumpObject(out, bean); + } + } + else if (containerLifeCycle != null && containerLifeCycle.isUnmanaged(bean)) + { + out.append(indent).append("+~ "); + dumpObject(out, bean); + } + else + { + out.append(indent).append("+- "); + if (bean instanceof Dumpable) + ((Dumpable)bean).dump(out,nextIndent); + else + dumpObjects(out, nextIndent, bean); + } + } + } + + static void dumpIterable(Appendable out, String indent, Iterable iterable, boolean last) throws IOException + { + for (Iterator i = iterable.iterator(); i.hasNext();) + { + Object item = i.next(); + String nextIndent = indent + ((i.hasNext() || !last) ? "| " : " "); + out.append(indent).append("+: "); + if (item instanceof Dumpable) + ((Dumpable)item).dump(out,nextIndent); + else + dumpObjects(out,nextIndent, item); + } + + } + + static void dumpMapEntries(Appendable out, String indent, Map map, boolean last) throws IOException + { + for (Iterator> i = map.entrySet().iterator(); i.hasNext();) + { + Map.Entry entry = i.next(); + String nextIndent = indent + ((i.hasNext() || !last) ? "| " : " "); + out.append(indent).append("+@ ").append(String.valueOf(entry.getKey())).append(" = "); + Object item = entry.getValue(); + if (item instanceof Dumpable) + ((Dumpable)item).dump(out,nextIndent); + else + dumpObjects(out,nextIndent, item); + } + } + + static Dumpable named(String name, Object object) + { + return (out, indent) -> + { + out.append(name).append(": "); + Dumpable.dumpObjects(out, indent, object); + }; + } } diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java index a96bc416363..a6bfc75eaf8 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppClassLoader.java @@ -724,7 +724,7 @@ public class WebAppClassLoader extends URLClassLoader implements ClassVisibility @Override public String toString() { - return "WebAppClassLoader=" + _name+"@"+Long.toHexString(hashCode()); + return String.format("%s{%s}@%x", this.getClass().getSimpleName(), _name, hashCode()); } @Override diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java index b38334fd0ff..6c15f207c75 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java @@ -34,7 +34,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; - import javax.servlet.ServletContext; import javax.servlet.ServletRegistration.Dynamic; import javax.servlet.ServletSecurityElement; @@ -1040,15 +1039,10 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL public String toString() { if (_war!=null) - { - String war=_war; - if (war.indexOf("/webapps/")>=0) - war=war.substring(war.indexOf("/webapps/")+8); - return super.toString()+"{"+war+"}"; - } + return super.toString()+"{"+_war+"}"; return super.toString(); } - + /* ------------------------------------------------------------ */ @Override public void dump(Appendable out, String indent) throws IOException @@ -1066,15 +1060,39 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL server_classes=new ArrayList<>(_serverClasses); Collections.sort(server_classes); } - + + String name = getDisplayName(); + if (name == null) + { + if (_war != null) + { + if (_war.indexOf("/webapps/") >= 0) + name = _war.substring(_war.indexOf("/webapps/") + 8); + else + name = _war; + } + else if (getResourceBase() != null) + { + name = getResourceBase(); + if (name.indexOf("/webapps/") >= 0) + name = name.substring(name.indexOf("/webapps/") + 8); + } + else + { + name = this.getClass().getSimpleName(); + } + } + + name = String.format("%s@%x", name, hashCode()); + dumpObjects(out,indent, new ClassLoaderDump(getClassLoader()), - new DumpableCollection("Systemclasses "+this,system_classes), - new DumpableCollection("Serverclasses "+this,server_classes), - new DumpableCollection("Configurations "+this,_configurations), - new DumpableCollection("Handler attributes "+this,((AttributesMap)getAttributes()).getAttributeEntrySet()), - new DumpableCollection("Context attributes "+this,((Context)getServletContext()).getAttributeEntrySet()), - new DumpableCollection("Initparams "+this,getInitParams().entrySet()) + new DumpableCollection("Systemclasses " + name, system_classes), + new DumpableCollection("Serverclasses " + name, server_classes), + new DumpableCollection("Configurations " + name, _configurations), + new DumpableCollection("Handler attributes " + name, ((AttributesMap)getAttributes()).getAttributeEntrySet()), + new DumpableCollection("Context attributes " + name, ((Context)getServletContext()).getAttributeEntrySet()), + new DumpableCollection("Initparams " + name, getInitParams().entrySet()) ); } From cd38756ef5ace34610d3dcfd2fafc4a8eb87b660 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 18 Jun 2019 09:51:54 +0200 Subject: [PATCH 22/22] fixed formatting from merge Signed-off-by: Greg Wilkins --- .../src/main/java/org/eclipse/jetty/util/component/Dumpable.java | 1 - 1 file changed, 1 deletion(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Dumpable.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Dumpable.java index 7908bf48adf..f1819cbb8de 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Dumpable.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Dumpable.java @@ -105,7 +105,6 @@ public interface Dumpable s = String.format("%s@%x[size=%d]",o.getClass().getComponentType(),o.hashCode(), Array.getLength(o)); else if (o instanceof Map) s = String.format("%s@%x{size=%d}",o.getClass().getName(),o.hashCode(),((Map)o).size()); - else { s = String.valueOf(o);