Error handling no longer resets headers on response, as this caused loss of important headers, such as CORS. (#10581)

The only component that fully resets response is now Jersey feature, as we move between frameworks.

Author: tomas-langer

webserver/cors/src/test/java/io/helidon/webserver/cors/HeadersWithErrorHandlerTest.java

@@ -0,0 +1,95 @@
+/*
+ * Copyright (c) 2025 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.helidon.webserver.cors;
+
+import io.helidon.common.testing.http.junit5.HttpHeaderMatcher;
+import io.helidon.http.Header;
+import io.helidon.http.HeaderNames;
+import io.helidon.http.HeaderValues;
+import io.helidon.http.Status;
+import io.helidon.webclient.api.WebClient;
+import io.helidon.webserver.http.ErrorHandler;
+import io.helidon.webserver.http.HttpRouting;
+import io.helidon.webserver.http.HttpRules;
+import io.helidon.webserver.http.HttpService;
+import io.helidon.webserver.http.ServerRequest;
+import io.helidon.webserver.http.ServerResponse;
+import io.helidon.webserver.testing.junit5.ServerTest;
+import io.helidon.webserver.testing.junit5.SetUpRoute;
+
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+@ServerTest
+class HeadersWithErrorHandlerTest {
+    private static final Header CORS_ALL_ORIGINS = HeaderValues.create(HeaderNames.ACCESS_CONTROL_ALLOW_ORIGIN, "*");
+    private static final Header TEST_ORIGIN = HeaderValues.create(HeaderNames.ORIGIN, "http://test.origin");
+
+    private final WebClient webClient;
+
+    HeadersWithErrorHandlerTest(WebClient webClient) {
+        this.webClient = webClient;
+    }
+
+    @SetUpRoute
+    static void routing(HttpRouting.Builder routing) {
+        routing.register(CorsSupport.create(), new TestHttpService())
+                .error(RuntimeException.class, new TestErrorHandler());
+    }
+
+    @Test
+    void checkCorsOnSuccess() {
+        var response = webClient.get("/ok")
+                .header(TEST_ORIGIN)
+                .request(String.class);
+
+        assertThat(response.status(), is(Status.OK_200));
+        assertThat(response.entity(), is("hello"));
+        assertThat(response.headers(), HttpHeaderMatcher.hasHeader(CORS_ALL_ORIGINS));
+    }
+
+    @Test
+    void checkCorsOnError() {
+        var response = webClient.get("/error")
+                .header(TEST_ORIGIN)
+                .request(String.class);
+
+        assertThat(response.status(), is(Status.BAD_REQUEST_400));
+        assertThat(response.entity(), is("Failed, but error handled"));
+        assertThat(response.headers(), HttpHeaderMatcher.hasHeader(CORS_ALL_ORIGINS));
+    }
+
+    private static class TestHttpService implements HttpService {
+        @Override
+        public void routing(HttpRules rules) {
+            rules.get("/ok", (req, res) -> res.send("hello"))
+                    .get("/error", (req, res) -> {
+                        throw new RuntimeException("Unit test exception");
+                    });
+        }
+    }
+
+    private static class TestErrorHandler implements ErrorHandler<RuntimeException> {
+        @Override
+        public void handle(ServerRequest req, ServerResponse res, RuntimeException throwable) {
+            res.status(Status.BAD_REQUEST_400)
+                    .send("Failed, but error handled");
+        }
+    }
+}

webserver/http2/src/main/java/io/helidon/webserver/http2/Http2ServerResponse.java

@@ -221,6 +221,16 @@ public boolean reset() {
         return true;
     }
 
+    @Override
+    public boolean resetStream() {
+        if (isSent || outputStream != null && outputStream.bytesWritten > 0) {
+            return false;
+        }
+        streamingEntity = false;
+        outputStream = null;
+        return true;
+    }
+
     @Override
     public void commit() {
         if (outputStream != null) {

webserver/tests/http2/src/test/java/io/helidon/webserver/tests/http2/Http2ErrorHandlingWithOutputStreamTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2024 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2025 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -218,6 +218,8 @@ private static class CustomRoutingHandler implements ErrorHandler<CustomExceptio
         public void handle(ServerRequest req, ServerResponse res, CustomException throwable) {
             res.status(Status.I_AM_A_TEAPOT_418);
             res.header(ERROR_HEADER_NAME, "err");
+            // this is now the responsibility of an error handler, as otherwise we may remove CORS headers etc.
+            res.headers().remove(MAIN_HEADER_NAME);
             res.send("TeaPotIAm");
         }
     }

webserver/tests/webserver/src/test/java/io/helidon/webserver/tests/ErrorHandlingWithOutputStreamTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2025 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -157,6 +157,8 @@ private static class CustomRoutingHandler implements ErrorHandler<CustomExceptio
         public void handle(ServerRequest req, ServerResponse res, CustomException throwable) {
             res.status(Status.I_AM_A_TEAPOT_418);
             res.header(ERROR_HEADER_NAME, "z");
+            // this is now the responsibility of an error handler, as otherwise we may remove CORS headers etc.
+            res.headers().remove(MAIN_HEADER_NAME);
             res.send("CustomErrorContent");
         }
     }

webserver/webserver/src/main/java/io/helidon/webserver/http/ErrorHandlers.java

@@ -143,7 +143,8 @@ private void handleRequestException(ConnectionContext ctx,
                                         ServerRequest request,
                                         RoutingResponse response,
                                         RequestException e) {
-        if (!response.reset()) {
+        // we are only interested in resetting the streams, headers are at the discretion of the error handler
+        if (!response.resetStream()) {
             ctx.log(LOGGER, System.Logger.Level.WARNING,
                     "Request failed: %s, cannot send error response, as response already sent",
                     e, request.prologue());
@@ -210,7 +211,8 @@ private void handleError(ConnectionContext ctx,
                              RoutingResponse response,
                              Throwable e,
                              ErrorHandler<Throwable> it) {
-        if (!response.reset()) {
+        // we are only interested in resetting the streams, headers are at the discretion of the error handler
+        if (!response.resetStream()) {
             ctx.log(LOGGER, System.Logger.Level.WARNING, "Unable to reset response for error handler.");
             throw new CloseConnectionException(
                     "Cannot send response of a simple handler, status and headers already written", e);

webserver/webserver/src/main/java/io/helidon/webserver/http/RoutingResponse.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2025 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -60,7 +60,7 @@ public interface RoutingResponse extends ServerResponse {
      * Return true if the underlying response buffers and headers can be reset and a new response can be sent.
      *
      * @return {@code true} if reset was successful and a new response can be created instead of the existing one,
-     *      {@code false} if reset failed and status and headers (and maybe entity bytes) were already sent
+     *         {@code false} if reset failed and status and headers (and maybe entity bytes) were already sent
      */
     boolean reset();
 
@@ -70,4 +70,18 @@ public interface RoutingResponse extends ServerResponse {
      * After this method is called, response cannot be {@link #reset()}.
      */
     void commit();
+
+    /**
+     * Return true if the underlying response buffers can be reset and a new response can be sent.
+     * <p>
+     * As opposed to {@link #reset()}, this method is not expected to reset headers already configured on the response
+     * <p>
+     * This method calls {@link #reset()} by default.
+     *
+     * @return {@code true} if reset was successful and a new response can be created instead of the existing one,
+     *         {@code false} if reset failed and status and headers (and maybe entity bytes) were already sent
+     */
+    default boolean resetStream() {
+        return reset();
+    }
 }

webserver/webserver/src/main/java/io/helidon/webserver/http1/Http1ServerResponse.java

@@ -282,6 +282,16 @@ public boolean reset() {
         return true;
     }
 
+    @Override
+    public boolean resetStream() {
+        if (isSent || outputStream != null && outputStream.totalBytesWritten() > 0) {
+            return false;
+        }
+        streamingEntity = false;
+        outputStream = null;
+        return true;
+    }
+
     @Override
     public void commit() {
         if (outputStream != null) {

webserver/webserver/src/test/java/io/helidon/webserver/http/ErrorHandlersTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2024 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2025 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -128,7 +128,7 @@ public void testCloseConnectionExceptionContainsCause() {
         ConnectionContext ctx = mock(ConnectionContext.class);
         RoutingRequest req = mock(RoutingRequest.class);
         RoutingResponse res = mock(RoutingResponse.class);
-        when(res.reset()).thenReturn(false);
+        when(res.resetStream()).thenReturn(false);
         ErrorHandlers handlers = ErrorHandlers.create(Map.of(OtherException.class,
                 (request, response, t) -> res.send(t.getMessage())));
         try {
@@ -145,7 +145,7 @@ private void testNoHandler(ErrorHandlers handlers, Exception e, String message)
         ConnectionContext ctx = mock(ConnectionContext.class);
         RoutingRequest req = mock(RoutingRequest.class);
         RoutingResponse res = mock(RoutingResponse.class);
-        when(res.reset()).thenReturn(true);
+        when(res.resetStream()).thenReturn(true);
 
         when(req.prologue()).thenReturn(HttpPrologue.create("http/1.0",
                                                             "http",
@@ -177,7 +177,7 @@ private void testHandler(ErrorHandlers handlers, Exception e, String message) {
         RoutingRequest req = mock(RoutingRequest.class);
         RoutingResponse res = mock(RoutingResponse.class);
         when(res.isSent()).thenReturn(true);
-        when(res.reset()).thenReturn(true);
+        when(res.resetStream()).thenReturn(true);
 
         handlers.runWithErrorHandling(ctx, req, res, () -> {
             throw e;