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 2022/11/02 01:01:56 UTC
[servicecomb-java-chassis] branch master updated: [SCB-2715]if body parameter not provided should fail for spring mvc (#3447)
This is an automated email from the ASF dual-hosted git repository.
liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git
The following commit(s) were added to refs/heads/master by this push:
new 58f39d54f [SCB-2715]if body parameter not provided should fail for spring mvc (#3447)
58f39d54f is described below
commit 58f39d54fd367587b49583de1a314a78afaf0051
Author: liubao68 <bi...@qq.com>
AuthorDate: Wed Nov 2 09:01:49 2022 +0800
[SCB-2715]if body parameter not provided should fail for spring mvc (#3447)
---
.../org/apache/servicecomb/common/rest/codec/RestCodec.java | 12 +++++++-----
.../common/rest/codec/param/BodyProcessorCreator.java | 10 ++++++++++
.../common/rest/codec/param/TestBodyProcessor.java | 13 ++++++-------
.../jaxrs/client/validation/ValidationServiceClient.java | 3 ++-
.../demo/jaxrs/server/validation/ValidationService.java | 4 +---
.../apache/servicecomb/demo/springmvc/SpringmvcClient.java | 9 +++++++++
.../servicecomb/demo/springmvc/server/AnnotationsTest.java | 8 ++++++++
.../reference/async/CseAsyncClientHttpRequestTest.java | 2 ++
8 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/RestCodec.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/RestCodec.java
index b3ab4c70b..bf5d83ddb 100644
--- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/RestCodec.java
+++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/RestCodec.java
@@ -60,15 +60,17 @@ public final class RestCodec {
for (RestParam param : paramList) {
try {
paramValues.put(param.getParamName(), param.getParamProcessor().getValue(request));
+ } catch (InvocationException e) {
+ throw e;
} catch (Exception e) {
// Avoid information leak of user input, and add option for debug use.
String message = String
- .format("Parameter is not valid for operation [%s]. Parameter is [%s]. Processor is [%s].",
- restOperation.getOperationMeta().getMicroserviceQualifiedName(),
- param.getParamName(),
- param.getParamProcessor().getProcessorType());
+ .format("Parameter is not valid for operation [%s]. Parameter is [%s]. Processor is [%s].",
+ restOperation.getOperationMeta().getMicroserviceQualifiedName(),
+ param.getParamName(),
+ param.getParamProcessor().getProcessorType());
if (DynamicPropertyFactory.getInstance().getBooleanProperty(
- RestConst.PRINT_CODEC_ERROR_MESSGAGE, false).get()) {
+ RestConst.PRINT_CODEC_ERROR_MESSGAGE, false).get()) {
LOG.error(message, e);
} else {
LOG.error("{} Add {}=true to print the details.", message, RestConst.PRINT_CODEC_ERROR_MESSGAGE);
diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/BodyProcessorCreator.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/BodyProcessorCreator.java
index 457bc8070..d05b2a91e 100644
--- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/BodyProcessorCreator.java
+++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/param/BodyProcessorCreator.java
@@ -26,6 +26,7 @@ import java.util.Locale;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response.Status;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
@@ -36,6 +37,7 @@ import org.apache.servicecomb.foundation.vertx.stream.BufferOutputStream;
import org.apache.servicecomb.swagger.SwaggerUtils;
import org.apache.servicecomb.swagger.converter.ConverterMgr;
import org.apache.servicecomb.swagger.generator.SwaggerConst;
+import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -98,6 +100,14 @@ public class BodyProcessorCreator implements ParamValueProcessorCreator {
@Override
public Object getValue(HttpServletRequest request) throws Exception {
+ Object result = getValueImpl(request);
+ if (result == null && this.isRequired) {
+ throw new InvocationException(Status.BAD_REQUEST, "Body parameter is required.");
+ }
+ return result;
+ }
+
+ private Object getValueImpl(HttpServletRequest request) throws IOException {
Object body = request.getAttribute(RestConst.BODY_PARAMETER);
if (body != null) {
return convertValue(body, targetType);
diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestBodyProcessor.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestBodyProcessor.java
index 0fecdf86e..2db296e4d 100644
--- a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestBodyProcessor.java
+++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/param/TestBodyProcessor.java
@@ -27,13 +27,16 @@ import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MediaType;
-import io.vertx.core.buffer.impl.BufferImpl;
import org.apache.servicecomb.common.rest.RestConst;
import org.apache.servicecomb.common.rest.codec.RestClientRequest;
import org.apache.servicecomb.common.rest.codec.param.BodyProcessorCreator.BodyProcessor;
import org.apache.servicecomb.common.rest.codec.param.BodyProcessorCreator.RawJsonBodyProcessor;
import org.apache.servicecomb.foundation.vertx.stream.BufferInputStream;
+import org.apache.servicecomb.swagger.invocation.exception.InvocationException;
import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
import com.fasterxml.jackson.databind.type.TypeFactory;
@@ -41,10 +44,8 @@ import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.vertx.core.MultiMap;
import io.vertx.core.buffer.Buffer;
+import io.vertx.core.buffer.impl.BufferImpl;
import io.vertx.core.http.impl.headers.HeadersMultiMap;
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
-import org.mockito.Mockito;
public class TestBodyProcessor {
@@ -79,7 +80,6 @@ public class TestBodyProcessor {
return null;
}).when(clientRequest).putHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON);
-
Mockito.when(clientRequest.getHeaders()).thenReturn(headers);
}
@@ -112,8 +112,7 @@ public class TestBodyProcessor {
public void testGetValueNoAttrNoStream() throws Exception {
createProcessor(String.class);
Mockito.when(request.getInputStream()).thenReturn(null);
- Object result = processor.getValue(request);
- Assertions.assertNull(result);
+ Assertions.assertThrows(InvocationException.class, () -> processor.getValue(request));
}
@Test
diff --git a/demo/demo-jaxrs/jaxrs-client/src/main/java/org/apache/servicecomb/demo/jaxrs/client/validation/ValidationServiceClient.java b/demo/demo-jaxrs/jaxrs-client/src/main/java/org/apache/servicecomb/demo/jaxrs/client/validation/ValidationServiceClient.java
index c0170f2be..803a1dfe4 100644
--- a/demo/demo-jaxrs/jaxrs-client/src/main/java/org/apache/servicecomb/demo/jaxrs/client/validation/ValidationServiceClient.java
+++ b/demo/demo-jaxrs/jaxrs-client/src/main/java/org/apache/servicecomb/demo/jaxrs/client/validation/ValidationServiceClient.java
@@ -89,7 +89,7 @@ public class ValidationServiceClient {
} catch (InvocationException e) {
TestMgr.check(400, e.getStatus().getStatusCode());
TestMgr.check(Status.BAD_REQUEST, e.getReasonPhrase());
- TestMgr.check(e.getErrorData().toString().contains("Parameter is not valid for operation"), true);
+ TestMgr.check(e.getErrorData().toString().contains("Parameter is required."), true);
}
Teacher teacher = new Teacher();
@@ -108,6 +108,7 @@ public class ValidationServiceClient {
TestMgr.check(e.getErrorData().toString().contains("must not be blank"), true);
}
+ // jax-rs body default required = false
response = template.postForObject(urlPrefix + "/sayTeacherInfo", null, Teacher.class);
TestMgr.check(null, response);
}
diff --git a/demo/demo-jaxrs/jaxrs-server/src/main/java/org/apache/servicecomb/demo/jaxrs/server/validation/ValidationService.java b/demo/demo-jaxrs/jaxrs-server/src/main/java/org/apache/servicecomb/demo/jaxrs/server/validation/ValidationService.java
index 90b14d043..4b4620375 100644
--- a/demo/demo-jaxrs/jaxrs-server/src/main/java/org/apache/servicecomb/demo/jaxrs/server/validation/ValidationService.java
+++ b/demo/demo-jaxrs/jaxrs-server/src/main/java/org/apache/servicecomb/demo/jaxrs/server/validation/ValidationService.java
@@ -27,7 +27,6 @@ import javax.ws.rs.QueryParam;
import org.apache.servicecomb.demo.validator.Teacher;
import org.apache.servicecomb.provider.rest.common.RestSchema;
-import org.springframework.web.bind.annotation.RequestBody;
@RestSchema(schemaId = "ValidationService")
@Path("ValidationService")
@@ -46,8 +45,7 @@ public class ValidationService {
@Path("/sayTeacherInfo")
@POST
- public Teacher sayTeacherInfo(@Valid @RequestBody Teacher teacher) {
+ public Teacher sayTeacherInfo(@Valid Teacher teacher) {
return teacher;
}
-
}
diff --git a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/SpringmvcClient.java b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/SpringmvcClient.java
index 7a023c84c..200f0047c 100644
--- a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/SpringmvcClient.java
+++ b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/SpringmvcClient.java
@@ -403,6 +403,15 @@ public class SpringmvcClient {
"test",
String.class,
"ha"));
+
+ try {
+ template.postForObject(prefix + "/annotations/testRequiredBody",
+ null,
+ String.class);
+ TestMgr.fail("should fail");
+ } catch (InvocationException e) {
+ TestMgr.check(e.getStatusCode(), HttpStatus.SC_BAD_REQUEST);
+ }
}
private static void testSpringMvcDefaultValuesRest(RestTemplate template, String microserviceName) {
diff --git a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/AnnotationsTest.java b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/AnnotationsTest.java
index 3af7e0ed3..b189ee894 100644
--- a/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/AnnotationsTest.java
+++ b/demo/demo-springmvc/springmvc-server/src/main/java/org/apache/servicecomb/demo/springmvc/server/AnnotationsTest.java
@@ -70,4 +70,12 @@ public class AnnotationsTest {
}
return user;
}
+
+ @RequestMapping(path = "/testRequiredBody", method = RequestMethod.POST)
+ public String testRequiredBody(@RequestBody(required = true) Person user) {
+ if (user == null) {
+ return "Should not happen";
+ }
+ return user.getName();
+ }
}
diff --git a/providers/provider-springmvc/src/test/java/org/apache/servicecomb/provider/springmvc/reference/async/CseAsyncClientHttpRequestTest.java b/providers/provider-springmvc/src/test/java/org/apache/servicecomb/provider/springmvc/reference/async/CseAsyncClientHttpRequestTest.java
index a1b50a88b..c58d77b37 100644
--- a/providers/provider-springmvc/src/test/java/org/apache/servicecomb/provider/springmvc/reference/async/CseAsyncClientHttpRequestTest.java
+++ b/providers/provider-springmvc/src/test/java/org/apache/servicecomb/provider/springmvc/reference/async/CseAsyncClientHttpRequestTest.java
@@ -108,6 +108,8 @@ public class CseAsyncClientHttpRequestTest {
return completableFuture;
}
};
+ byte[] body = "abc".getBytes();
+ client.setRequestBody(body);
ListenableFuture<ClientHttpResponse> future = client.executeAsync();
future.addCallback(
new ListenableFutureCallback<ClientHttpResponse>() {