Adding cors headers directly to the job

dpacheco-groupieticket · rhernandez35 · commit 9fdd7eb845c3 · 2025-06-11T17:42:46.000-07:00
diff --git a/server/server-core/build.gradle.kts b/server/server-core/build.gradle.kts
@@ -16,5 +16,4 @@ dependencies {
     implementation(libs.smithy.model)
     implementation(project(":io"))
     implementation(project(":logging"))
-    implementation(libs.netty.all)
 }
diff --git a/server/server-core/src/main/java/software/amazon/smithy/java/server/core/CorsHeaders.java b/server/server-core/src/main/java/software/amazon/smithy/java/server/core/CorsHeaders.java
@@ -7,7 +7,6 @@
 
 import static software.amazon.smithy.java.core.schema.TraitKey.CORS_TRAIT;
 
-import io.netty.handler.codec.http.HttpHeaders;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
@@ -16,15 +15,15 @@ public final class CorsHeaders {
 
     private CorsHeaders() {}
 
-    private static final Map<String, Iterable<?>> BASE_CORS_HEADERS = Map.of(
+    private static final Map<String, List<String>> BASE_CORS_HEADERS = Map.of(
             "Access-Control-Allow-Methods",
             List.of("GET, POST, PUT, DELETE, OPTIONS"),
             "Access-Control-Allow-Headers",
             List.of("*,Access-Control-Allow-Headers,Access-Control-Allow-Methods,Access-Control-Allow-Origin,Amz-Sdk-Invocation-Id,Amz-Sdk-Request,Authorization,Content-Length,Content-Type,X-Amz-User-Agent,X-Amzn-Trace-Id"),
             "Access-Control-Max-Age",
             List.of("600"));
 
-    public static void of(HttpJob job, HttpHeaders headers) {
+    public static void addCorsHeaders(HttpJob job) {
         if (!shouldAddCorsHeaders(job)) {
             return;
         }
@@ -36,11 +35,8 @@ public static void of(HttpJob job, HttpHeaders headers) {
             return;
         }
 
-        for (Map.Entry<String, Iterable<?>> entrySet : BASE_CORS_HEADERS.entrySet()) {
-            headers.set(entrySet.getKey(), entrySet.getValue());
-        }
-
-        headers.set("Access-Control-Allow-Origin", List.of(requestOrigin));
+        job.response().headers().putHeaders(BASE_CORS_HEADERS);
+        job.response().headers().putHeader("Access-Control-Allow-Origin", List.of(requestOrigin));
     }
 
     private static boolean shouldAddCorsHeaders(HttpJob job) {
diff --git a/server/server-core/src/test/java/software/amazon/smithy/java/server/core/CorsHeadersTest.java b/server/server-core/src/test/java/software/amazon/smithy/java/server/core/CorsHeadersTest.java
@@ -8,7 +8,6 @@
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import io.netty.handler.codec.http.DefaultHttpHeaders;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.List;
@@ -19,6 +18,7 @@
 import software.amazon.smithy.java.core.schema.ApiService;
 import software.amazon.smithy.java.core.schema.Schema;
 import software.amazon.smithy.java.http.api.HttpHeaders;
+import software.amazon.smithy.java.http.api.ModifiableHttpHeaders;
 import software.amazon.smithy.java.server.Operation;
 import software.amazon.smithy.model.traits.CorsTrait;
 
@@ -99,29 +99,28 @@ public Schema schema() {
         HttpJob job = new HttpJob(testOperation, protocol, request, response);
 
         // Apply CORS headers
-        var responseHeaders = new DefaultHttpHeaders();
-        CorsHeaders.of(job, responseHeaders);
+        CorsHeaders.addCorsHeaders(job);
 
         // Verify headers
         if (testCase.shouldHaveHeaders) {
-            assertContainsHeader(responseHeaders, testCase.expectedAllowOrigin);
+            assertContainsHeader(job.response().headers(), testCase.expectedAllowOrigin);
         } else {
-            assertDoNotContainsHeader(responseHeaders);
+            assertDoNotContainsHeader(job.response().headers());
         }
     }
 
-    private void assertContainsHeader(io.netty.handler.codec.http.HttpHeaders responseHeaders, String allowedOrigin) {
-        assertTrue(responseHeaders.contains("Access-Control-Allow-Methods"));
-        assertTrue(responseHeaders.contains("Access-Control-Allow-Headers"));
-        assertTrue(responseHeaders.contains("Access-Control-Max-Age"));
-        assertTrue(responseHeaders.contains("Access-Control-Allow-Origin"));
-        assertTrue(responseHeaders.get("Access-Control-Allow-Origin").contains(allowedOrigin));
+    private void assertContainsHeader(ModifiableHttpHeaders responseHeaders, String allowedOrigin) {
+        assertTrue(responseHeaders.hasHeader("Access-Control-Allow-Methods"));
+        assertTrue(responseHeaders.hasHeader("Access-Control-Allow-Headers"));
+        assertTrue(responseHeaders.hasHeader("Access-Control-Max-Age"));
+        assertTrue(responseHeaders.hasHeader("Access-Control-Allow-Origin"));
+        assertTrue(responseHeaders.allValues("Access-Control-Allow-Origin").contains(allowedOrigin));
     }
 
-    private void assertDoNotContainsHeader(io.netty.handler.codec.http.HttpHeaders responseHeaders) {
-        assertFalse(responseHeaders.contains("Access-Control-Allow-Methods"));
-        assertFalse(responseHeaders.contains("Access-Control-Allow-Headers"));
-        assertFalse(responseHeaders.contains("Access-Control-Max-Age"));
-        assertFalse(responseHeaders.contains("Access-Control-Allow-Origin"));
+    private void assertDoNotContainsHeader(ModifiableHttpHeaders responseHeaders) {
+        assertFalse(responseHeaders.hasHeader("Access-Control-Allow-Methods"));
+        assertFalse(responseHeaders.hasHeader("Access-Control-Allow-Headers"));
+        assertFalse(responseHeaders.hasHeader("Access-Control-Max-Age"));
+        assertFalse(responseHeaders.hasHeader("Access-Control-Allow-Origin"));
     }
 }
diff --git a/server/server-core/src/test/java/software/amazon/smithy/java/server/core/TestStructs.java b/server/server-core/src/test/java/software/amazon/smithy/java/server/core/TestStructs.java
@@ -5,6 +5,7 @@
 
 package software.amazon.smithy.java.server.core;
 
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -130,39 +131,41 @@ public ApiService service() {
     }
 
     public static class TestModifiableHttpHeaders implements ModifiableHttpHeaders {
+        Map<String, List<String>> headers = new HashMap<>();
+
         @Override
         public void putHeader(String name, String value) {
-
+            headers.put(name, List.of(value));
         }
 
         @Override
         public void putHeader(String name, List<String> values) {
-
+            headers.put(name, values);
         }
 
         @Override
         public void removeHeader(String name) {
-
+            headers.remove(name);
         }
 
         @Override
         public List<String> allValues(String name) {
-            return List.of();
+            return headers.getOrDefault(name, List.of());
         }
 
         @Override
         public int size() {
-            return 0;
+            return headers.size();
         }
 
         @Override
         public Map<String, List<String>> map() {
-            return Map.of();
+            return headers;
         }
 
         @Override
         public Iterator<Map.Entry<String, List<String>>> iterator() {
-            return null;
+            return headers.entrySet().iterator();
         }
     }
 
diff --git a/server/server-netty/src/main/java/software/amazon/smithy/java/server/netty/HttpRequestHandler.java b/server/server-netty/src/main/java/software/amazon/smithy/java/server/netty/HttpRequestHandler.java
@@ -99,12 +99,12 @@ private void writeResponse(Channel channel, HttpJob job) {
                     HttpVersion.HTTP_1_1,
                     HttpResponseStatus.valueOf(job.response().getStatusCode()),
                     Unpooled.wrappedBuffer(serializedValue.waitForByteBuffer()));
+            CorsHeaders.addCorsHeaders(job);
             response.headers().set(((NettyHttpHeaders) job.response().headers()).getNettyHeaders());
             response.headers().set("content-length", serializedValue.contentLength());
             if (serializedValue.contentType() != null) {
                 response.headers().set("content-type", serializedValue.contentType());
             }
-            CorsHeaders.of(job, response.headers());
         } catch (Throwable e) {
             response = new DefaultFullHttpResponse(
                     HttpVersion.HTTP_1_1,

Original file line number	Diff line number	Diff line change
`@@ -16,5 +16,4 @@ dependencies {`
`16`	`16`	`implementation(libs.smithy.model)`
`17`	`17`	`implementation(project(":io"))`
`18`	`18`	`implementation(project(":logging"))`
`19`		`- implementation(libs.netty.all)`
`20`	`19`	`}`
Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,7 @@`
`5`	`5`
`6`	`6`	`package software.amazon.smithy.java.server.core;`
`7`	`7`
	`8`	`+import java.util.HashMap;`
`8`	`9`	`import java.util.Iterator;`
`9`	`10`	`import java.util.List;`
`10`	`11`	`import java.util.Map;`
`@@ -130,39 +131,41 @@ public ApiService service() {`
`130`	`131`	`}`
`131`	`132`
`132`	`133`	`public static class TestModifiableHttpHeaders implements ModifiableHttpHeaders {`
	`134`	`+ Map<String, List<String>> headers = new HashMap<>();`
	`135`	`+`
`133`	`136`	`@Override`
`134`	`137`	`public void putHeader(String name, String value) {`
`135`		`-`
	`138`	`+ headers.put(name, List.of(value));`
`136`	`139`	`}`
`137`	`140`
`138`	`141`	`@Override`
`139`	`142`	`public void putHeader(String name, List<String> values) {`
`140`		`-`
	`143`	`+ headers.put(name, values);`
`141`	`144`	`}`
`142`	`145`
`143`	`146`	`@Override`
`144`	`147`	`public void removeHeader(String name) {`
`145`		`-`
	`148`	`+ headers.remove(name);`
`146`	`149`	`}`
`147`	`150`
`148`	`151`	`@Override`
`149`	`152`	`public List<String> allValues(String name) {`
`150`		`- return List.of();`
	`153`	`+ return headers.getOrDefault(name, List.of());`
`151`	`154`	`}`
`152`	`155`
`153`	`156`	`@Override`
`154`	`157`	`public int size() {`
`155`		`- return 0;`
	`158`	`+ return headers.size();`
`156`	`159`	`}`
`157`	`160`
`158`	`161`	`@Override`
`159`	`162`	`public Map<String, List<String>> map() {`
`160`		`- return Map.of();`
	`163`	`+ return headers;`
`161`	`164`	`}`
`162`	`165`
`163`	`166`	`@Override`
`164`	`167`	`public Iterator<Map.Entry<String, List<String>>> iterator() {`
`165`		`- return null;`
	`168`	`+ return headers.entrySet().iterator();`
`166`	`169`	`}`
`167`	`170`	`}`
`168`	`171`