[OLINGO-659] empty JSON input should result in Bad Request

Change-Id: Ib46d7454abcc405e3fa6a41fd3e17d9e947490c2

Signed-off-by: Christia Holzer <c.holzer@sap.com>

fit/src/test/java/org/apache/olingo/fit/tecsvc/client/ActionImportITCase.java

@@ -69,9 +69,12 @@ public class ActionImportITCase extends AbstractTecSvcITCase {
   }
 
   @Test
-  public void primitiveActionInvalidParameters() throws Exception {
+  public void primitiveCollectionActionInvalidParameters() throws Exception {
     try {
-      callAction("AIRTString", ClientProperty.class, buildParameterInt16(42), false);
+      callAction("AIRTCollStringTwoParam", ClientProperty.class,
+          Collections.singletonMap("ParameterInt16",
+              (ClientValue) getFactory().newPrimitiveValueBuilder().buildString("42")),
+          false);
       fail("Expected an ODataClientErrorException");
     } catch (ODataClientErrorException e) {
       assertEquals(HttpStatusCode.BAD_REQUEST.getStatusCode(), e.getStatusLine().getStatusCode());

lib/server-core/src/main/java/org/apache/olingo/server/core/deserializer/json/ODataJsonDeserializer.java

@@ -74,7 +74,7 @@ public class ODataJsonDeserializer implements ODataDeserializer {
 
   private static final String AN_IO_EXCEPTION_OCCURRED_MSG = "An IOException occurred";
   private static final String DUPLICATE_JSON_PROPERTY_DETECTED_MSG = "Duplicate json property detected";
-  private static final String AN_JSON_PARSE_EXCEPTION_OCCURRED_MSG = "An JsonParseException occurred";
+  private static final String AN_JSON_PARSE_EXCEPTION_OCCURRED_MSG = "A JsonParseException occurred";
   private static final String ODATA_ANNOTATION_MARKER = "@";
   private static final String ODATA_CONTROL_INFORMATION_PREFIX = "@odata.";
   private static final EdmPrimitiveType EDM_INT64 = EdmPrimitiveTypeFactory.getInstance(EdmPrimitiveTypeKind.Int64);
@@ -204,28 +204,25 @@ public class ODataJsonDeserializer implements ODataDeserializer {
       throws DeserializerException {
     try {
       ObjectNode tree = parseJsonTree(stream);
-      if (tree != null) {
+      Map<String, Parameter> parameters = consumeParameters(edmAction, tree);
-        Map<String, Parameter> parameters = consumeParameters(edmAction, tree);
 
-        final List<String> toRemove = new ArrayList<String>();
+      final List<String> toRemove = new ArrayList<String>();
-        Iterator<Entry<String, JsonNode>> fieldsIterator = tree.fields();
+      Iterator<Entry<String, JsonNode>> fieldsIterator = tree.fields();
-        while (fieldsIterator.hasNext()) {
+      while (fieldsIterator.hasNext()) {
-          Map.Entry<String, JsonNode> field = fieldsIterator.next();
+        Map.Entry<String, JsonNode> field = fieldsIterator.next();
 
-          if (field.getKey().contains(ODATA_CONTROL_INFORMATION_PREFIX)) {
+        if (field.getKey().contains(ODATA_CONTROL_INFORMATION_PREFIX)) {
-            // Control Information is ignored for requests as per specification chapter "4.5 Control Information"
+          // Control Information is ignored for requests as per specification chapter "4.5 Control Information"
-            toRemove.add(field.getKey());
+          toRemove.add(field.getKey());
-          } else if (field.getKey().contains(ODATA_ANNOTATION_MARKER)) {
+        } else if (field.getKey().contains(ODATA_ANNOTATION_MARKER)) {
-            throw new DeserializerException("Custom annotation with field name: " + field.getKey() + " not supported",
+          throw new DeserializerException("Custom annotation with field name: " + field.getKey() + " not supported",
-                DeserializerException.MessageKeys.NOT_IMPLEMENTED);
+              DeserializerException.MessageKeys.NOT_IMPLEMENTED);
-          }
         }
-        // remove here to avoid iterator issues.
-        tree.remove(toRemove);
-        assertJsonNodeIsEmpty(tree);
-        return DeserializerResultImpl.with().actionParameters(parameters).build();
       }
-      return DeserializerResultImpl.with().build();
+      // remove here to avoid iterator issues.
+      tree.remove(toRemove);
+      assertJsonNodeIsEmpty(tree);
+      return DeserializerResultImpl.with().actionParameters(parameters).build();
 
     } catch (final JsonParseException e) {
       throw new DeserializerException(AN_JSON_PARSE_EXCEPTION_OCCURRED_MSG, e,
@@ -239,11 +236,16 @@ public class ODataJsonDeserializer implements ODataDeserializer {
     }
   }
 
-  private ObjectNode parseJsonTree(final InputStream stream) throws IOException {
+  private ObjectNode parseJsonTree(final InputStream stream) throws IOException, DeserializerException {
     ObjectMapper objectMapper = new ObjectMapper();
     objectMapper.configure(DeserializationFeature.FAIL_ON_READING_DUP_TREE_KEY, true);
     JsonParser parser = new JsonFactory(objectMapper).createParser(stream);
-    return parser.getCodec().readTree(parser);
+    final ObjectNode tree = parser.getCodec().readTree(parser);
+    if (tree == null) {
+      throw new DeserializerException("Invalid JSON syntax.",
+          DeserializerException.MessageKeys.JSON_SYNTAX_EXCEPTION);
+    }
+    return tree;
   }
 
   private Map<String, Parameter> consumeParameters(final EdmAction edmAction, final ObjectNode node)
@@ -845,10 +847,7 @@ public class ODataJsonDeserializer implements ODataDeserializer {
   public DeserializerResult property(final InputStream stream, final EdmProperty edmProperty)
       throws DeserializerException {
     try {
-      ObjectMapper objectMapper = new ObjectMapper();
+      final ObjectNode tree = parseJsonTree(stream);
-      objectMapper.configure(DeserializationFeature.FAIL_ON_READING_DUP_TREE_KEY, true);
-      JsonParser parser = new JsonFactory(objectMapper).createParser(stream);
-      final ObjectNode tree = parser.getCodec().readTree(parser);
 
       final Property property;
       JsonNode jsonNode = tree.get(Constants.VALUE);
@@ -882,11 +881,8 @@ public class ODataJsonDeserializer implements ODataDeserializer {
   public DeserializerResult entityReferences(final InputStream stream) throws DeserializerException {
     try {
       ArrayList<URI> parsedValues = new ArrayList<URI>();
-      ObjectMapper objectMapper = new ObjectMapper();
+      final ObjectNode tree = parseJsonTree(stream);
-      objectMapper.configure(DeserializationFeature.FAIL_ON_READING_DUP_TREE_KEY, true);
+      final String key = Constants.JSON_ID;
-      JsonParser parser = new JsonFactory(objectMapper).createParser(stream);
-      final ObjectNode tree = parser.getCodec().readTree(parser);
-      final String key = "@odata.id";
       JsonNode jsonNode = tree.get(Constants.VALUE);
       if (jsonNode != null) {
         if (jsonNode.isArray()) {

lib/server-core/src/test/java/org/apache/olingo/server/core/deserializer/json/ODataJsonDeserializerBasicTest.java

@@ -174,9 +174,13 @@ public class ODataJsonDeserializerBasicTest {
 
     InputStream stream = new ByteArrayInputStream(entityString.getBytes());
     ODataDeserializer deserializer = OData.newInstance().createDeserializer(ContentType.JSON);
-    final List<URI> entityReferences = deserializer.entityReferences(stream).getEntityReferences();
+    deserializer.entityReferences(stream).getEntityReferences();
+  }
 
-    assertEquals(0, entityReferences.size());
+  @Test(expected = DeserializerException.class)
+  public void referencesNoContent() throws Exception {
+    OData.newInstance().createDeserializer(ContentType.JSON).entityReferences(
+        new ByteArrayInputStream(new byte[] {}));
   }
 
   @Test(expected = DeserializerException.class)

lib/server-tecsvc/src/main/java/org/apache/olingo/server/tecsvc/processor/TechnicalActionProcessor.java

@@ -18,12 +18,16 @@
  */
 package org.apache.olingo.server.tecsvc.processor;
 
+import java.io.InputStream;
+import java.util.Collections;
 import java.util.Locale;
+import java.util.Map;
 
 import org.apache.olingo.commons.api.data.ContextURL;
 import org.apache.olingo.commons.api.data.ContextURL.Builder;
 import org.apache.olingo.commons.api.data.ContextURL.Suffix;
 import org.apache.olingo.commons.api.data.EntityCollection;
+import org.apache.olingo.commons.api.data.Parameter;
 import org.apache.olingo.commons.api.data.Property;
 import org.apache.olingo.commons.api.edm.EdmAction;
 import org.apache.olingo.commons.api.edm.EdmComplexType;
@@ -38,7 +42,7 @@ import org.apache.olingo.server.api.ODataLibraryException;
 import org.apache.olingo.server.api.ODataRequest;
 import org.apache.olingo.server.api.ODataResponse;
 import org.apache.olingo.server.api.ServiceMetadata;
-import org.apache.olingo.server.api.deserializer.DeserializerResult;
+import org.apache.olingo.server.api.deserializer.DeserializerException;
 import org.apache.olingo.server.api.prefer.Preferences.Return;
 import org.apache.olingo.server.api.prefer.PreferencesApplied;
 import org.apache.olingo.server.api.processor.ActionComplexCollectionProcessor;
@@ -78,12 +82,9 @@ public class TechnicalActionProcessor extends TechnicalProcessor
     blockBoundActions(uriInfo);
     final EdmAction action = ((UriResourceAction) uriInfo.asUriInfoResource().getUriResourceParts().get(0))
         .getAction();
-
+    final Map<String, Parameter> parameters = readParameters(action, request.getBody(), requestFormat);
-    DeserializerResult deserializerResult =
-        odata.createDeserializer(requestFormat).actionParameters(request.getBody(), action);
-
     EntityCollection collection =
-        dataProvider.processActionEntityCollection(action.getName(), deserializerResult.getActionParameters());
+        dataProvider.processActionEntityCollection(action.getName(), parameters);
 
     // Collections must never be null.
     // Not nullable return types must not contain a null value.
@@ -123,11 +124,9 @@ public class TechnicalActionProcessor extends TechnicalProcessor
     final EdmEntitySet edmEntitySet = getEdmEntitySet(uriInfo.asUriInfoResource());
     final EdmEntityType type = (EdmEntityType) action.getReturnType().getType();
 
-    final DeserializerResult deserializerResult =
+    final Map<String, Parameter> parameters = readParameters(action, request.getBody(), requestFormat);
-        odata.createDeserializer(requestFormat).actionParameters(request.getBody(), action);
-
     final EntityActionResult entityResult =
-        dataProvider.processActionEntity(action.getName(), deserializerResult.getActionParameters());
+        dataProvider.processActionEntity(action.getName(), parameters);
     if (entityResult == null || entityResult.getEntity() == null) {
       if (action.getReturnType().isNullable()) {
         response.setStatusCode(HttpStatusCode.NO_CONTENT.getStatusCode());
@@ -178,11 +177,9 @@ public class TechnicalActionProcessor extends TechnicalProcessor
     blockBoundActions(uriInfo);
     final EdmAction action = ((UriResourceAction) uriInfo.asUriInfoResource().getUriResourceParts().get(0))
         .getAction();
-    DeserializerResult deserializerResult =
+    final Map<String, Parameter> parameters = readParameters(action, request.getBody(), requestFormat);
-        odata.createDeserializer(requestFormat).actionParameters(request.getBody(), action);
-
     Property property =
-        dataProvider.processActionPrimitiveCollection(action.getName(), deserializerResult.getActionParameters());
+        dataProvider.processActionPrimitiveCollection(action.getName(), parameters);
 
     if (property == null || property.isNull()) {
       // Collection Propertys must never be null
@@ -220,10 +217,8 @@ public class TechnicalActionProcessor extends TechnicalProcessor
     blockBoundActions(uriInfo);
     final EdmAction action = ((UriResourceAction) uriInfo.asUriInfoResource().getUriResourceParts().get(0))
         .getAction();
-    DeserializerResult deserializerResult =
+    final Map<String, Parameter> parameters = readParameters(action, request.getBody(), requestFormat);
-        odata.createDeserializer(requestFormat).actionParameters(request.getBody(), action);
+    Property property = dataProvider.processActionPrimitive(action.getName(), parameters);
-
-    Property property = dataProvider.processActionPrimitive(action.getName(), deserializerResult.getActionParameters());
     EdmPrimitiveType type = (EdmPrimitiveType) action.getReturnType().getType();
     if (property == null || property.isNull()) {
       if (action.getReturnType().isNullable()) {
@@ -260,11 +255,9 @@ public class TechnicalActionProcessor extends TechnicalProcessor
     blockBoundActions(uriInfo);
     final EdmAction action = ((UriResourceAction) uriInfo.asUriInfoResource().getUriResourceParts().get(0))
         .getAction();
-    DeserializerResult deserializerResult =
+    final Map<String, Parameter> parameters = readParameters(action, request.getBody(), requestFormat);
-        odata.createDeserializer(requestFormat).actionParameters(request.getBody(), action);
-
     Property property =
-        dataProvider.processActionComplexCollection(action.getName(), deserializerResult.getActionParameters());
+        dataProvider.processActionComplexCollection(action.getName(), parameters);
 
     if (property == null || property.isNull()) {
       // Collection Propertys must never be null
@@ -301,10 +294,8 @@ public class TechnicalActionProcessor extends TechnicalProcessor
     blockBoundActions(uriInfo);
     final EdmAction action = ((UriResourceAction) uriInfo.asUriInfoResource().getUriResourceParts().get(0))
         .getAction();
-    DeserializerResult deserializerResult =
+    final Map<String, Parameter> parameters = readParameters(action, request.getBody(), requestFormat);
-        odata.createDeserializer(requestFormat).actionParameters(request.getBody(), action);
+    Property property = dataProvider.processActionComplex(action.getName(), parameters);
-
-    Property property = dataProvider.processActionComplex(action.getName(), deserializerResult.getActionParameters());
     EdmComplexType type = (EdmComplexType) action.getReturnType().getType();
     if (property == null || property.isNull()) {
       if (action.getReturnType().isNullable()) {
@@ -340,11 +331,17 @@ public class TechnicalActionProcessor extends TechnicalProcessor
     final UriResourceAction resource =
         ((UriResourceAction) uriInfo.getUriResourceParts().get(uriInfo.getUriResourceParts().size() - 1));
     final EdmAction action = resource.getAction();
+    readParameters(action, request.getBody(), requestFormat);
+    response.setStatusCode(HttpStatusCode.NO_CONTENT.getStatusCode());
+  }
+
+  private Map<String, Parameter> readParameters(final EdmAction action, final InputStream body, final ContentType requestFormat)
+      throws ODataApplicationException, DeserializerException {
     if (action.getParameterNames().size() - (action.isBound() ? 1 : 0) > 0) {
       checkRequestFormat(requestFormat);
-      odata.createDeserializer(requestFormat).actionParameters(request.getBody(), action);
+      return odata.createDeserializer(requestFormat).actionParameters(body, action).getActionParameters();
     }
-    response.setStatusCode(HttpStatusCode.NO_CONTENT.getStatusCode());
+    return Collections.<String, Parameter> emptyMap();
   }
 
   private ContextURL getContextUrl(final EdmEntitySet entitySet, final EdmEntityType entityType,

lib/server-test/src/test/java/org/apache/olingo/server/core/deserializer/json/AbstractODataDeserializerTest.java

@@ -31,7 +31,7 @@ import org.apache.olingo.server.tecsvc.provider.EdmTechProvider;
 public class AbstractODataDeserializerTest {
   protected static final ContentType CONTENT_TYPE_JSON = ContentType.JSON;
   protected static final ContentType CONTENT_TYPE_JSON_IEEE754Compatible = 
-      ContentType.parse("application/json;odata.format=minimal;IEEE754Compatible=true");
+      ContentType.create(ContentType.JSON, ContentType.PARAMETER_IEEE754_COMPATIBLE, "true");
   
   protected static final Edm edm = OData.newInstance().createServiceMetadata(
       new EdmTechProvider(), Collections.<EdmxReference> emptyList()).getEdm();

lib/server-test/src/test/java/org/apache/olingo/server/core/deserializer/json/ODataDeserializerEntityCollectionTest.java

@@ -221,6 +221,13 @@ public class ODataDeserializerEntityCollectionTest extends AbstractODataDeserial
     }
   }
 
+  @Test(expected = DeserializerException.class)
+  public void emptyInput() throws Exception {
+    OData.newInstance().createDeserializer(CONTENT_TYPE_JSON).entityCollection(
+        new ByteArrayInputStream(new byte[] {}),
+        edm.getEntityType(new FullQualifiedName("Namespace1_Alias", "ETAllPrim")));
+  }
+
   @Test(expected = DeserializerException.class)
   public void unknownContentInCollection() throws Exception {
     String entityCollectionString = "{\"value\" : [],"

lib/server-test/src/test/java/org/apache/olingo/server/core/deserializer/json/ODataJsonDeserializerActionParametersTest.java

@@ -32,18 +32,15 @@ import java.util.List;
 import java.util.Map;
 
 import org.apache.olingo.commons.api.data.ComplexValue;
-import org.apache.olingo.commons.api.data.Entity;
 import org.apache.olingo.commons.api.data.Parameter;
 import org.apache.olingo.commons.api.edm.EdmAction;
 import org.apache.olingo.commons.api.edm.FullQualifiedName;
 import org.apache.olingo.commons.api.edm.provider.CsdlAction;
 import org.apache.olingo.commons.api.edm.provider.CsdlComplexType;
-import org.apache.olingo.commons.api.edm.provider.CsdlEntityType;
 import org.apache.olingo.commons.api.edm.provider.CsdlParameter;
 import org.apache.olingo.commons.api.edm.provider.CsdlProperty;
 import org.apache.olingo.commons.core.edm.EdmActionImpl;
 import org.apache.olingo.commons.core.edm.EdmComplexTypeImpl;
-import org.apache.olingo.commons.core.edm.EdmEntityTypeImpl;
 import org.apache.olingo.commons.core.edm.EdmProviderImpl;
 import org.apache.olingo.server.api.OData;
 import org.apache.olingo.server.api.deserializer.DeserializerException;
@@ -233,7 +230,12 @@ public class ODataJsonDeserializerActionParametersTest extends AbstractODataDese
     assertNotNull(parameter);
     assertEquals(null, parameter.getValue());
   }
-  
+
+  @Test(expected = DeserializerException.class)
+  public void noContent() throws Exception {
+    deserialize("", "BAETAllPrimRT", "ETAllPrim");
+  }
+
   @Test(expected = DeserializerException.class)
   public void bindingParameter() throws Exception {
     deserialize("{\"ParameterETAllPrim\":{\"PropertyInt16\":42}}", "BAETAllPrimRT", "ETAllPrim");

lib/server-test/src/test/java/org/apache/olingo/server/core/deserializer/json/ODataJsonDeserializerEntityTest.java

@@ -686,6 +686,13 @@ public class ODataJsonDeserializerEntityTest extends AbstractODataDeserializerTe
 
   // ---------------------------------- Negative Tests -----------------------------------------------------------
 
+  @Test(expected = DeserializerException.class)
+  public void emptyInput() throws Exception {
+    OData.newInstance().createDeserializer(CONTENT_TYPE_JSON).entity(
+        new ByteArrayInputStream(new byte[] {}),
+        edm.getEntityType(new FullQualifiedName("Namespace1_Alias", "ETAllPrim")));
+  }
+
   @Test(expected = DeserializerException.class)
   public void etAllPrimWithInvalidNullValue() throws Exception {
     String entityString =