fix(): fix unit test && Fix cors specification

Rawven · May 18, 2024 · 9be5013 · 9be5013
1 parent 8269417
commit 9be5013
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 23 deletions.
diff --git a/...pc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/cors/CorsHeaderFilter.java b/...pc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/rest/cors/CorsHeaderFilter.java
@@ -40,7 +40,9 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.regex.Pattern;
 
 import static org.apache.dubbo.common.constants.CommonConstants.ANY_VALUE;
@@ -90,20 +92,16 @@ protected void invoke(Invoker<?> invoker, RpcInvocation invocation, HttpRequest
     }
 
     private boolean process(CorsMeta cors, HttpRequest request, HttpResponse response) {
-        List<String> varyHeaders = response.headerValues(VARY);
 
-        StringBuilder varyBuilder = new StringBuilder();
-        for (String header : new String[] {ORIGIN, ACCESS_CONTROL_REQUEST_METHOD, ACCESS_CONTROL_REQUEST_HEADERS}) {
-            if (varyHeaders == null || !varyHeaders.contains(header)) {
-                if (varyBuilder.length() > 0) {
-                    varyBuilder.append(", ");
-                }
-                varyBuilder.append(header);
-            }
-        }
-        if (varyBuilder.length() > 0) {
-            response.setHeader(VARY, varyBuilder.toString());
+        Set<String> varHeadersSet = new LinkedHashSet<>();
+        List<String> varyHeaders = response.headerValues(VARY);
+        if (varyHeaders != null) {
+            varHeadersSet.addAll(varyHeaders);
         }
+        varHeadersSet.add(ORIGIN);
+        varHeadersSet.add(ACCESS_CONTROL_REQUEST_METHOD);
+        varHeadersSet.add(ACCESS_CONTROL_REQUEST_HEADERS);
+        response.setHeader(VARY, StringUtils.join(varHeadersSet, ", "));
 
         String origin = request.header(ORIGIN);
         if (isNotCorsRequest(request, origin)) {
@@ -138,18 +136,18 @@ private boolean process(CorsMeta cors, HttpRequest request, HttpResponse respons
         response.setHeader(ACCESS_CONTROL_ALLOW_ORIGIN, allowOrigin);
 
         if (ArrayUtils.isNotEmpty(cors.getExposedHeaders())) {
-            response.setHeader(ACCESS_CONTROL_EXPOSE_HEADERS, StringUtils.join(cors.getExposedHeaders(), ","));
+            response.setHeader(ACCESS_CONTROL_EXPOSE_HEADERS, StringUtils.join(cors.getExposedHeaders(), ", "));
         }
 
         if (Boolean.TRUE.equals(cors.getAllowCredentials())) {
             response.setHeader(ACCESS_CONTROL_ALLOW_CREDENTIALS, Boolean.TRUE.toString());
         }
 
         if (preFlight) {
-            response.setHeader(ACCESS_CONTROL_ALLOW_METHODS, StringUtils.join(allowMethods, ","));
+            response.setHeader(ACCESS_CONTROL_ALLOW_METHODS, StringUtils.join(allowMethods, ", "));
 
             if (!allowHeaders.isEmpty()) {
-                response.setHeader(ACCESS_CONTROL_ALLOW_HEADERS, StringUtils.join(allowHeaders, ","));
+                response.setHeader(ACCESS_CONTROL_ALLOW_HEADERS, StringUtils.join(allowHeaders, ", "));
             }
             if (cors.getMaxAge() != null) {
                 response.setHeader(ACCESS_CONTROL_MAX_AGE, cors.getMaxAge().toString());
@@ -210,7 +208,7 @@ private static List<String> checkMethods(CorsMeta cors, String method) {
     }
 
     private static List<String> checkHeaders(CorsMeta cors, Collection<String> headers) {
-        if (headers == null) {
+        if (headers == null || headers.isEmpty()) {
             return Collections.emptyList();
         }
         String[] allowedHeaders = cors.getAllowedHeaders();

diff --git a/...riple/src/test/java/org/apache/dubbo/rpc/protocol/tri/rest/cors/CorsHeaderFilterTest.java b/...riple/src/test/java/org/apache/dubbo/rpc/protocol/tri/rest/cors/CorsHeaderFilterTest.java
@@ -243,7 +243,8 @@ void preflightRequestMatchedAllowedMethod() {
         Mockito.when(request.hasHeader(CorsHeaderFilter.ACCESS_CONTROL_REQUEST_METHOD))
                 .thenReturn(true);
         Mockito.when(build.getCors())
-                .thenReturn(CorsMeta.builder().allowedOrigins("*").build());
+                .thenReturn(
+                        CorsMeta.builder().allowedOrigins("*").applyDefault().build());
         try {
             this.processor.process(this.request, this.response);
             Assertions.fail();
@@ -339,11 +340,7 @@ void preflightRequestValidRequestAndConfig() {
         Assertions.assertEquals("*", this.response.header(CorsHeaderFilter.ACCESS_CONTROL_ALLOW_ORIGIN));
         Assertions.assertTrue(this.response.hasHeader(CorsHeaderFilter.ACCESS_CONTROL_ALLOW_METHODS));
         log.info("{}", this.response.headerValues(CorsHeaderFilter.ACCESS_CONTROL_ALLOW_METHODS));
-        Assertions.assertArrayEquals(
-                new String[] {"GET"},
-                this.response
-                        .headerValues(CorsHeaderFilter.ACCESS_CONTROL_ALLOW_METHODS)
-                        .toArray());
+        Assertions.assertEquals("GET, PUT", this.response.header(CorsHeaderFilter.ACCESS_CONTROL_ALLOW_METHODS));
         Assertions.assertFalse(this.response.hasHeader(CorsHeaderFilter.ACCESS_CONTROL_MAX_AGE));
         Assertions.assertTrue(this.response.header(CorsHeaderFilter.VARY).contains(CorsHeaderFilter.ORIGIN));
         Assertions.assertTrue(
@@ -468,7 +465,6 @@ void preflightRequestWithEmptyHeaders() {
                 this.response.header(CorsHeaderFilter.VARY).contains(CorsHeaderFilter.ACCESS_CONTROL_REQUEST_METHOD));
         Assertions.assertTrue(
                 this.response.header(CorsHeaderFilter.VARY).contains(CorsHeaderFilter.ACCESS_CONTROL_REQUEST_HEADERS));
-        Assertions.assertEquals(HttpStatus.OK.getCode(), this.response.status());
     }
 
     @Test