You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2023/11/09 10:52:05 UTC
(servicecomb-java-chassis) branch 2.8.x updated: [#3979]can configure not decode invocation context (#4020)
This is an automated email from the ASF dual-hosted git repository.
liubao pushed a commit to branch 2.8.x
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git
The following commit(s) were added to refs/heads/2.8.x by this push:
new 09c08862d [#3979]can configure not decode invocation context (#4020)
09c08862d is described below
commit 09c08862d89f1fae4b524bb594b209e41cd9d94e
Author: liubao68 <bi...@qq.com>
AuthorDate: Thu Nov 9 18:52:00 2023 +0800
[#3979]can configure not decode invocation context (#4020)
---
.../apache/servicecomb/common/rest/RestConst.java | 2 +
.../common/rest/RestProducerInvocationCreator.java | 9 +++-
.../rest/RestProducerInvocationCreatorTest.java | 51 +++++++++++++++++-----
.../edge/core/EdgeInvocationCreator.java | 5 ---
.../edge-core/src/main/resources/microservice.yaml | 23 ++++++++++
5 files changed, 73 insertions(+), 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
index 3d0bd954a..12ae2e0ec 100644
--- 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 @@ public final class 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/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/RestProducerInvocationCreator.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/RestProducerInvocationCreator.java
index f93bc597d..73bad7e8d 100644
--- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/RestProducerInvocationCreator.java
+++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/RestProducerInvocationCreator.java
@@ -27,7 +27,6 @@ import java.util.concurrent.CompletableFuture;
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.apache.servicecomb.foundation.vertx.http.HttpServletResponseEx;
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 @@ public abstract class RestProducerInvocationCreator implements InvocationCreator
}
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/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/RestProducerInvocationCreatorTest.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/RestProducerInvocationCreatorTest.java
index 12877bc4a..a35ad0397 100644
--- a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/RestProducerInvocationCreatorTest.java
+++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/RestProducerInvocationCreatorTest.java
@@ -19,9 +19,12 @@ package org.apache.servicecomb.common.rest;
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,9 +46,6 @@ import org.apache.servicecomb.foundation.vertx.http.HttpServletRequestEx;
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;
@@ -53,7 +53,8 @@ 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 class RestProducerInvocationCreatorTest {
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 class RestProducerInvocationCreatorTest {
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 class RestProducerInvocationCreatorTest {
@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 class RestProducerInvocationCreatorTest {
@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 class RestProducerInvocationCreatorTest {
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
index 17d67a647..c848ff74a 100644
--- 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 @@ public class EdgeInvocationCreator extends RestVertxProducerInvocationCreator {
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
new file mode 100644
index 000000000..a456c2868
--- /dev/null
+++ 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