Skip to content

Commit

Permalink
Only set LEGACY UriCompliance for EE8 runtime by default
Browse files Browse the repository at this point in the history
Signed-off-by: Lachlan Roberts <[email protected]>
  • Loading branch information
lachlan-roberts committed Jan 29, 2025
1 parent f079dea commit 99dd82f
Show file tree
Hide file tree
Showing 12 changed files with 84 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@

/** {@code AppEngineConstants} centralizes some constants that are specific to our use of Jetty. */
public final class AppEngineConstants {

/**
* If Legacy Mode is turned on, then Jetty is configured to be more forgiving of bad requests and
* to act more in the style of Jetty-9.3
*/
public static final boolean LEGACY_MODE =
Boolean.getBoolean("com.google.apphosting.runtime.jetty94.LEGACY_MODE");

public static final String GAE_RUNTIME = System.getenv("GAE_RUNTIME");

/**
* This {@code ServletContext} attribute contains the {@link AppVersion} for the current
* application.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.apphosting.runtime;

import static com.google.apphosting.runtime.AppEngineConstants.GAE_RUNTIME;
import static java.util.concurrent.TimeUnit.MILLISECONDS;

import com.google.appengine.api.ThreadManager;
Expand Down Expand Up @@ -240,7 +241,7 @@ private void dispatchBackgroundRequest() throws InterruptedException, TimeoutExc
String requestId = getBackgroundRequestId(upRequest);
// For java21 runtime, RPC path, do the new background thread handling for now, and keep it for
// other runtimes.
if (!Objects.equals(System.getenv("GAE_RUNTIME"), "java21")) {
if (!Objects.equals(GAE_RUNTIME, "java21")) {
// Wait here for synchronization with the ThreadFactory.
CountDownLatch latch = ThreadGroupPool.resetCurrentThread();
Thread thread = new ThreadProxy();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,28 @@
import org.eclipse.jetty.server.Server;

public interface AppVersionHandlerFactory {

enum EEVersion
{
EE8,
EE10
}

static EEVersion getEEVersion() {
if (Boolean.getBoolean("appengine.use.EE10"))
return EEVersion.EE10;
else
return EEVersion.EE8;
}

static AppVersionHandlerFactory newInstance(Server server, String serverInfo) {
if (Boolean.getBoolean("appengine.use.EE10")) {
return new EE10AppVersionHandlerFactory(server, serverInfo);
} else {
return new EE8AppVersionHandlerFactory(server, serverInfo);
switch (getEEVersion()) {
case EE10:
return new EE10AppVersionHandlerFactory(server, serverInfo);
case EE8:
return new EE8AppVersionHandlerFactory(server, serverInfo);
default:
throw new IllegalStateException("Unknown EE version: " + getEEVersion());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
*/
package com.google.apphosting.runtime.jetty;

import static com.google.apphosting.runtime.AppEngineConstants.GAE_RUNTIME;
import static com.google.apphosting.runtime.AppEngineConstants.HTTP_CONNECTOR_MODE;
import static com.google.apphosting.runtime.AppEngineConstants.IGNORE_RESPONSE_SIZE_LIMIT;
import static com.google.apphosting.runtime.AppEngineConstants.LEGACY_MODE;
import static java.nio.charset.StandardCharsets.UTF_8;

import com.google.apphosting.api.ApiProxy;
Expand Down Expand Up @@ -50,7 +52,7 @@
import org.eclipse.jetty.http.UriCompliance;
import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.SizeLimitHandler;
import org.eclipse.jetty.server.handler.SizeLimitHandler;
import org.eclipse.jetty.util.VirtualThreads;
import org.eclipse.jetty.util.thread.QueuedThreadPool;

Expand All @@ -64,13 +66,6 @@ public class JettyServletEngineAdapter implements ServletEngineAdapter {
private static final int MAX_THREAD_POOL_THREADS = 100;
private static final long MAX_RESPONSE_SIZE = 32 * 1024 * 1024;

/**
* If Legacy Mode is turned on, then Jetty is configured to be more forgiving of bad requests and
* to act more in the style of Jetty-9.3
*/
public static final boolean LEGACY_MODE =
Boolean.getBoolean("com.google.apphosting.runtime.jetty94.LEGACY_MODE");

private AppVersionKey lastAppVersionKey;

static {
Expand Down Expand Up @@ -107,7 +102,7 @@ public void start(String serverInfo, ServletEngineAdapter.Config runtimeOptions)
new QueuedThreadPool(MAX_THREAD_POOL_THREADS, MIN_THREAD_POOL_THREADS);
// Try to enable virtual threads if requested and on java21:
if (Boolean.getBoolean("appengine.use.virtualthreads")
&& "java21".equals(System.getenv("GAE_RUNTIME"))) {
&& "java21".equals(GAE_RUNTIME)) {
threadPool.setVirtualThreadsExecutor(VirtualThreads.getDefaultVirtualThreadsExecutor());
logger.atInfo().log("Configuring Appengine web server virtual threads.");
}
Expand Down Expand Up @@ -137,8 +132,14 @@ public void run(Runnable runnable) {
httpConfiguration.setSendDateHeader(false);
httpConfiguration.setSendServerVersion(false);
httpConfiguration.setSendXPoweredBy(false);
httpConfiguration.setUriCompliance(UriCompliance.LEGACY);

// If runtime is using EE8, then set URI compliance to LEGACY to behave like Jetty 9.4.
if (Objects.equals(AppVersionHandlerFactory.getEEVersion(), AppVersionHandlerFactory.EEVersion.EE8)) {
httpConfiguration.setUriCompliance(UriCompliance.LEGACY);
}

if (LEGACY_MODE) {
httpConfiguration.setUriCompliance(UriCompliance.LEGACY);
httpConfiguration.setHttpCompliance(HttpCompliance.RFC7230_LEGACY);
httpConfiguration.setRequestCookieCompliance(CookieCompliance.RFC2965);
httpConfiguration.setResponseCookieCompliance(CookieCompliance.RFC2965);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,33 +16,32 @@

package com.google.apphosting.runtime.jetty.delegate.impl;

import static com.google.apphosting.runtime.AppEngineConstants.LEGACY_MODE;

import com.google.apphosting.base.protos.HttpPb;
import com.google.apphosting.base.protos.HttpPb.ParsedHttpHeader;
import com.google.apphosting.base.protos.RuntimePb;
import com.google.apphosting.runtime.MutableUpResponse;
import com.google.apphosting.runtime.jetty.delegate.api.DelegateExchange;
import com.google.common.base.Ascii;
import com.google.protobuf.ByteString;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.io.ByteBufferAccumulator;
import org.eclipse.jetty.io.Content;
import org.eclipse.jetty.util.Attributes;
import org.eclipse.jetty.util.Callback;

import java.net.InetSocketAddress;
import java.nio.ByteBuffer;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicReference;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.io.ByteBufferAccumulator;
import org.eclipse.jetty.io.Content;
import org.eclipse.jetty.util.Attributes;
import org.eclipse.jetty.util.Callback;

public class DelegateRpcExchange implements DelegateExchange {
private static final Content.Chunk EOF = Content.Chunk.EOF;
private static final String X_GOOGLE_INTERNAL_SKIPADMINCHECK = "x-google-internal-skipadmincheck";
private static final String SKIP_ADMIN_CHECK_ATTR = "com.google.apphosting.internal.SkipAdminCheck";
static final boolean LEGACY_MODE =
Boolean.getBoolean("com.google.apphosting.runtime.jetty94.LEGACY_MODE");

private final HttpPb.HttpRequest _request;
private final AtomicReference<Content.Chunk> _content = new AtomicReference<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import com.google.apphosting.api.ApiProxy;
import com.google.apphosting.api.ApiProxy.LogRecord;
import com.google.apphosting.runtime.AppEngineConstants;
import com.google.apphosting.runtime.jetty.EE10AppEngineAuthentication;
import com.google.apphosting.utils.servlet.ee10.DeferredTaskServlet;
import com.google.apphosting.utils.servlet.ee10.JdbcMySqlConnectionCleanupFilter;
Expand Down Expand Up @@ -278,6 +279,9 @@ public boolean handle(Request request, Response response, Callback callback) thr
protected ServletHandler newServletHandler() {
ServletHandler handler = new ServletHandler();
handler.setAllowDuplicateMappings(true);
if (AppEngineConstants.LEGACY_MODE) {
handler.setDecodeAmbiguousURIs(true);
}
return handler;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,19 @@
import com.google.apphosting.base.protos.RuntimePb;
import com.google.apphosting.base.protos.RuntimePb.UPRequest;
import com.google.apphosting.base.protos.RuntimePb.UPResponse;
import com.google.apphosting.runtime.AppEngineConstants;
import com.google.apphosting.runtime.LocalRpcContext;
import com.google.apphosting.runtime.ServletEngineAdapter;
import com.google.apphosting.runtime.anyrpc.EvaluationRuntimeServerInterface;
import com.google.apphosting.runtime.jetty.AppInfoFactory;
import com.google.apphosting.runtime.jetty.JettyServletEngineAdapter;
import com.google.apphosting.runtime.jetty.AppVersionHandlerFactory;
import com.google.common.base.Ascii;
import com.google.common.base.Throwables;
import com.google.common.flogger.GoogleLogger;
import com.google.common.primitives.Ints;
import java.time.Duration;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ExecutionException;
import java.util.logging.Level;
import org.eclipse.jetty.http.CookieCompliance;
Expand Down Expand Up @@ -94,8 +96,14 @@ public static ServerConnector newConnector(

HttpConfiguration config =
connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration();
config.setUriCompliance(UriCompliance.LEGACY);
if (JettyServletEngineAdapter.LEGACY_MODE) {

// If runtime is using EE8, then set URI compliance to LEGACY to behave like Jetty 9.4.
if (Objects.equals(AppVersionHandlerFactory.getEEVersion(), AppVersionHandlerFactory.EEVersion.EE8)) {
config.setUriCompliance(UriCompliance.LEGACY);
}

if (AppEngineConstants.LEGACY_MODE) {
config.setUriCompliance(UriCompliance.LEGACY);
config.setHttpCompliance(HttpCompliance.RFC7230_LEGACY);
config.setRequestCookieCompliance(CookieCompliance.RFC2965);
config.setResponseCookieCompliance(CookieCompliance.RFC2965);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.apphosting.runtime.jetty9;

import static com.google.apphosting.runtime.AppEngineConstants.GAE_RUNTIME;
import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR;
import static java.nio.charset.StandardCharsets.UTF_8;

Expand Down Expand Up @@ -79,8 +80,7 @@ public class AppEngineWebAppContext extends WebAppContext {
private static final int MAX_RESPONSE_SIZE = 32 * 1024 * 1024;
private static final boolean APP_IS_ASYNC =
Boolean.getBoolean(RpcConnection.ASYNC_ENABLE_PPROPERTY);
private static final boolean IS_JAVA_8_RUNTIME =
Objects.equals(System.getenv("GAE_RUNTIME"), "java8");
private static final boolean IS_JAVA_8_RUNTIME = Objects.equals(GAE_RUNTIME, "java8");
private static final ImmutableSet<HolderMatcher> EMPTY_SET =
ImmutableSet.<HolderMatcher>builder().build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package com.google.apphosting.runtime.jetty9;

import static com.google.apphosting.runtime.AppEngineConstants.LEGACY_MODE;

import com.google.apphosting.base.protos.AppLogsPb;
import com.google.apphosting.base.protos.RuntimePb;
import com.google.apphosting.base.protos.RuntimePb.UPRequest;
Expand Down Expand Up @@ -93,7 +95,7 @@ public static ServerConnector newConnector(

HttpConnectionFactory factory = connector.getConnectionFactory(HttpConnectionFactory.class);
factory.setHttpCompliance(
RpcConnector.LEGACY_MODE ? HttpCompliance.RFC7230_LEGACY : HttpCompliance.RFC7230);
LEGACY_MODE ? HttpCompliance.RFC7230_LEGACY : HttpCompliance.RFC7230);

HttpConfiguration config = factory.getHttpConfiguration();
config.setRequestHeaderSize(runtimeOptions.jettyRequestHeaderSize());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.apphosting.runtime.jetty9;

import static com.google.apphosting.runtime.AppEngineConstants.GAE_RUNTIME;
import static com.google.apphosting.runtime.AppEngineConstants.HTTP_CONNECTOR_MODE;
import static com.google.apphosting.runtime.AppEngineConstants.IGNORE_RESPONSE_SIZE_LIMIT;
import static java.nio.charset.StandardCharsets.UTF_8;
Expand Down Expand Up @@ -63,7 +64,7 @@ public class JettyServletEngineAdapter implements ServletEngineAdapter {
// java.util.logging) instead of writing to System.err
// Documentation: http://www.eclipse.org/jetty/documentation/current/configuring-logging.html
System.setProperty("org.eclipse.jetty.util.log.class", JettyLogger.class.getName());
if (Objects.equals(System.getenv("GAE_RUNTIME"), "java8")) {
if (Objects.equals(GAE_RUNTIME, "java8")) {
// Remove internal URLs.
System.setProperty("java.vendor.url", "");
System.setProperty("java.vendor.url.bug", "");
Expand Down Expand Up @@ -118,7 +119,7 @@ public void start(String serverInfo, ServletEngineAdapter.Config runtimeOptions)
server.setHandler(appVersionHandlerMap);

boolean ignoreResponseSizeLimit =
Objects.equals(System.getenv("GAE_RUNTIME"), "java8")
Objects.equals(GAE_RUNTIME, "java8")
|| Boolean.getBoolean(IGNORE_RESPONSE_SIZE_LIMIT);
if (!ignoreResponseSizeLimit && !isHttpConnectorMode) {
server.insertHandler(new SizeLimitHandler(-1, MAX_RESPONSE_SIZE));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

package com.google.apphosting.runtime.jetty9;

import static com.google.apphosting.runtime.AppEngineConstants.GAE_RUNTIME;
import static com.google.apphosting.runtime.AppEngineConstants.LEGACY_MODE;

import com.google.apphosting.base.AppVersionKey;
import com.google.apphosting.base.protos.HttpPb.HttpRequest;
import com.google.apphosting.base.protos.HttpPb.ParsedHttpHeader;
Expand Down Expand Up @@ -70,7 +73,7 @@ public class RpcConnection implements Connection, HttpTransport {
Boolean.parseBoolean(
System.getProperty(
"com.google.appengine.nomalize_inet_addr",
Boolean.toString(!"java8".equals(System.getenv("GAE_RUNTIME")))));
Boolean.toString(!"java8".equals(GAE_RUNTIME))));

private final List<Listener> listeners = new CopyOnWriteArrayList<>();
private final RpcConnector connector;
Expand Down Expand Up @@ -180,7 +183,7 @@ public void onCompleted() {
// pretend to parse the request line

// LEGACY_MODE is case insensitive for known methods
HttpMethod method = RpcConnector.LEGACY_MODE
HttpMethod method = LEGACY_MODE
? HttpMethod.INSENSITIVE_CACHE.get(rpc.getProtocol())
: HttpMethod.CACHE.get(rpc.getProtocol());
String methodS = method != null ? method.asString() : rpc.getProtocol();
Expand All @@ -201,7 +204,7 @@ public void onCompleted() {
HttpField field = getField(header);

// Handle LegacyMode Headers
if (RpcConnector.LEGACY_MODE && field.getHeader() != null) {
if (LEGACY_MODE && field.getHeader() != null) {
switch (field.getHeader()) {
case CONTENT_ENCODING:
continue;
Expand Down Expand Up @@ -281,7 +284,7 @@ public void onCompleted() {
// enable it only for non java8 runtimes.
if ((exception == null)
&& (abortedError != null)
&& !"java8".equals(System.getenv("GAE_RUNTIME"))) {
&& !"java8".equals(GAE_RUNTIME)) {
exception = abortedError;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@

package com.google.apphosting.runtime.jetty9;

import static com.google.apphosting.runtime.AppEngineConstants.LEGACY_MODE;

import com.google.apphosting.base.AppVersionKey;
import com.google.apphosting.base.protos.RuntimePb.UPRequest;
import com.google.apphosting.runtime.MutableUpResponse;
import com.google.apphosting.runtime.jetty9.RpcEndPoint;
import java.io.IOException;
import javax.servlet.ServletException;
import org.eclipse.jetty.http.CookieCompliance;
Expand Down Expand Up @@ -50,14 +51,6 @@ public class RpcConnector extends AbstractConnector {

private final HttpConfiguration httpConfiguration = new HttpConfiguration();

/**
* If Legacy Mode is tunred on, then Jetty is configured to be more forgiving of bad requests
* and to act more in the style of Jetty-9.3
*/
// Keep this public property name, do not change to jetty9 as it is public contract.
static final boolean LEGACY_MODE =
Boolean.getBoolean("com.google.apphosting.runtime.jetty94.LEGACY_MODE"); // Keep 94 name.

public RpcConnector(Server server) {
super(server, null, null, null, 0, new RpcConnectionFactory());

Expand Down

0 comments on commit 99dd82f

Please sign in to comment.