Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SCB-828]In some tomcat implementation inputstream available is null #866

Merged
merged 3 commits into from
Aug 10, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -20,10 +20,10 @@
import java.util.List;

import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.core.Response.Status;

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;
Expand Down Expand Up @@ -67,7 +67,8 @@ public static Object[] restToArgs(HttpServletRequest request,
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(Status.BAD_REQUEST, "Parameter is not valid.");
}
}
}
Expand Up @@ -31,14 +31,18 @@
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";

Expand Down Expand Up @@ -75,15 +79,21 @@ public Object getValue(HttpServletRequest request) throws Exception {
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
Expand Down
Expand Up @@ -54,10 +54,6 @@ default Buffer encodeResponse(Object result) throws Exception {
}

default Object decodeResponse(InputStream input, JavaType type) throws Exception {
if (input.available() == 0) {
return null;
}

return doDecodeResponse(input, type);
}

Expand Down
Expand Up @@ -133,7 +133,7 @@ public void testRestToArgsExcetpion(@Mocked HttpServletRequest request,
RestCodec.restToArgs(request, restOperation);
success = true;
} catch (InvocationException e) {
Assert.assertEquals(590, e.getStatusCode());
Assert.assertEquals(400, e.getStatusCode());
Assert.assertEquals("Parameter is not valid.", ((CommonExceptionData) e.getErrorData()).getMessage());
}
Assert.assertEquals(success, false);
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -54,5 +58,15 @@ public void testJsonObjectWork() {
.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();
}
}
}
Expand Up @@ -25,6 +25,7 @@
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;
Expand All @@ -51,8 +52,13 @@ public void testdecodeResponseNull() throws Exception {
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
Expand Down
Expand Up @@ -52,7 +52,7 @@ public void testdecodeResponseNull() throws Exception {

ByteArrayInputStream is = new ByteArrayInputStream(new byte[] {});
result = pp.decodeResponse(is, resultType);
Assert.assertNull(result);
Assert.assertEquals(result, "");
}

@Test
Expand Down