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/incubator-servicecomb-java-chassis.git
commit 45471e76d9a6cef4d997ef2b496b7006558e829b Author: liubao <bao....@huawei.com> AuthorDate: Thu Aug 9 22:22:59 2018 +0800 [SCB-828]In some tomcat implementation inputstream available is null --- .../servicecomb/common/rest/codec/RestCodec.java | 4 ++-- .../rest/codec/param/BodyProcessorCreator.java | 20 +++++++++++++++----- .../common/rest/codec/produce/ProduceProcessor.java | 4 ---- .../servicecomb/common/rest/codec/TestRestCodec.java | 4 ++-- .../common/rest/codec/TestRestObjectMapper.java | 14 ++++++++++++++ .../rest/codec/produce/TestProduceJsonProcessor.java | 10 ++++++++-- .../codec/produce/TestProduceTextPlainProcessor.java | 2 +- 7 files changed, 42 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 7c80dfd..127b76d 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 @@ -23,7 +23,6 @@ import javax.servlet.http.HttpServletRequest; import org.apache.servicecomb.common.rest.definition.RestOperationMeta; import org.apache.servicecomb.common.rest.definition.RestParam; -import org.apache.servicecomb.swagger.invocation.exception.ExceptionFactory; import org.apache.servicecomb.swagger.invocation.exception.InvocationException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -67,7 +66,8 @@ public final class RestCodec { LOG.error("Parameter is not valid for operation {}. ", restOperation.getOperationMeta().getMicroserviceQualifiedName(), e); - throw ExceptionFactory.convertProducerException(e, "Parameter is not valid."); + // give standard http error code for invalid parameter + throw new InvocationException(400, "", "Parameter is not valid."); } } } 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 c21f527..61f0ea9 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 @@ -31,14 +31,18 @@ import org.apache.servicecomb.common.rest.codec.RestClientRequest; import org.apache.servicecomb.common.rest.codec.RestObjectMapperFactory; import org.apache.servicecomb.foundation.vertx.stream.BufferOutputStream; import org.apache.servicecomb.swagger.generator.core.utils.ClassUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.fasterxml.jackson.databind.JavaType; +import com.fasterxml.jackson.databind.exc.MismatchedInputException; import com.fasterxml.jackson.databind.type.TypeFactory; import io.swagger.models.parameters.Parameter; import io.vertx.core.buffer.Buffer; public class BodyProcessorCreator implements ParamValueProcessorCreator { + private static final Logger LOGGER = LoggerFactory.getLogger(BodyProcessorCreator.class); public static final String PARAMTYPE = "body"; @@ -75,15 +79,21 @@ public class BodyProcessorCreator implements ParamValueProcessorCreator { return null; } - if (isRequired == false && inputStream.available() == 0) { - return null; - } - if (!contentType.isEmpty() && !contentType.startsWith(MediaType.APPLICATION_JSON)) { // TODO: we should consider body encoding return IOUtils.toString(inputStream, "UTF-8"); } - return RestObjectMapperFactory.getRestObjectMapper().readValue(inputStream, targetType); + + try { + return RestObjectMapperFactory.getRestObjectMapper().readValue(inputStream, targetType); + } catch (MismatchedInputException e) { + // there is no way to detect InputStream is empty, so have to catch the exception + if (!isRequired) { + LOGGER.warn("Mismatched content and required is false, taken as null. Msg=" + e.getMessage()); + return null; + } + throw e; + } } @Override diff --git a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/produce/ProduceProcessor.java b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/produce/ProduceProcessor.java index 127250f..4172cb5 100644 --- a/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/produce/ProduceProcessor.java +++ b/common/common-rest/src/main/java/org/apache/servicecomb/common/rest/codec/produce/ProduceProcessor.java @@ -54,10 +54,6 @@ public interface ProduceProcessor { } default Object decodeResponse(InputStream input, JavaType type) throws Exception { - if (input.available() == 0) { - return null; - } - return doDecodeResponse(input, type); } diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/TestRestCodec.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/TestRestCodec.java index d3817a1..ce6489b 100644 --- a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/TestRestCodec.java +++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/TestRestCodec.java @@ -133,8 +133,8 @@ public class TestRestCodec { RestCodec.restToArgs(request, restOperation); success = true; } catch (InvocationException e) { - Assert.assertEquals(590, e.getStatusCode()); - Assert.assertEquals("Parameter is not valid.", ((CommonExceptionData) e.getErrorData()).getMessage()); + Assert.assertEquals(400, e.getStatusCode()); + Assert.assertEquals("Parameter is not valid.", e.getErrorData()); } Assert.assertEquals(success, false); } diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/TestRestObjectMapper.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/TestRestObjectMapper.java index 1a9980e..af840e1 100644 --- a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/TestRestObjectMapper.java +++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/TestRestObjectMapper.java @@ -17,11 +17,15 @@ package org.apache.servicecomb.common.rest.codec; +import java.io.ByteArrayInputStream; +import java.io.InputStream; + import org.junit.Assert; import org.junit.Test; import com.fasterxml.jackson.core.JsonParser.Feature; import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.exc.MismatchedInputException; import com.fasterxml.jackson.databind.type.TypeFactory; import io.vertx.core.json.JsonObject; @@ -54,5 +58,15 @@ public class TestRestObjectMapper { .convertValue(obj, TypeFactory.defaultInstance().constructType(PojoModel.class)); Assert.assertEquals("a", model.getName()); Assert.assertEquals("b", model.getDesc()); + + InputStream inputStream = new ByteArrayInputStream(new byte[0]); + try { + RestObjectMapperFactory.getRestObjectMapper().readValue(inputStream, PojoModel.class); + Assert.fail(); + } catch (MismatchedInputException e) { + // right place, nothing to do. + } catch (Exception e) { + Assert.fail(); + } } } diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/produce/TestProduceJsonProcessor.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/produce/TestProduceJsonProcessor.java index b186bda..98a83d9 100644 --- a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/produce/TestProduceJsonProcessor.java +++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/produce/TestProduceJsonProcessor.java @@ -25,6 +25,7 @@ import org.junit.Assert; import org.junit.Test; import com.fasterxml.jackson.databind.JavaType; +import com.fasterxml.jackson.databind.exc.MismatchedInputException; import com.fasterxml.jackson.databind.type.TypeFactory; import io.vertx.core.buffer.Buffer; @@ -51,8 +52,13 @@ public class TestProduceJsonProcessor { Assert.assertNull(result); ByteArrayInputStream is = new ByteArrayInputStream(new byte[] {}); - result = pp.decodeResponse(is, resultType); - Assert.assertNull(result); + try { + pp.decodeResponse(is, resultType); + Assert.fail(); + } catch (Exception e) { + Assert.assertTrue(e instanceof MismatchedInputException); + } + } @Test diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/produce/TestProduceTextPlainProcessor.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/produce/TestProduceTextPlainProcessor.java index b1df575..905f70f 100644 --- a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/produce/TestProduceTextPlainProcessor.java +++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/codec/produce/TestProduceTextPlainProcessor.java @@ -52,7 +52,7 @@ public class TestProduceTextPlainProcessor { ByteArrayInputStream is = new ByteArrayInputStream(new byte[] {}); result = pp.decodeResponse(is, resultType); - Assert.assertNull(result); + Assert.assertEquals(result, ""); } @Test