Implement Jetty request event handler

server/src/main/java/com/defold/extender/RequestErrorAdvice.java

@@ -0,0 +1,87 @@
+package com.defold.extender;
+
+import java.util.Collections;
+import java.util.IdentityHashMap;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
+import org.springframework.http.ResponseEntity;
+import org.springframework.web.bind.annotation.ExceptionHandler;
+import org.springframework.web.bind.annotation.RestControllerAdvice;
+import org.springframework.web.multipart.MaxUploadSizeExceededException;
+import org.springframework.web.multipart.MultipartException;
+
+/**
+ * Handles uploads that fail while Jetty parses the multipart body. Parsing happens in
+ * DispatcherServlet.checkMultipart(), i.e. before the request is mapped to a controller method, so
+ * the @ExceptionHandler methods inside {@link ExtenderController} are never consulted for these
+ * failures - the request would otherwise end up in Spring's default /error handling, which
+ * answers with a bare "400 Bad Request" and logs nothing (see defold/extender#590).
+ */
+@RestControllerAdvice
+public class RequestErrorAdvice {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(RequestErrorAdvice.class);
+
+    // Matches Jetty's message for the part limit: Form with too many keys [1043 > 1000]
+    private static final Pattern TOO_MANY_PARTS_RE = Pattern.compile("too many keys \\[(\\d+) > (\\d+)]");
+
+    private final String maxFileSize;
+    private final String maxRequestSize;
+
+    public RequestErrorAdvice(@Value("${spring.servlet.multipart.max-file-size}") String maxFileSize,
+                              @Value("${spring.servlet.multipart.max-request-size}") String maxRequestSize) {
+        this.maxFileSize = maxFileSize;
+        this.maxRequestSize = maxRequestSize;
+    }
+
+    @ExceptionHandler(MultipartException.class)
+    public ResponseEntity<String> handleMultipartException(MultipartException ex) {
+        final Matcher tooManyParts = findTooManyParts(ex);
+        if (tooManyParts != null) {
+            final String message = String.format(
+                    "The build request contains too many files (%s, the limit is %s). Reduce the number of files in the project extensions, or update the editor to a version that uploads the sources as a single archive.",
+                    tooManyParts.group(1), tooManyParts.group(2));
+            LOGGER.warn(message);
+            return textResponse(HttpStatus.PAYLOAD_TOO_LARGE, message);
+        }
+
+        if (ex instanceof MaxUploadSizeExceededException) {
+            // Spring raises this for both limits without saying which one was hit.
+            final String message = String.format("The build request is too large. Max allowed size is %s per file and %s per request.",
+                    maxFileSize, maxRequestSize);
+            LOGGER.warn(message);
+            return textResponse(HttpStatus.PAYLOAD_TOO_LARGE, message);
+        }
+
+        final String message = "The build request is not a valid multipart request and could not be read by the server.";
+        LOGGER.warn(message, ex);
+        return textResponse(HttpStatus.BAD_REQUEST, message);
+    }
+
+    private static Matcher findTooManyParts(Throwable ex) {
+        final Set<Throwable> seen = Collections.newSetFromMap(new IdentityHashMap<>());
+        for (Throwable cause = ex; cause != null && seen.add(cause); cause = cause.getCause()) {
+            if (cause.getMessage() != null) {
+                final Matcher matcher = TOO_MANY_PARTS_RE.matcher(cause.getMessage());
+                if (matcher.find()) {
+                    return matcher;
+                }
+            }
+        }
+        return null;
+    }
+
+    private static ResponseEntity<String> textResponse(HttpStatus status, String message) {
+        HttpHeaders headers = new HttpHeaders();
+        headers.setContentType(MediaType.TEXT_PLAIN);
+        return new ResponseEntity<>(message, headers, status);
+    }
+}

server/src/main/java/com/defold/extender/jetty/ExtenderJettyErrorHandler.java

@@ -0,0 +1,83 @@
+package com.defold.extender.jetty;
+
+import java.io.IOException;
+
+import org.eclipse.jetty.http.HttpException;
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.HttpStatus;
+import org.eclipse.jetty.io.Content;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Response;
+import org.eclipse.jetty.server.handler.ErrorHandler;
+import org.eclipse.jetty.util.Callback;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.defold.extender.log.Markers;
+
+/**
+ * Error handler installed on the Jetty {@link org.eclipse.jetty.server.Server}, i.e. the one used
+ * for requests that fail before they enter the servlet context - malformed HTTP, too large headers,
+ * too long URI, unsupported HTTP version. Those never reach Spring, so neither the controller
+ * exception handlers nor {@link com.defold.extender.RequestErrorAdvice} can see them; without this
+ * handler they are answered with an HTML page and are not logged at all. Errors raised inside the
+ * context still go through Spring Boot's own context error handler.
+ */
+public class ExtenderJettyErrorHandler extends ErrorHandler {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExtenderJettyErrorHandler.class);
+
+    private static final String MIME_TYPE = "text/plain";
+
+    private static final String LOG_MESSAGE = "Jetty rejected request {} {} from {} with status {}: {}";
+
+    public ExtenderJettyErrorHandler() {
+        setShowStacks(false);
+        setShowCauses(false);
+        setShowMessageInTitle(false);
+        setDefaultResponseMimeType(MIME_TYPE);
+    }
+
+    @Override
+    public boolean handle(Request request, Response response, Callback callback) throws Exception {
+        final String message = (String)request.getAttribute(ERROR_MESSAGE);
+        final Throwable cause = (Throwable)request.getAttribute(ERROR_EXCEPTION);
+        final int code = (cause instanceof HttpException httpException) ? httpException.getCode() : response.getStatus();
+
+        final Object[] details = { request.getMethod(), request.getHttpURI(), Request.getRemoteAddr(request), code, message, cause };
+        if (HttpStatus.isServerError(code)) {
+            LOGGER.error(Markers.SERVER_ERROR, LOG_MESSAGE, details);
+        } else {
+            LOGGER.warn(LOG_MESSAGE, details);
+        }
+
+        return super.handle(request, response, callback);
+    }
+
+    @Override
+    protected void generateResponse(Request request, Response response, int code, String message, Throwable cause, Callback callback) throws IOException {
+        response.getHeaders().put(HttpHeader.CONTENT_TYPE, MIME_TYPE + ";charset=utf-8");
+        Content.Sink.write(response, true, describe(code), callback);
+    }
+
+    /** The body the user gets to see. What exactly Jetty disliked stays in the log. */
+    static String describe(int code) {
+        final String explanation = switch (code) {
+            case HttpStatus.BAD_REQUEST_400 ->
+                "The server could not parse the request, so the build was never started. This usually means the request was truncated or malformed on the way to the server.";
+            case HttpStatus.REQUEST_TIMEOUT_408 ->
+                "The server timed out while waiting for the request to be sent.";
+            case HttpStatus.PAYLOAD_TOO_LARGE_413 ->
+                "The build request is too large.";
+            case HttpStatus.URI_TOO_LONG_414 ->
+                "The request URI is too long.";
+            case HttpStatus.REQUEST_HEADER_FIELDS_TOO_LARGE_431 ->
+                "The request headers are too large.";
+            case HttpStatus.HTTP_VERSION_NOT_SUPPORTED_505 ->
+                "The HTTP version used by the client is not supported by the server.";
+            default ->
+                "The request was rejected by the server before the build was started.";
+        };
+        return String.format("%d %s%n%n%s%n", code, HttpStatus.getMessage(code), explanation);
+    }
+}

server/src/main/java/com/defold/extender/jetty/JettyRequestEventsHandler.java

@@ -0,0 +1,48 @@
+package com.defold.extender.jetty;
+
+import java.util.Set;
+
+import org.eclipse.jetty.http.HttpStatus;
+import org.eclipse.jetty.server.Handler;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Response;
+import org.eclipse.jetty.util.Callback;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Logs requests that fail on the Jetty side. This complements {@link ExtenderJettyErrorHandler}:
+ * the error handler only runs when Jetty generates an error response, while this handler also sees
+ * failures that never produce one - a client that aborts mid upload, an idle timeout, or a failure
+ * after the response was already committed. All of those are client side problems, hence the
+ * warnings without com.defold.extender.log.Markers.SERVER_ERROR.
+ */
+public class JettyRequestEventsHandler extends Handler.Wrapper {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(JettyRequestEventsHandler.class);
+
+    // Already logged by ExtenderController's @ExceptionHandler methods, with more context than is
+    // available here, and for two of them with a SERVER_ERROR alert attached.
+    private static final Set<Integer> STATUSES_LOGGED_BY_CONTROLLER = Set.of(
+            HttpStatus.UNPROCESSABLE_ENTITY_422,   // handleExtenderException
+            HttpStatus.INTERNAL_SERVER_ERROR_500,  // handleException
+            HttpStatus.NOT_IMPLEMENTED_501);       // handleUsupportedExceptions
+
+    @Override
+    public boolean handle(Request request, Response response, Callback callback) throws Exception {
+        // Not EventsHandler, which offers the same event only by wrapping the request and the
+        // response, adding an allocation to every chunk read and written.
+        Request.addCompletionListener(request, failure -> onComplete(request, response.getStatus(), failure));
+        return super.handle(request, response, callback);
+    }
+
+    private static void onComplete(Request request, int status, Throwable failure) {
+        if (failure != null) {
+            LOGGER.warn("Request {} {} from {} failed with status {}",
+                    request.getMethod(), request.getHttpURI(), Request.getRemoteAddr(request), status, failure);
+        } else if (status >= 400 && !STATUSES_LOGGED_BY_CONTROLLER.contains(status)) {
+            LOGGER.warn("Request {} {} from {} completed with status {}",
+                    request.getMethod(), request.getHttpURI(), Request.getRemoteAddr(request), status);
+        }
+    }
+}

server/src/main/java/com/defold/extender/jetty/JettyServerConfiguration.java

@@ -0,0 +1,24 @@
+package com.defold.extender.jetty;
+
+import org.eclipse.jetty.server.Handler;
+import org.springframework.boot.jetty.ConfigurableJettyWebServerFactory;
+import org.springframework.boot.web.server.WebServerFactoryCustomizer;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+@Configuration(proxyBeanMethods = false)
+public class JettyServerConfiguration {
+
+    @Bean
+    WebServerFactoryCustomizer<ConfigurableJettyWebServerFactory> extenderJettyCustomizer() {
+        return factory -> factory.addServerCustomizers(server -> {
+            // Must be the server level handler: the context one is Spring Boot's /error dispatch.
+            server.setErrorHandler(new ExtenderJettyErrorHandler());
+
+            final JettyRequestEventsHandler eventsHandler = new JettyRequestEventsHandler();
+            final Handler currentHandler = server.getHandler();
+            eventsHandler.setHandler(currentHandler);
+            server.setHandler(eventsHandler);
+        });
+    }
+}

server/src/main/resources/application.yml

@@ -1,7 +1,11 @@
 server:
     port: 9000
+    max-http-request-header-size: 16KB
     jetty:
         connection-idle-timeout: 600000
+        # Jetty reuses maxFormKeys as the multipart part limit, i.e. the max number of files
+        # in a build request.
+        max-form-keys: 5000
 
 extender:
     sdk: