Bug 1100 - Invoking an RPC with no input should not throw 500 when expected

Patch Set 3:
*Modified the Json/Xml readers to not throw an except if the content is empty
*Moved the validation of non empty object into restconf where the knowledge
of what is expected exists.

Change-Id: I4e689770d1a6843e99ce498bd12cecc5e1817eaa
Signed-off-by: Devin Avery <devin.avery@brocade.com>

diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/JsonReader.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/JsonReader.java
index 4d9958ee6b4720b66fe5fc2a286e754630274e20..cdea81f2b79676e1e332d4826df24f93c4a5f5be 100644 (file)
--- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/JsonReader.java
+++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/JsonReader.java
@@ -31,6 +31,12 @@ class JsonReader {
         JsonParser parser = new JsonParser();
 
         JsonElement rootElement = parser.parse(new InputStreamReader(entityStream));
+        if( rootElement.isJsonNull() )
+        {
+            //no content, so return null to indicate no input
+            return null;
+        }
+
         if (!rootElement.isJsonObject()) {
             throw new UnsupportedFormatException("Root element of Json has to be Object");
         }

diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/XmlReader.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/XmlReader.java
index a75f6b4a85f8277bffa57ad6034abe6bc323f2a0..2965ae7209761c367ed11c9e9e88ea00e6e83e93 100644 (file)
--- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/XmlReader.java
+++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/XmlReader.java
@@ -9,6 +9,8 @@ package org.opendaylight.controller.sal.rest.impl;
 
 import static com.google.common.base.Preconditions.checkArgument;
 
+import java.io.BufferedInputStream;
+import java.io.IOException;
 import java.io.InputStream;
 import java.net.URI;
 import java.util.Stack;
@@ -32,7 +34,17 @@ public class XmlReader {
     private final static XMLInputFactory xmlInputFactory = XMLInputFactory.newInstance();
     private XMLEventReader eventReader;
 
-    public CompositeNodeWrapper read(InputStream entityStream) throws XMLStreamException, UnsupportedFormatException {
+    public CompositeNodeWrapper read(InputStream entityStream) throws XMLStreamException,
+                                                                      UnsupportedFormatException,
+                                                                      IOException {
+        //Get an XML stream which can be marked, and reset, so we can check and see if there is
+        //any content being provided.
+        entityStream = getMarkableStream(entityStream);
+
+        if( isInputStreamEmpty( entityStream ) ) {
+            return null;
+        }
+
         eventReader = xmlInputFactory.createXMLEventReader(entityStream);
 
         if (eventReader.hasNext()) {
@@ -91,6 +103,31 @@ public class XmlReader {
         return root;
     }
 
+    /**
+     * If the input stream is not markable, then it wraps the input stream with a buffered stream,
+     * which is mark able. That way we can check if the stream is empty safely.
+     * @param entityStream
+     * @return
+     */
+    private InputStream getMarkableStream(InputStream entityStream) {
+        if( !entityStream.markSupported() )
+        {
+            entityStream = new BufferedInputStream( entityStream );
+        }
+        return entityStream;
+    }
+
+    private boolean isInputStreamEmpty(InputStream entityStream)
+            throws IOException {
+        boolean isEmpty = false;
+        entityStream.mark( 1 );
+        if( entityStream.read() == -1 ){
+            isEmpty = true;
+        }
+        entityStream.reset();
+        return isEmpty;
+    }
+
     private boolean isSimpleNodeEvent(final XMLEvent event) throws XMLStreamException {
         checkArgument(event != null, "XML Event cannot be NULL!");
         if (event.isStartElement()) {

diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java
index ad682bc8291d50e52d674607e30142009bac459e..c0ce90e15dde780a18e40e0e216c78059c9183fc 100644 (file)
--- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java
+++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java
@@ -8,13 +8,6 @@
  */
 package org.opendaylight.controller.sal.restconf.impl;
 
-import com.google.common.base.Objects;
-import com.google.common.base.Preconditions;
-import com.google.common.base.Splitter;
-import com.google.common.base.Strings;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
-
 import java.net.URI;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
@@ -38,21 +31,11 @@ import org.opendaylight.controller.md.sal.common.api.TransactionStatus;
 import org.opendaylight.controller.sal.core.api.mount.MountInstance;
 import org.opendaylight.controller.sal.rest.api.Draft02;
 import org.opendaylight.controller.sal.rest.api.RestconfService;
+import org.opendaylight.controller.sal.restconf.impl.RestconfError.ErrorTag;
+import org.opendaylight.controller.sal.restconf.impl.RestconfError.ErrorType;
 import org.opendaylight.controller.sal.restconf.rpc.impl.BrokerRpcExecutor;
 import org.opendaylight.controller.sal.restconf.rpc.impl.MountPointRpcExecutor;
 import org.opendaylight.controller.sal.restconf.rpc.impl.RpcExecutor;
-import org.opendaylight.controller.sal.restconf.impl.BrokerFacade;
-import org.opendaylight.controller.sal.restconf.impl.CompositeNodeWrapper;
-import org.opendaylight.controller.sal.restconf.impl.ControllerContext;
-import org.opendaylight.controller.sal.restconf.impl.EmptyNodeWrapper;
-import org.opendaylight.controller.sal.restconf.impl.IdentityValuesDTO;
-import org.opendaylight.controller.sal.restconf.impl.InstanceIdWithSchemaNode;
-import org.opendaylight.controller.sal.restconf.impl.NodeWrapper;
-import org.opendaylight.controller.sal.restconf.impl.RestCodec;
-import org.opendaylight.controller.sal.restconf.impl.SimpleNodeWrapper;
-import org.opendaylight.controller.sal.restconf.impl.StructuredData;
-import org.opendaylight.controller.sal.restconf.impl.RestconfError.ErrorTag;
-import org.opendaylight.controller.sal.restconf.impl.RestconfError.ErrorType;
 import org.opendaylight.controller.sal.streams.listeners.ListenerAdapter;
 import org.opendaylight.controller.sal.streams.listeners.Notificator;
 import org.opendaylight.controller.sal.streams.websockets.WebSocketServer;
@@ -84,6 +67,13 @@ import org.opendaylight.yangtools.yang.model.util.EmptyType;
 import org.opendaylight.yangtools.yang.parser.builder.impl.ContainerSchemaNodeBuilder;
 import org.opendaylight.yangtools.yang.parser.builder.impl.LeafSchemaNodeBuilder;
 
+import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
+import com.google.common.base.Strings;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+
 public class RestconfImpl implements RestconfService {
     private final static RestconfImpl INSTANCE = new RestconfImpl();
 
@@ -401,13 +391,36 @@ public class RestconfImpl implements RestconfService {
         URI rpcNamespace = rpcName.getNamespace();
         if (Objects.equal(rpcNamespace.toString(), SAL_REMOTE_NAMESPACE) &&
             Objects.equal(rpcName.getLocalName(), SAL_REMOTE_RPC_SUBSRCIBE)) {
-
             return invokeSalRemoteRpcSubscribeRPC(payload, rpc.getRpcDefinition());
         }
 
+        validateInput( rpc.getRpcDefinition().getInput(), payload );
+
         return callRpc(rpc, payload);
     }
 
+    private void validateInput(DataSchemaNode inputSchema, CompositeNode payload) {
+        if( inputSchema != null && payload == null )
+        {
+            //expected a non null payload
+            throw new RestconfDocumentedException( "Input is required.",
+                                                   ErrorType.PROTOCOL,
+                                                   ErrorTag.MALFORMED_MESSAGE );
+        }
+        else if( inputSchema == null && payload != null )
+        {
+            //did not expect any input
+            throw new RestconfDocumentedException( "No input expected.",
+                                                   ErrorType.PROTOCOL,
+                                                   ErrorTag.MALFORMED_MESSAGE );
+        }
+        //else
+        //{
+            //TODO: Validate "mandatory" and "config" values here??? Or should those be
+        // validate in a more central location inside MD-SAL core.
+        //}
+    }
+
     private StructuredData invokeSalRemoteRpcSubscribeRPC(final CompositeNode payload,
                                                           final RpcDefinition rpc) {
         final CompositeNode value = this.normalizeNode(payload, rpc.getInput(), null);
@@ -455,8 +468,7 @@ public class RestconfImpl implements RestconfService {
             throw new RestconfDocumentedException(
                     "Content must be empty.", ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE );
         }
-        final RpcExecutor rpc = resolveIdentifierInInvokeRpc(identifier);
-        return callRpc(rpc, null);
+        return invokeRpc( identifier, (CompositeNode)null );
     }
 
     private RpcExecutor resolveIdentifierInInvokeRpc(final String identifier) {
@@ -586,6 +598,9 @@ public class RestconfImpl implements RestconfService {
     @Override
     public Response updateConfigurationData(final String identifier, final CompositeNode payload) {
         final InstanceIdWithSchemaNode iiWithData = this.controllerContext.toInstanceIdentifier(identifier);
+
+        validateInput(iiWithData.getSchemaNode(), payload);
+
         MountInstance mountPoint = iiWithData.getMountPoint();
         final CompositeNode value = this.normalizeNode(payload, iiWithData.getSchemaNode(), mountPoint);
         RpcResult<TransactionStatus> status = null;
@@ -610,6 +625,12 @@ public class RestconfImpl implements RestconfService {
 
     @Override
     public Response createConfigurationData(final String identifier, final CompositeNode payload) {
+        if( payload == null ) {
+            throw new RestconfDocumentedException( "Input is required.",
+                    ErrorType.PROTOCOL,
+                    ErrorTag.MALFORMED_MESSAGE );
+        }
+
         URI payloadNS = this.namespace(payload);
         if (payloadNS == null) {
             throw new RestconfDocumentedException(
@@ -685,6 +706,12 @@ public class RestconfImpl implements RestconfService {
 
     @Override
     public Response createConfigurationData(final CompositeNode payload) {
+        if( payload == null ) {
+            throw new RestconfDocumentedException( "Input is required.",
+                    ErrorType.PROTOCOL,
+                    ErrorTag.MALFORMED_MESSAGE );
+        }
+
         URI payloadNS = this.namespace(payload);
         if (payloadNS == null) {
             throw new RestconfDocumentedException(

diff --git a/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/json/to/cnsn/test/JsonToCnSnTest.java b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/json/to/cnsn/test/JsonToCnSnTest.java
index 3c70cca0f87806d28b73273423fa7b546b1631c6..47e329cc3ef11a4cdfc8842c9a1285a445989517 100644 (file)
--- a/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/json/to/cnsn/test/JsonToCnSnTest.java
+++ b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/json/to/cnsn/test/JsonToCnSnTest.java
@@ -9,8 +9,11 @@ package org.opendaylight.controller.sal.restconf.impl.json.to.cnsn.test;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -176,6 +179,14 @@ public class JsonToCnSnTest {
 
     }
 
+    @Test
+    public void testJsonBlankInput() throws Exception{
+        InputStream inputStream = new ByteArrayInputStream( "".getBytes() );
+        CompositeNode compositeNode =
+                JsonToCompositeNodeProvider.INSTANCE.readFrom(null, null, null, null, null, inputStream);
+        assertNull( compositeNode );
+    }
+
     /**
      * Tests whether namespace <b>stay unchanged</b> if concrete values are
      * present in composite or simple node and if the method for update is

diff --git a/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/InvokeRpcMethodTest.java b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/InvokeRpcMethodTest.java
index c0c86c3f25384fdbd3ea979debef62fa7f1512ab..910ca8e20aab453d02b4e320b0493775666d3575 100644 (file)
--- a/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/InvokeRpcMethodTest.java
+++ b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/InvokeRpcMethodTest.java
@@ -308,7 +308,7 @@ public class InvokeRpcMethodTest {
         ListenableFuture<RpcResult<CompositeNode>> mockListener = mock( ListenableFuture.class );
         when( mockListener.get() ).thenReturn( rpcResult );
 
-        QName cancelToastQName = QName.create( "cancelToast" );
+        QName cancelToastQName = QName.create( "namespace", "2014-05-28", "cancelToast" );
 
         RpcDefinition mockRpc = mock( RpcDefinition.class );
         when( mockRpc.getQName() ).thenReturn( cancelToastQName );

diff --git a/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestPostOperationTest.java b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestPostOperationTest.java
index ce460fe4746d9137f0775f6536297e5cc841ab44..cfbc9fdb767c2c9c8fb8bc7e8d4954f5e5e210e8 100644 (file)
--- a/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestPostOperationTest.java
+++ b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestPostOperationTest.java
@@ -149,6 +149,8 @@ public class RestPostOperationTest extends JerseyTest {
 
         mockCommitConfigurationDataPostMethod(TransactionStatus.FAILED);
         assertEquals(500, post(uri, MediaType.APPLICATION_XML, xmlDataInterfaceAbsolutePath));
+
+        assertEquals( 400, post(uri, MediaType.APPLICATION_JSON, "" ));
     }
 
     @Test
@@ -172,6 +174,8 @@ public class RestPostOperationTest extends JerseyTest {
         assertEquals(204, post(uri, Draft02.MediaTypes.DATA + XML, xmlData4));
         uri = "/config/ietf-interfaces:interfaces/interface/0/yang-ext:mount/test-module:cont";
         assertEquals(204, post(uri, Draft02.MediaTypes.DATA + XML, xmlData3));
+
+        assertEquals( 400, post(uri, MediaType.APPLICATION_JSON, "" ));
     }
 
     private void mockInvokeRpc(CompositeNode result, boolean sucessful) {

diff --git a/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestPutOperationTest.java b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestPutOperationTest.java
index 3af2945526466fabf3e9c139b79eb4134107752f..77b39b73529dca04ee8b6279b824c01a4361bf25 100644 (file)
--- a/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestPutOperationTest.java
+++ b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestPutOperationTest.java
@@ -22,6 +22,7 @@ import java.util.concurrent.Future;
 import javax.ws.rs.client.Entity;
 import javax.ws.rs.core.Application;
 import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
 
 import org.glassfish.jersey.server.ResourceConfig;
 import org.glassfish.jersey.test.JerseyTest;
@@ -31,6 +32,7 @@ import org.opendaylight.controller.md.sal.common.api.TransactionStatus;
 import org.opendaylight.controller.sal.core.api.mount.MountInstance;
 import org.opendaylight.controller.sal.core.api.mount.MountService;
 import org.opendaylight.controller.sal.rest.impl.JsonToCompositeNodeProvider;
+import org.opendaylight.controller.sal.rest.impl.RestconfDocumentedExceptionMapper;
 import org.opendaylight.controller.sal.rest.impl.StructuredDataToJsonProvider;
 import org.opendaylight.controller.sal.rest.impl.StructuredDataToXmlProvider;
 import org.opendaylight.controller.sal.rest.impl.XmlToCompositeNodeProvider;
@@ -86,6 +88,7 @@ public class RestPutOperationTest extends JerseyTest {
         resourceConfig = resourceConfig.registerInstances(restconfImpl, StructuredDataToXmlProvider.INSTANCE,
                 StructuredDataToJsonProvider.INSTANCE, XmlToCompositeNodeProvider.INSTANCE,
                 JsonToCompositeNodeProvider.INSTANCE);
+        resourceConfig.registerClasses( RestconfDocumentedExceptionMapper.class );
         return resourceConfig;
     }
 
@@ -100,6 +103,15 @@ public class RestPutOperationTest extends JerseyTest {
 
         mockCommitConfigurationDataPutMethod(TransactionStatus.FAILED);
         assertEquals(500, put(uri, MediaType.APPLICATION_XML, xmlData));
+
+        assertEquals( 400, put(uri, MediaType.APPLICATION_JSON, "" ));
+    }
+
+    @Test
+    public void putConfigStatusCodesEmptyBody() throws UnsupportedEncodingException {
+        String uri = "/config/ietf-interfaces:interfaces/interface/eth0";
+        Response resp = target(uri).request( MediaType.APPLICATION_JSON).put(Entity.entity( "", MediaType.APPLICATION_JSON));
+        assertEquals( 400, put(uri, MediaType.APPLICATION_JSON, "" ));
     }
 
     @Test

diff --git a/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/xml/to/cnsn/test/XmlToCnSnTest.java b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/xml/to/cnsn/test/XmlToCnSnTest.java
index 5008d28bbfb26ea0e8d9ef8ab2b1814e8736671d..5cda4a7f52014dc4a9ab3572d9073877ec12e74e 100644 (file)
--- a/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/xml/to/cnsn/test/XmlToCnSnTest.java
+++ b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/xml/to/cnsn/test/XmlToCnSnTest.java
@@ -9,8 +9,12 @@ package org.opendaylight.controller.sal.restconf.impl.xml.to.cnsn.test;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.opendaylight.controller.sal.rest.impl.XmlToCompositeNodeProvider;
@@ -52,4 +56,27 @@ public class XmlToCnSnTest extends YangAndXmlAndDataSchemaLoader {
         assertEquals("121", lf2.getValue());
     }
 
+    @Test
+    public void testXmlBlankInput() throws Exception{
+        InputStream inputStream = new ByteArrayInputStream( "".getBytes() );
+        CompositeNode compositeNode =
+                XmlToCompositeNodeProvider.INSTANCE.readFrom(null, null, null, null, null, inputStream);
+
+        assertNull( compositeNode );
+    }
+
+    @Test
+    public void testXmlBlankInputUnmarkableStream() throws Exception{
+        InputStream inputStream = new ByteArrayInputStream( "".getBytes() ){
+            @Override
+            public boolean markSupported() {
+                return false;
+            }
+        };
+        CompositeNode compositeNode =
+                XmlToCompositeNodeProvider.INSTANCE.readFrom(null, null, null, null, null, inputStream);
+
+        assertNull( compositeNode );
+    }
+
 }