From bfb073272e1668c63c27711e9af66651780fe921 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 13 May 2024 14:30:43 +1000 Subject: [PATCH 1/7] Use AppEngineWebAppContext startWebapp for EE8 Signed-off-by: Lachlan Roberts --- .../jetty/ee10/AppEngineWebAppContext.java | 1 - .../jetty/ee8/AppEngineWebAppContext.java | 97 +++++++++---------- 2 files changed, 46 insertions(+), 52 deletions(-) diff --git a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee10/AppEngineWebAppContext.java b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee10/AppEngineWebAppContext.java index 8af7e2b31..633f12918 100644 --- a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee10/AppEngineWebAppContext.java +++ b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee10/AppEngineWebAppContext.java @@ -241,7 +241,6 @@ protected void startWebapp() throws Exception { // - Removed deprecated filters and servlets // - Ensure known runtime filters/servlets are instantiated from this classloader // - Ensure known runtime mappings exist. - ServletHandler servletHandler = getServletHandler(); TrimmedFilters trimmedFilters = new TrimmedFilters( diff --git a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java index 3441e2e52..13d3b5433 100644 --- a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java +++ b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java @@ -238,64 +238,59 @@ public void doStart() throws Exception { addEventListener(new TransactionCleanupListener(getClassLoader())); } + @Override - protected void startContext() throws Exception { - ServletHandler servletHandler = getServletHandler(); - getServletHandler().addListener(new ListenerHolder() { - @Override - public void doStart() throws Exception { + protected void startWebapp() throws Exception { // This Listener doStart is called after the web.xml metadata has been resolved, so we can // clean configuration here: // - Removed deprecated filters and servlets // - Ensure known runtime filters/servlets are instantiated from this classloader // - Ensure known runtime mappings exist. - setListener(new EventListener() {}); - - TrimmedFilters trimmedFilters = - new TrimmedFilters( - servletHandler.getFilters(), - servletHandler.getFilterMappings(), - DEPRECATED_SERVLETS_FILTERS); - trimmedFilters.ensure( - "CloudSqlConnectionCleanupFilter", JdbcMySqlConnectionCleanupFilter.class, "/*"); - - TrimmedServlets trimmedServlets = - new TrimmedServlets( - servletHandler.getServlets(), - servletHandler.getServletMappings(), - DEPRECATED_SERVLETS_FILTERS); - trimmedServlets.ensure("_ah_warmup", WarmupServlet.class, "/_ah/warmup"); - trimmedServlets.ensure( - "_ah_sessioncleanup", SessionCleanupServlet.class, "/_ah/sessioncleanup"); - trimmedServlets.ensure( - "_ah_queue_deferred", DeferredTaskServlet.class, "/_ah/queue/__deferred__"); - trimmedServlets.ensure("_ah_snapshot", SnapshotServlet.class, "/_ah/snapshot"); - trimmedServlets.ensure("_ah_default", ResourceFileServlet.class, "/"); - trimmedServlets.ensure("default", NamedDefaultServlet.class); - trimmedServlets.ensure("jsp", NamedJspServlet.class); - - trimmedServlets.instantiateJettyServlets(); - trimmedFilters.instantiateJettyFilters(); - instantiateJettyListeners(); - - servletHandler.setFilters(trimmedFilters.getHolders()); - servletHandler.setFilterMappings(trimmedFilters.getMappings()); - servletHandler.setServlets(trimmedServlets.getHolders()); - servletHandler.setServletMappings(trimmedServlets.getMappings()); - servletHandler.setAllowDuplicateMappings(true); - - // Protect deferred task queue with constraint - ConstraintSecurityHandler security = getChildHandlerByClass(ConstraintSecurityHandler.class); - ConstraintMapping cm = new ConstraintMapping(); - cm.setConstraint(new ServletConstraint("deferred_queue", "admin")); - cm.setPathSpec("/_ah/queue/__deferred__"); - security.addConstraintMapping(cm); - } - }); - - // continue starting the webapp - super.startContext(); + ServletHandler servletHandler = getServletHandler(); + TrimmedFilters trimmedFilters = + new TrimmedFilters( + servletHandler.getFilters(), + servletHandler.getFilterMappings(), + DEPRECATED_SERVLETS_FILTERS); + trimmedFilters.ensure( + "CloudSqlConnectionCleanupFilter", JdbcMySqlConnectionCleanupFilter.class, "/*"); + + TrimmedServlets trimmedServlets = + new TrimmedServlets( + servletHandler.getServlets(), + servletHandler.getServletMappings(), + DEPRECATED_SERVLETS_FILTERS); + trimmedServlets.ensure("_ah_warmup", WarmupServlet.class, "/_ah/warmup"); + trimmedServlets.ensure( + "_ah_sessioncleanup", SessionCleanupServlet.class, "/_ah/sessioncleanup"); + trimmedServlets.ensure( + "_ah_queue_deferred", DeferredTaskServlet.class, "/_ah/queue/__deferred__"); + trimmedServlets.ensure("_ah_snapshot", SnapshotServlet.class, "/_ah/snapshot"); + trimmedServlets.ensure("_ah_default", ResourceFileServlet.class, "/"); + trimmedServlets.ensure("default", NamedDefaultServlet.class); + trimmedServlets.ensure("jsp", NamedJspServlet.class); + + trimmedServlets.instantiateJettyServlets(); + trimmedFilters.instantiateJettyFilters(); + instantiateJettyListeners(); + + servletHandler.setFilters(trimmedFilters.getHolders()); + servletHandler.setFilterMappings(trimmedFilters.getMappings()); + servletHandler.setServlets(trimmedServlets.getHolders()); + servletHandler.setServletMappings(trimmedServlets.getMappings()); + servletHandler.setAllowDuplicateMappings(true); + + // Protect deferred task queue with constraint + ConstraintSecurityHandler security = getChildHandlerByClass(ConstraintSecurityHandler.class); + ConstraintMapping cm = new ConstraintMapping(); + cm.setConstraint(new ServletConstraint("deferred_queue", "admin")); + cm.setPathSpec("/_ah/queue/__deferred__"); + security.addConstraintMapping(cm); + + + // continue starting the webapp + super.startWebapp(); } @Override From d0a6d378c0ef294e0e6caf2bd8684d76379960b4 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 13 May 2024 14:31:55 +1000 Subject: [PATCH 2/7] Fix redirect loop issue in the LocalResourceFileServlet Signed-off-by: Lachlan Roberts --- .../jetty/ee10/LocalResourceFileServlet.java | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/runtime/local_jetty12_ee10/src/main/java/com/google/appengine/tools/development/jetty/ee10/LocalResourceFileServlet.java b/runtime/local_jetty12_ee10/src/main/java/com/google/appengine/tools/development/jetty/ee10/LocalResourceFileServlet.java index eb25388bf..3d2650c0f 100644 --- a/runtime/local_jetty12_ee10/src/main/java/com/google/appengine/tools/development/jetty/ee10/LocalResourceFileServlet.java +++ b/runtime/local_jetty12_ee10/src/main/java/com/google/appengine/tools/development/jetty/ee10/LocalResourceFileServlet.java @@ -24,18 +24,19 @@ import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import java.io.IOException; -import java.net.MalformedURLException; -import java.util.Objects; -import java.util.logging.Level; -import java.util.logging.Logger; import org.eclipse.jetty.ee10.servlet.ServletContextHandler; import org.eclipse.jetty.ee10.servlet.ServletHandler; -import org.eclipse.jetty.http.pathmap.MatchedResource; +import org.eclipse.jetty.ee10.servlet.ServletMapping; import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceFactory; +import java.io.IOException; +import java.net.MalformedURLException; +import java.util.Objects; +import java.util.logging.Level; +import java.util.logging.Logger; + /** * {@code ResourceFileServlet} is a copy of {@code org.mortbay.jetty.servlet.DefaultServlet} that * has been trimmed down to only support the subset of features that we want to take advantage of @@ -57,6 +58,7 @@ public class LocalResourceFileServlet extends HttpServlet { private Resource resourceBase; private String[] welcomeFiles; private String resourceRoot; + private String defaultServletName; /** * Initialize the servlet by extracting some useful configuration @@ -73,6 +75,12 @@ public void init() throws ServletException { ServletContextHandler.getServletContextHandler(servletContext); welcomeFiles = contextHandler.getWelcomeFiles(); + ServletMapping servletMapping = contextHandler.getServletHandler().getServletMapping("/"); + if (servletMapping == null) { + throw new ServletException("No servlet mapping found"); + } + defaultServletName = servletMapping.getServletName(); + AppEngineWebXml appEngineWebXml = (AppEngineWebXml) servletContext.getAttribute("com.google.appengine.tools.development.appEngineWebXml"); @@ -254,16 +262,15 @@ private boolean maybeServeWelcomeFile( ServletContext context = getServletContext(); ServletContextHandler contextHandler = ServletContextHandler.getServletContextHandler(context); ServletHandler handler = contextHandler.getServletHandler(); - MatchedResource defaultEntry = handler.getMatchedServlet("/"); - MatchedResource jspEntry = handler.getMatchedServlet("/foo.jsp"); + ServletHandler.MappedServlet jspEntry = handler.getMappedServlet("/foo.jsp"); // Search for dynamic welcome files. for (String welcomeName : welcomeFiles) { String welcomePath = path + welcomeName; String relativePath = welcomePath.substring(1); - MatchedResource entry = handler.getMatchedServlet(welcomePath); - if (!Objects.equals(entry, defaultEntry) && !Objects.equals(entry, jspEntry)) { + ServletHandler.MappedServlet mappedServlet = handler.getMappedServlet(welcomePath); + if (!Objects.equals(mappedServlet.getServletHolder().getName(), defaultServletName) && !Objects.equals(mappedServlet, jspEntry)) { // It's a path mapped to a servlet. Forward to it. RequestDispatcher dispatcher = request.getRequestDispatcher(path + welcomeName); return staticFileUtils.serveWelcomeFileAsForward(dispatcher, included, request, response); @@ -271,7 +278,7 @@ private boolean maybeServeWelcomeFile( Resource welcomeFile = getResource(path + welcomeName); if (welcomeFile != null && welcomeFile.exists()) { - if (!Objects.equals(entry, defaultEntry)) { + if (!Objects.equals(mappedServlet.getServletHolder().getName(), defaultServletName)) { RequestDispatcher dispatcher = request.getRequestDispatcher(path + welcomeName); return staticFileUtils.serveWelcomeFileAsForward(dispatcher, included, request, response); } From 15fc90fab56f37a3fdfe71f19bb58f2c4fb9c563 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 13 May 2024 23:40:13 +1000 Subject: [PATCH 3/7] add the jetty-servlet jar to getJetty12JspJars to allow DefaultServlet with DevAppServer Signed-off-by: Lachlan Roberts --- .../appengine/tools/info/Jetty12Sdk.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/api_dev/src/main/java/com/google/appengine/tools/info/Jetty12Sdk.java b/api_dev/src/main/java/com/google/appengine/tools/info/Jetty12Sdk.java index b40bd8ab9..b1676d91d 100644 --- a/api_dev/src/main/java/com/google/appengine/tools/info/Jetty12Sdk.java +++ b/api_dev/src/main/java/com/google/appengine/tools/info/Jetty12Sdk.java @@ -17,6 +17,7 @@ package com.google.appengine.tools.info; import com.google.common.base.Joiner; + import java.io.File; import java.io.FileFilter; import java.net.URL; @@ -191,6 +192,24 @@ public List getSharedJspLibs() { return Collections.unmodifiableList(toURLs(getSharedJspLibFiles())); } + private File getJetty12Jar(String fileNamePattern) { + File path = new File(sdkRoot, JETTY12_HOME_LIB_PATH + File.separator); + + if (!path.exists()) { + throw new IllegalArgumentException("Unable to find " + path.getAbsolutePath()); + } + for (File f : listFiles(path)) { + if (f.getName().endsWith(".jar")) { + // All but CDI jar. All the tests are still passing without CDI that should not be exposed + // in our runtime (private Jetty dependency we do not want to expose to the customer). + if (f.getName().contains(fileNamePattern)) { + return f; + } + } + } + throw new IllegalArgumentException("Unable to find " + fileNamePattern + " at " + path.getAbsolutePath()); + } + private List getJetty12Jars(String subDir) { File path = new File(sdkRoot, JETTY12_HOME_LIB_PATH + File.separator + subDir); @@ -219,10 +238,12 @@ List getJetty12JspJars() { if (Boolean.getBoolean("appengine.use.EE10")) { List lf = getJetty12Jars("ee10-apache-jsp"); lf.addAll(getJetty12Jars("ee10-glassfish-jstl")); + lf.add(getJetty12Jar("ee10-servlet-")); return lf; } List lf = getJetty12Jars("ee8-apache-jsp"); lf.addAll(getJetty12Jars("ee8-glassfish-jstl")); + lf.add(getJetty12Jar("ee8-servlet-")); return lf; } From 3ab43113b56774af7d0aac31de608249b99cf6fc Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 13 May 2024 23:40:59 +1000 Subject: [PATCH 4/7] Fix NPE from the ResponseRewriterFilter because of null errorMessage Signed-off-by: Lachlan Roberts --- .../development/ResponseRewriterFilter.java | 30 +++++----- .../ee10/ResponseRewriterFilter.java | 4 +- .../jetty/proxy/UPRequestTranslator.java | 58 ++++++++++--------- 3 files changed, 49 insertions(+), 43 deletions(-) diff --git a/api_dev/src/main/java/com/google/appengine/tools/development/ResponseRewriterFilter.java b/api_dev/src/main/java/com/google/appengine/tools/development/ResponseRewriterFilter.java index a2efa07bc..12b3c838e 100644 --- a/api_dev/src/main/java/com/google/appengine/tools/development/ResponseRewriterFilter.java +++ b/api_dev/src/main/java/com/google/appengine/tools/development/ResponseRewriterFilter.java @@ -21,6 +21,21 @@ import com.google.common.base.Preconditions; import com.google.common.html.HtmlEscapers; import com.google.common.net.HttpHeaders; +import org.eclipse.jetty.util.StringUtil; + +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletOutputStream; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.WriteListener; +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletRequestWrapper; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpServletResponseWrapper; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; @@ -35,19 +50,6 @@ import java.util.NoSuchElementException; import java.util.TimeZone; import java.util.Vector; -import javax.servlet.Filter; -import javax.servlet.FilterChain; -import javax.servlet.FilterConfig; -import javax.servlet.ServletException; -import javax.servlet.ServletOutputStream; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.WriteListener; -import javax.servlet.http.Cookie; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletRequestWrapper; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpServletResponseWrapper; /** * A filter that rewrites the response headers and body from the user's @@ -728,7 +730,7 @@ public void sendError(int sc, String msg) throws IOException { checkNotCommitted(); // This has to be re-implemented to avoid committing the response. setStatus(sc, msg); - setErrorBody(sc + " " + HtmlEscapers.htmlEscaper().escape(msg)); + setErrorBody(sc + " " + (StringUtil.isEmpty(msg) ? "" : HtmlEscapers.htmlEscaper().escape(msg))); } /** Sets the response body to an HTML page with an error message. diff --git a/api_dev/src/main/java/com/google/appengine/tools/development/ee10/ResponseRewriterFilter.java b/api_dev/src/main/java/com/google/appengine/tools/development/ee10/ResponseRewriterFilter.java index 9d08dda8c..1689b2349 100644 --- a/api_dev/src/main/java/com/google/appengine/tools/development/ee10/ResponseRewriterFilter.java +++ b/api_dev/src/main/java/com/google/appengine/tools/development/ee10/ResponseRewriterFilter.java @@ -35,6 +35,8 @@ import jakarta.servlet.http.HttpServletRequestWrapper; import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.http.HttpServletResponseWrapper; +import org.eclipse.jetty.util.StringUtil; + import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; @@ -729,7 +731,7 @@ public void sendError(int sc, String msg) throws IOException { checkNotCommitted(); // This has to be re-implemented to avoid committing the response. super.sendError(sc, msg); - setErrorBody(sc + " " + HtmlEscapers.htmlEscaper().escape(msg)); + setErrorBody(sc + " " + (StringUtil.isEmpty(msg) ? "" : HtmlEscapers.htmlEscaper().escape(msg))); } /** Sets the response body to an HTML page with an error message. diff --git a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/proxy/UPRequestTranslator.java b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/proxy/UPRequestTranslator.java index 2ba466e81..1721132b8 100644 --- a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/proxy/UPRequestTranslator.java +++ b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/proxy/UPRequestTranslator.java @@ -16,6 +16,34 @@ package com.google.apphosting.runtime.jetty.proxy; +import com.google.apphosting.base.protos.AppinfoPb; +import com.google.apphosting.base.protos.HttpPb; +import com.google.apphosting.base.protos.HttpPb.HttpRequest; +import com.google.apphosting.base.protos.HttpPb.ParsedHttpHeader; +import com.google.apphosting.base.protos.RuntimePb; +import com.google.apphosting.base.protos.RuntimePb.UPRequest; +import com.google.apphosting.base.protos.TracePb.TraceContextProto; +import com.google.apphosting.runtime.TraceContextHelper; +import com.google.apphosting.runtime.jetty.AppInfoFactory; +import com.google.common.base.Ascii; +import com.google.common.base.Strings; +import com.google.common.flogger.GoogleLogger; +import com.google.common.html.HtmlEscapers; +import com.google.protobuf.ByteString; +import com.google.protobuf.TextFormat; +import org.eclipse.jetty.http.HttpField; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.HttpURI; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.util.Callback; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.io.PrintWriter; + import static com.google.apphosting.runtime.jetty.AppEngineConstants.DEFAULT_SECRET_KEY; import static com.google.apphosting.runtime.jetty.AppEngineConstants.IS_ADMIN_HEADER_VALUE; import static com.google.apphosting.runtime.jetty.AppEngineConstants.IS_TRUSTED; @@ -47,33 +75,6 @@ import static com.google.apphosting.runtime.jetty.AppEngineConstants.X_GOOGLE_INTERNAL_SKIPADMINCHECK; import static com.google.apphosting.runtime.jetty.AppEngineConstants.X_GOOGLE_INTERNAL_SKIPADMINCHECK_UC; -import com.google.apphosting.base.protos.AppinfoPb; -import com.google.apphosting.base.protos.HttpPb; -import com.google.apphosting.base.protos.HttpPb.HttpRequest; -import com.google.apphosting.base.protos.HttpPb.ParsedHttpHeader; -import com.google.apphosting.base.protos.RuntimePb; -import com.google.apphosting.base.protos.RuntimePb.UPRequest; -import com.google.apphosting.base.protos.TracePb.TraceContextProto; -import com.google.apphosting.runtime.TraceContextHelper; -import com.google.apphosting.runtime.jetty.AppInfoFactory; -import com.google.common.base.Ascii; -import com.google.common.base.Strings; -import com.google.common.flogger.GoogleLogger; -import com.google.common.html.HtmlEscapers; -import com.google.protobuf.ByteString; -import com.google.protobuf.TextFormat; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.io.PrintWriter; -import org.eclipse.jetty.http.HttpField; -import org.eclipse.jetty.http.HttpStatus; -import org.eclipse.jetty.http.HttpURI; -import org.eclipse.jetty.io.Content; -import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.Response; -import org.eclipse.jetty.util.Callback; - /** Translates HttpServletRequest to the UPRequest proto, and vice versa for the response. */ public class UPRequestTranslator { private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); @@ -366,7 +367,8 @@ public static void populateErrorResponse(Response resp, String errMsg, Callback try (OutputStream outstr = Content.Sink.asOutputStream(resp)) { PrintWriter writer = new PrintWriter(outstr); writer.print("Server Error"); - writer.print("" + HtmlEscapers.htmlEscaper().escape(errMsg) + ""); + String escapedMessage = (errMsg == null) ? "" : HtmlEscapers.htmlEscaper().escape(errMsg); + writer.print("" + escapedMessage + ""); writer.close(); callback.succeeded(); } catch (Throwable t) { From 46505a097f3e5c4393d68c11d9794b41847fb851 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 14 May 2024 11:19:12 +1000 Subject: [PATCH 5/7] avoid committing response twice in ResponseRewriterFilter Signed-off-by: Lachlan Roberts --- .../tools/development/ee10/ResponseRewriterFilter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api_dev/src/main/java/com/google/appengine/tools/development/ee10/ResponseRewriterFilter.java b/api_dev/src/main/java/com/google/appengine/tools/development/ee10/ResponseRewriterFilter.java index 1689b2349..2a0c4d565 100644 --- a/api_dev/src/main/java/com/google/appengine/tools/development/ee10/ResponseRewriterFilter.java +++ b/api_dev/src/main/java/com/google/appengine/tools/development/ee10/ResponseRewriterFilter.java @@ -730,7 +730,7 @@ public void sendError(int sc) throws IOException { public void sendError(int sc, String msg) throws IOException { checkNotCommitted(); // This has to be re-implemented to avoid committing the response. - super.sendError(sc, msg); + setStatus(sc); setErrorBody(sc + " " + (StringUtil.isEmpty(msg) ? "" : HtmlEscapers.htmlEscaper().escape(msg))); } From cb51f94bf08ebd32c1f52e6413046e8d55345282 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 14 May 2024 12:11:50 +1000 Subject: [PATCH 6/7] restore import ordering Signed-off-by: Lachlan Roberts --- .../development/ResponseRewriterFilter.java | 26 ++++----- .../jetty/ee10/LocalResourceFileServlet.java | 11 ++-- .../jetty/ee8/AppEngineWebAppContext.java | 12 ++-- .../jetty/proxy/UPRequestTranslator.java | 55 +++++++++---------- 4 files changed, 50 insertions(+), 54 deletions(-) diff --git a/api_dev/src/main/java/com/google/appengine/tools/development/ResponseRewriterFilter.java b/api_dev/src/main/java/com/google/appengine/tools/development/ResponseRewriterFilter.java index 12b3c838e..f54739b22 100644 --- a/api_dev/src/main/java/com/google/appengine/tools/development/ResponseRewriterFilter.java +++ b/api_dev/src/main/java/com/google/appengine/tools/development/ResponseRewriterFilter.java @@ -23,19 +23,6 @@ import com.google.common.net.HttpHeaders; import org.eclipse.jetty.util.StringUtil; -import javax.servlet.Filter; -import javax.servlet.FilterChain; -import javax.servlet.FilterConfig; -import javax.servlet.ServletException; -import javax.servlet.ServletOutputStream; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.WriteListener; -import javax.servlet.http.Cookie; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletRequestWrapper; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpServletResponseWrapper; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; @@ -50,6 +37,19 @@ import java.util.NoSuchElementException; import java.util.TimeZone; import java.util.Vector; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletOutputStream; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.WriteListener; +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletRequestWrapper; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpServletResponseWrapper; /** * A filter that rewrites the response headers and body from the user's diff --git a/runtime/local_jetty12_ee10/src/main/java/com/google/appengine/tools/development/jetty/ee10/LocalResourceFileServlet.java b/runtime/local_jetty12_ee10/src/main/java/com/google/appengine/tools/development/jetty/ee10/LocalResourceFileServlet.java index 3d2650c0f..36f1e1ce3 100644 --- a/runtime/local_jetty12_ee10/src/main/java/com/google/appengine/tools/development/jetty/ee10/LocalResourceFileServlet.java +++ b/runtime/local_jetty12_ee10/src/main/java/com/google/appengine/tools/development/jetty/ee10/LocalResourceFileServlet.java @@ -24,6 +24,11 @@ import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.net.MalformedURLException; +import java.util.Objects; +import java.util.logging.Level; +import java.util.logging.Logger; import org.eclipse.jetty.ee10.servlet.ServletContextHandler; import org.eclipse.jetty.ee10.servlet.ServletHandler; import org.eclipse.jetty.ee10.servlet.ServletMapping; @@ -31,12 +36,6 @@ import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceFactory; -import java.io.IOException; -import java.net.MalformedURLException; -import java.util.Objects; -import java.util.logging.Level; -import java.util.logging.Logger; - /** * {@code ResourceFileServlet} is a copy of {@code org.mortbay.jetty.servlet.DefaultServlet} that * has been trimmed down to only support the subset of features that we want to take advantage of diff --git a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java index 13d3b5433..4785de300 100644 --- a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java +++ b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/ee8/AppEngineWebAppContext.java @@ -238,15 +238,13 @@ public void doStart() throws Exception { addEventListener(new TransactionCleanupListener(getClassLoader())); } - @Override protected void startWebapp() throws Exception { - - // This Listener doStart is called after the web.xml metadata has been resolved, so we can - // clean configuration here: - // - Removed deprecated filters and servlets - // - Ensure known runtime filters/servlets are instantiated from this classloader - // - Ensure known runtime mappings exist. + // This Listener doStart is called after the web.xml metadata has been resolved, so we can + // clean configuration here: + // - Removed deprecated filters and servlets + // - Ensure known runtime filters/servlets are instantiated from this classloader + // - Ensure known runtime mappings exist. ServletHandler servletHandler = getServletHandler(); TrimmedFilters trimmedFilters = new TrimmedFilters( diff --git a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/proxy/UPRequestTranslator.java b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/proxy/UPRequestTranslator.java index 1721132b8..ee2d5d66e 100644 --- a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/proxy/UPRequestTranslator.java +++ b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/proxy/UPRequestTranslator.java @@ -16,34 +16,6 @@ package com.google.apphosting.runtime.jetty.proxy; -import com.google.apphosting.base.protos.AppinfoPb; -import com.google.apphosting.base.protos.HttpPb; -import com.google.apphosting.base.protos.HttpPb.HttpRequest; -import com.google.apphosting.base.protos.HttpPb.ParsedHttpHeader; -import com.google.apphosting.base.protos.RuntimePb; -import com.google.apphosting.base.protos.RuntimePb.UPRequest; -import com.google.apphosting.base.protos.TracePb.TraceContextProto; -import com.google.apphosting.runtime.TraceContextHelper; -import com.google.apphosting.runtime.jetty.AppInfoFactory; -import com.google.common.base.Ascii; -import com.google.common.base.Strings; -import com.google.common.flogger.GoogleLogger; -import com.google.common.html.HtmlEscapers; -import com.google.protobuf.ByteString; -import com.google.protobuf.TextFormat; -import org.eclipse.jetty.http.HttpField; -import org.eclipse.jetty.http.HttpStatus; -import org.eclipse.jetty.http.HttpURI; -import org.eclipse.jetty.io.Content; -import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.Response; -import org.eclipse.jetty.util.Callback; - -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.io.PrintWriter; - import static com.google.apphosting.runtime.jetty.AppEngineConstants.DEFAULT_SECRET_KEY; import static com.google.apphosting.runtime.jetty.AppEngineConstants.IS_ADMIN_HEADER_VALUE; import static com.google.apphosting.runtime.jetty.AppEngineConstants.IS_TRUSTED; @@ -75,6 +47,33 @@ import static com.google.apphosting.runtime.jetty.AppEngineConstants.X_GOOGLE_INTERNAL_SKIPADMINCHECK; import static com.google.apphosting.runtime.jetty.AppEngineConstants.X_GOOGLE_INTERNAL_SKIPADMINCHECK_UC; +import com.google.apphosting.base.protos.AppinfoPb; +import com.google.apphosting.base.protos.HttpPb; +import com.google.apphosting.base.protos.HttpPb.HttpRequest; +import com.google.apphosting.base.protos.HttpPb.ParsedHttpHeader; +import com.google.apphosting.base.protos.RuntimePb; +import com.google.apphosting.base.protos.RuntimePb.UPRequest; +import com.google.apphosting.base.protos.TracePb.TraceContextProto; +import com.google.apphosting.runtime.TraceContextHelper; +import com.google.apphosting.runtime.jetty.AppInfoFactory; +import com.google.common.base.Ascii; +import com.google.common.base.Strings; +import com.google.common.flogger.GoogleLogger; +import com.google.common.html.HtmlEscapers; +import com.google.protobuf.ByteString; +import com.google.protobuf.TextFormat; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.io.PrintWriter; +import org.eclipse.jetty.http.HttpField; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.HttpURI; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.util.Callback; + /** Translates HttpServletRequest to the UPRequest proto, and vice versa for the response. */ public class UPRequestTranslator { private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); From f48c889441ce2e0821f500ebabf058a4aecde6f4 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 14 May 2024 14:51:25 +1000 Subject: [PATCH 7/7] remove usage of StringUtil Signed-off-by: Lachlan Roberts --- .../appengine/tools/development/ResponseRewriterFilter.java | 3 +-- .../tools/development/ee10/ResponseRewriterFilter.java | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/api_dev/src/main/java/com/google/appengine/tools/development/ResponseRewriterFilter.java b/api_dev/src/main/java/com/google/appengine/tools/development/ResponseRewriterFilter.java index f54739b22..a58b7d2d6 100644 --- a/api_dev/src/main/java/com/google/appengine/tools/development/ResponseRewriterFilter.java +++ b/api_dev/src/main/java/com/google/appengine/tools/development/ResponseRewriterFilter.java @@ -21,7 +21,6 @@ import com.google.common.base.Preconditions; import com.google.common.html.HtmlEscapers; import com.google.common.net.HttpHeaders; -import org.eclipse.jetty.util.StringUtil; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -730,7 +729,7 @@ public void sendError(int sc, String msg) throws IOException { checkNotCommitted(); // This has to be re-implemented to avoid committing the response. setStatus(sc, msg); - setErrorBody(sc + " " + (StringUtil.isEmpty(msg) ? "" : HtmlEscapers.htmlEscaper().escape(msg))); + setErrorBody(sc + " " + (msg == null ? "" : HtmlEscapers.htmlEscaper().escape(msg))); } /** Sets the response body to an HTML page with an error message. diff --git a/api_dev/src/main/java/com/google/appengine/tools/development/ee10/ResponseRewriterFilter.java b/api_dev/src/main/java/com/google/appengine/tools/development/ee10/ResponseRewriterFilter.java index 2a0c4d565..26fdb03a4 100644 --- a/api_dev/src/main/java/com/google/appengine/tools/development/ee10/ResponseRewriterFilter.java +++ b/api_dev/src/main/java/com/google/appengine/tools/development/ee10/ResponseRewriterFilter.java @@ -35,7 +35,6 @@ import jakarta.servlet.http.HttpServletRequestWrapper; import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.http.HttpServletResponseWrapper; -import org.eclipse.jetty.util.StringUtil; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -731,7 +730,7 @@ public void sendError(int sc, String msg) throws IOException { checkNotCommitted(); // This has to be re-implemented to avoid committing the response. setStatus(sc); - setErrorBody(sc + " " + (StringUtil.isEmpty(msg) ? "" : HtmlEscapers.htmlEscaper().escape(msg))); + setErrorBody(sc + " " + (msg == null ? "" : HtmlEscapers.htmlEscaper().escape(msg))); } /** Sets the response body to an HTML page with an error message.