[#3979]can configure not decode invocation context

apache · Nov 9, 2023 · d935c22 · d935c22
1 parent e0bd12b
commit d935c22
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 17 deletions.
diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/RestConst.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/RestConst.java
@@ -86,4 +86,6 @@ private RestConst() {
   public static final String HEADER_CONTEXT_MAPPER = "servicecomb.context.headerContextMapper";
 
   public static final String QUERY_CONTEXT_MAPPER = "servicecomb.context.queryContextMapper";
+
+  public static final String DECODE_INVOCATION_CONTEXT = "servicecomb.context.decodeInvocationContext";
 }
diff --git a/...-rest/src/main/java/org/apache/servicecomb/common/rest/RestProducerInvocationCreator.java b/...-rest/src/main/java/org/apache/servicecomb/common/rest/RestProducerInvocationCreator.java
@@ -27,7 +27,6 @@
 import javax.annotation.Nonnull;
 import javax.ws.rs.core.HttpHeaders;
 
-import com.google.common.annotations.VisibleForTesting;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.servicecomb.common.rest.codec.produce.ProduceProcessor;
 import org.apache.servicecomb.common.rest.definition.RestOperationMeta;
@@ -45,6 +44,9 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.annotations.VisibleForTesting;
+import com.netflix.config.DynamicPropertyFactory;
+
 import io.vertx.core.json.Json;
 
 public abstract class RestProducerInvocationCreator implements InvocationCreator {
@@ -90,6 +92,11 @@ protected Invocation createInstance() {
   }
 
   protected void initInvocationContext(Invocation invocation) {
+    if (!DynamicPropertyFactory.getInstance()
+        .getBooleanProperty(RestConst.DECODE_INVOCATION_CONTEXT, true).get()) {
+      return;
+    }
+
     String strCseContext = requestEx.getHeader(Const.CSE_CONTEXT);
     if (StringUtils.isEmpty(strCseContext)) {
       return;

diff --git a/...t/src/test/java/org/apache/servicecomb/common/rest/RestProducerInvocationCreatorTest.java b/...t/src/test/java/org/apache/servicecomb/common/rest/RestProducerInvocationCreatorTest.java
@@ -19,9 +19,12 @@
 
 import static javax.ws.rs.core.Response.Status.NOT_ACCEPTABLE;
 import static javax.ws.rs.core.Response.Status.NOT_FOUND;
+import static org.apache.servicecomb.common.rest.RestConst.DECODE_INVOCATION_CONTEXT;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.catchThrowable;
 
+import java.util.ArrayList;
+
 import javax.ws.rs.core.HttpHeaders;
 
 import org.apache.servicecomb.common.rest.definition.RestOperationMeta;
@@ -43,17 +46,15 @@
 import org.apache.servicecomb.foundation.vertx.http.HttpServletResponseEx;
 import org.apache.servicecomb.swagger.invocation.exception.CommonExceptionData;
 import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
-
-import io.vertx.core.json.Json;
-import io.vertx.ext.web.RoutingContext;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.MockedStatic;
 import org.mockito.Mockito;
 
-import java.util.ArrayList;
+import io.vertx.core.json.Json;
+import io.vertx.ext.web.RoutingContext;
 
 public class RestProducerInvocationCreatorTest {
 
@@ -115,14 +116,15 @@ public void should_failed_when_not_defined_any_schema() {
 
       assertThat(throwable.getStatusCode()).isEqualTo(NOT_FOUND.getStatusCode());
       assertThat(Json.encode(data)).isIn("{\"code\":\"SCB.00000002\",\"message\":\"Not Found\"}",
-              "{\"message\":\"Not Found\",\"code\":\"SCB.00000002\"}");
+          "{\"message\":\"Not Found\",\"code\":\"SCB.00000002\"}");
     }
   }
 
   @Test
   public void should_failed_when_accept_is_not_support() {
     try (MockedStatic<ServicePathManager> mockedStatic = Mockito.mockStatic(ServicePathManager.class)) {
-      mockedStatic.when(() -> ServicePathManager.getServicePathManager(microserviceMeta)).thenReturn(servicePathManager);
+      mockedStatic.when(() -> ServicePathManager.getServicePathManager(microserviceMeta))
+          .thenReturn(servicePathManager);
       Mockito.when(requestEx.getHeader(HttpHeaders.ACCEPT)).thenReturn("test-type");
       Mockito.when(restOperationMeta.ensureFindProduceProcessor(requestEx)).thenReturn(null);
       Mockito.when(creator.locateOperation(microserviceMeta)).thenReturn(locator);
@@ -137,15 +139,17 @@ public void should_failed_when_accept_is_not_support() {
       CommonExceptionData data = (CommonExceptionData) throwable.getErrorData();
 
       assertThat(throwable.getStatusCode()).isEqualTo(NOT_ACCEPTABLE.getStatusCode());
-      assertThat(Json.encode(data)).isIn("{\"code\":\"SCB.00000000\",\"message\":\"Accept test-type is not supported\"}",
-              "{\"message\":\"Accept test-type is not supported\",\"code\":\"SCB.00000000\"}");
+      assertThat(Json.encode(data)).isIn(
+          "{\"code\":\"SCB.00000000\",\"message\":\"Accept test-type is not supported\"}",
+          "{\"message\":\"Accept test-type is not supported\",\"code\":\"SCB.00000000\"}");
     }
   }
 
   @Test
   public void should_save_requestEx_in_invocation_context() {
     try (MockedStatic<ServicePathManager> mockedStatic = Mockito.mockStatic(ServicePathManager.class)) {
-      mockedStatic.when(() -> ServicePathManager.getServicePathManager(microserviceMeta)).thenReturn(servicePathManager);
+      mockedStatic.when(() -> ServicePathManager.getServicePathManager(microserviceMeta))
+          .thenReturn(servicePathManager);
       Mockito.when(creator.locateOperation(microserviceMeta)).thenReturn(locator);
       Mockito.when(locator.getOperation()).thenReturn(restOperationMeta);
       Mockito.when(restOperationMeta.getOperationMeta()).thenReturn(operationMeta);
@@ -165,7 +169,8 @@ public void should_save_requestEx_in_invocation_context() {
   @Test
   public void should_save_path_var_map_in_requestEx() {
     try (MockedStatic<ServicePathManager> mockedStatic = Mockito.mockStatic(ServicePathManager.class)) {
-      mockedStatic.when(() -> ServicePathManager.getServicePathManager(microserviceMeta)).thenReturn(servicePathManager);
+      mockedStatic.when(() -> ServicePathManager.getServicePathManager(microserviceMeta))
+          .thenReturn(servicePathManager);
       Mockito.when(creator.locateOperation(microserviceMeta)).thenReturn(locator);
       Mockito.when(locator.getOperation()).thenReturn(restOperationMeta);
       Mockito.when(restOperationMeta.getOperationMeta()).thenReturn(operationMeta);
@@ -183,8 +188,10 @@ public void should_save_path_var_map_in_requestEx() {
 
   @Test
   public void should_merge_invocation_context_from_request() {
+    ArchaiusUtils.setProperty(DECODE_INVOCATION_CONTEXT, true);
     try (MockedStatic<ServicePathManager> mockedStatic = Mockito.mockStatic(ServicePathManager.class)) {
-      mockedStatic.when(() -> ServicePathManager.getServicePathManager(microserviceMeta)).thenReturn(servicePathManager);
+      mockedStatic.when(() -> ServicePathManager.getServicePathManager(microserviceMeta))
+          .thenReturn(servicePathManager);
       Mockito.when(creator.locateOperation(microserviceMeta)).thenReturn(locator);
       Mockito.when(locator.getOperation()).thenReturn(restOperationMeta);
       Mockito.when(restOperationMeta.getOperationMeta()).thenReturn(operationMeta);
@@ -200,4 +207,26 @@ public void should_merge_invocation_context_from_request() {
       assertThat(invocation.getContext("k")).isEqualTo("v");
     }
   }
+
+  @Test
+  public void should_not_merge_invocation_context_from_request() {
+    ArchaiusUtils.setProperty(DECODE_INVOCATION_CONTEXT, false);
+    try (MockedStatic<ServicePathManager> mockedStatic = Mockito.mockStatic(ServicePathManager.class)) {
+      mockedStatic.when(() -> ServicePathManager.getServicePathManager(microserviceMeta))
+          .thenReturn(servicePathManager);
+      Mockito.when(creator.locateOperation(microserviceMeta)).thenReturn(locator);
+      Mockito.when(locator.getOperation()).thenReturn(restOperationMeta);
+      Mockito.when(restOperationMeta.getOperationMeta()).thenReturn(operationMeta);
+      Mockito.when(operationMeta.buildBaseProviderRuntimeType()).thenReturn(invocationRuntimeType);
+      Mockito.when(operationMeta.getSchemaMeta()).thenReturn(schemaMeta);
+      Mockito.when(schemaMeta.getMicroserviceMeta()).thenReturn(microserviceMeta);
+      Mockito.when(microserviceMeta.getHandlerChain()).thenReturn(new ArrayList<>());
+      Mockito.doNothing().when(creator).initProduceProcessor();
+      Mockito.when(requestEx.getHeader(Const.CSE_CONTEXT)).thenReturn("{\"k\":\"v\"}");
+
+      Invocation invocation = creator.createAsync().join();
+
+      assertThat(invocation.getContext("k")).isNull();
+    }
+  }
 }
diff --git a/edge/edge-core/src/main/java/org/apache/servicecomb/edge/core/EdgeInvocationCreator.java b/edge/edge-core/src/main/java/org/apache/servicecomb/edge/core/EdgeInvocationCreator.java
@@ -73,11 +73,6 @@ protected OperationLocator locateOperation(ServicePathManager servicePathManager
     return servicePathManager.consumerLocateOperation(path, requestEx.getMethod());
   }
 
-  @Override
-  protected void initInvocationContext(Invocation invocation) {
-    // do not read InvocationContext from HTTP header, for security reason
-  }
-
   @Override
   protected Invocation createInstance() {
     ReferenceConfig referenceConfig = microserviceReferenceConfig

diff --git a/edge/edge-core/src/main/resources/microservice.yaml b/edge/edge-core/src/main/resources/microservice.yaml
@@ -0,0 +1,23 @@
+## ---------------------------------------------------------------------------
+## Licensed to the Apache Software Foundation (ASF) under one or more
+## contributor license agreements.  See the NOTICE file distributed with
+## this work for additional information regarding copyright ownership.
+## The ASF licenses this file to You 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.
+## ---------------------------------------------------------------------------
+
+servicecomb-config-order: -500
+
+servicecomb:
+  context:
+    # do not decode invocation context for edge service by default
+    decodeInvocationContext: false