Add validation for fcr:import

- Improve error responses Resolves: https://www.pivotaltracker.com/story/show/82429414
fcrepo · Nov 18, 2014 · 024b9b3 · 024b9b3
1 parent cd2d11c
commit 024b9b3
Show file tree

Hide file tree

Showing 13 changed files with 253 additions and 18 deletions.
diff --git a/fcrepo-http-api/src/main/java/org/fcrepo/http/api/FedoraImport.java b/fcrepo-http-api/src/main/java/org/fcrepo/http/api/FedoraImport.java
@@ -39,6 +39,7 @@
 import org.fcrepo.http.commons.domain.ContentLocation;
 import org.fcrepo.kernel.exception.InvalidChecksumException;
 import org.fcrepo.kernel.exception.RepositoryRuntimeException;
+import org.fcrepo.serialization.InvalidSerializationFormatException;
 import org.fcrepo.serialization.SerializerUtil;
 import org.slf4j.Logger;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -93,6 +94,8 @@ public Response importObject(@PathParam("path") final String externalPath,
             return status(CONFLICT).entity("Item already exists").build();
         } catch (final RepositoryException e) {
             throw new RepositoryRuntimeException(e);
+        } catch (InvalidSerializationFormatException e) {
+            throw new RepositoryRuntimeException(e);
         }
     }
 

diff --git a/fcrepo-http-api/src/test/java/org/fcrepo/integration/http/api/FedoraExportIT.java b/fcrepo-http-api/src/test/java/org/fcrepo/integration/http/api/FedoraExportIT.java
@@ -214,4 +214,26 @@ public void shouldExportObjectRecurse() throws IOException {
         assertEquals(200, response.getStatusLine().getStatusCode());
         assertTrue(EntityUtils.toString(response.getEntity()).indexOf("sv:name=\"" + childName + "\"") > 0);
     }
+
+    @Test
+    public void importNonJCRXMLShouldFail() throws IOException {
+        final HttpPost importMethod = new HttpPost(serverAddress + "fcr:import");
+        importMethod.setEntity(new StringEntity("<test><this></this></test>"));
+        assertEquals("Should not have been able to import non JCR/XML.", 400, getStatus(importMethod));
+    }
+
+    @Test
+    public void importMalformedXMLShouldFail() throws IOException {
+        final HttpPost importMethod = new HttpPost(serverAddress + "fcr:import");
+        importMethod.setEntity(new StringEntity("this isn't xml at all."));
+        assertEquals("Should not have been able to import malformed XML.", 400, getStatus(importMethod));
+    }
+
+    @Test
+    public void importNonsensicalJCRXMLShouldFail() throws IOException {
+        final HttpPost importMethod = new HttpPost(serverAddress + "fcr:import");
+        importMethod.setEntity(
+                new StringEntity("<sv:value xmlns:sv=\"http://www.jcp.org/jcr/sv/1.0\">just a value?</sv:value>"));
+        assertEquals("Should not have been able to import nonsensical JCR/XML..", 400, getStatus(importMethod));
+    }
 }
diff --git a/.../org/fcrepo/http/commons/exceptionhandlers/InvalidSerializationFormatExceptionMapper.java b/.../org/fcrepo/http/commons/exceptionhandlers/InvalidSerializationFormatExceptionMapper.java
@@ -0,0 +1,54 @@
+/**
+ * Copyright 2014 DuraSpace, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.fcrepo.http.commons.exceptionhandlers;
+
+import org.fcrepo.serialization.InvalidSerializationFormatException;
+import org.slf4j.Logger;
+
+import javax.ws.rs.core.Response;
+import javax.ws.rs.ext.ExceptionMapper;
+import javax.ws.rs.ext.Provider;
+
+import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
+import static javax.ws.rs.core.Response.status;
+import static org.slf4j.LoggerFactory.getLogger;
+
+/**
+ *  Translate InvalidSerializationFormatException errors into reasonable
+ *  HTTP error codes
+ *
+ * @author md5wz
+ * @since 11/17/14
+ */
+@Provider
+public class InvalidSerializationFormatExceptionMapper implements
+        ExceptionMapper<InvalidSerializationFormatException> {
+
+    private static final Logger LOGGER =
+        getLogger(InvalidSerializationFormatExceptionMapper.class);
+
+    @Override
+    public Response toResponse(final InvalidSerializationFormatException e) {
+        if (LOGGER.isDebugEnabled()) {
+            LOGGER.debug("InvalidSerializationFormatException intercepted by InvalidSerializationFormatExceptionMapper"
+                    + (e.getMessage() != null ? ": " + e.getMessage() : "."), e);
+        } else {
+            LOGGER.info("InvalidSerializationFormatException intercepted by InvalidSerializationFormatExceptionMapper"
+                    + (e.getMessage() != null ? ": " + e.getMessage() : "."));
+        }
+        return status(BAD_REQUEST).entity(e.getMessage()).build();
+    }
+}
diff --git a/fcrepo-kernel-impl/pom.xml b/fcrepo-kernel-impl/pom.xml
@@ -101,7 +101,6 @@
     <dependency>
       <groupId>commons-io</groupId>
       <artifactId>commons-io</artifactId>
-      <version>2.4</version>
     </dependency>
     <dependency>
       <groupId>org.infinispan</groupId>

diff --git a/fcrepo-serialization/pom.xml b/fcrepo-serialization/pom.xml
@@ -24,6 +24,16 @@
       <artifactId>spring-context</artifactId>
     </dependency>
 
+    <dependency>
+      <groupId>xml-apis</groupId>
+      <artifactId>xml-apis</artifactId>
+    </dependency>
+
+    <dependency>
+      <groupId>commons-io</groupId>
+      <artifactId>commons-io</artifactId>
+    </dependency>
+
     <!-- test gear -->
     <dependency>
       <groupId>junit</groupId>
@@ -34,6 +44,7 @@
       <groupId>org.mockito</groupId>
       <artifactId>mockito-core</artifactId>
     </dependency>
+
   </dependencies>
 
   <build>

diff --git a/fcrepo-serialization/src/main/java/org/fcrepo/serialization/BaseFedoraObjectSerializer.java b/fcrepo-serialization/src/main/java/org/fcrepo/serialization/BaseFedoraObjectSerializer.java
@@ -47,6 +47,6 @@ public abstract void serialize(final FedoraResource obj,
     @Override
     public abstract void deserialize(final Session session, final String path,
             final InputStream stream) throws IOException, RepositoryException,
-        InvalidChecksumException;
+            InvalidChecksumException, InvalidSerializationFormatException;
 
 }
diff --git a/fcrepo-serialization/src/main/java/org/fcrepo/serialization/FedoraObjectSerializer.java b/fcrepo-serialization/src/main/java/org/fcrepo/serialization/FedoraObjectSerializer.java
@@ -75,9 +75,10 @@ void serialize(final FedoraResource obj, final OutputStream out, final boolean s
      * @throws IOException
      * @throws RepositoryException
      * @throws InvalidChecksumException
+     * @throws org.fcrepo.serialization.InvalidSerializationFormatException
      */
     void deserialize(final Session session, final String path,
             final InputStream stream) throws IOException, RepositoryException,
-        InvalidChecksumException;
+            InvalidChecksumException, InvalidSerializationFormatException;
 
 }
diff --git a/...alization/src/main/java/org/fcrepo/serialization/InvalidSerializationFormatException.java b/...alization/src/main/java/org/fcrepo/serialization/InvalidSerializationFormatException.java
@@ -0,0 +1,37 @@
+/**
+ * Copyright 2014 DuraSpace, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.fcrepo.serialization;
+
+/**
+ * Exception thrown when during deserialization it becomes obvious that the
+ * InputStream is not in the expected format.
+ *
+ * @author md5wz
+ * @since November 2014
+ */
+public class InvalidSerializationFormatException extends Exception {
+
+    private static final long serialVersionUID = 1L;
+
+    /**
+     * Exception with message
+     * @param message
+     */
+    public InvalidSerializationFormatException(final String message) {
+        super(message);
+    }
+
+}
diff --git a/fcrepo-serialization/src/main/java/org/fcrepo/serialization/JcrXmlSerializer.java b/fcrepo-serialization/src/main/java/org/fcrepo/serialization/JcrXmlSerializer.java
@@ -15,17 +15,27 @@
  */
 package org.fcrepo.serialization;
 
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
+import org.apache.commons.io.IOUtils;
+import org.fcrepo.kernel.models.FedoraResource;
+import org.springframework.stereotype.Component;
 
 import javax.jcr.ImportUUIDBehavior;
 import javax.jcr.Node;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
-
-import org.fcrepo.kernel.models.FedoraResource;
-import org.springframework.stereotype.Component;
+import javax.xml.namespace.QName;
+import javax.xml.stream.XMLEventReader;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.events.StartElement;
+import javax.xml.stream.events.XMLEvent;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
 
 /**
  * Serialize a FedoraObject using the modeshape-provided JCR/XML format
@@ -68,11 +78,78 @@ public void serialize(final FedoraResource obj,
 
     @Override
     public void deserialize(final Session session, final String path,
-            final InputStream stream) throws RepositoryException, IOException {
+            final InputStream stream) throws RepositoryException, IOException, InvalidSerializationFormatException {
 
-        session.importXML(path, stream,
-                ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW);
+        final File temp = File.createTempFile("fcrepo-unsanitized-input", ".xml");
+        final FileOutputStream fos = new FileOutputStream(temp);
+        try {
+            IOUtils.copy(stream, fos);
+        } finally {
+            IOUtils.closeQuietly(stream);
+            IOUtils.closeQuietly(fos);
+        }
+        validateJCRXML(temp);
+        try (final InputStream tmpInputStream = new TempFileInputStream(temp)) {
+            session.importXML(path, tmpInputStream, ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW);
+        } catch (UnsupportedOperationException | IllegalArgumentException e) {
+            // These come from ModeShape when there's various problems in the formatting of the XML
+            // that are not caught by JCRXMLValidatingInputStreamBridge.
+            throw new InvalidSerializationFormatException("Invalid JCR/XML."
+                    + (e.getMessage() != null ? " (" + e.getMessage() + ")" : ""));
+        }
+    }
 
+    private void validateJCRXML(final File file) throws InvalidSerializationFormatException, IOException {
+        try (final FileInputStream fis = new FileInputStream(file)) {
+            final XMLEventReader reader = XMLInputFactory.newFactory().createXMLEventReader(fis);
+            while (reader.hasNext()) {
+                final XMLEvent event = reader.nextEvent();
+                if (event.isStartElement()) {
+                    final StartElement startElement = event.asStartElement();
+                    final QName name = startElement.getName();
+                    if (!(name.getNamespaceURI().equals("http://www.jcp.org/jcr/sv/1.0")
+                            && (name.getLocalPart().equals("node") || name.getLocalPart().equals("property")
+                            || name.getLocalPart().equals("value")))) {
+                        throw new InvalidSerializationFormatException(
+                                "Unrecognized element \"" + name.toString() + "\", in import XML.");
+                    }
+                }
+            }
+            reader.close();
+        } catch (XMLStreamException e) {
+            throw new InvalidSerializationFormatException("Unable to parse XML"
+                    + (e.getMessage() != null ? " (" + e.getMessage() + ")." : "."));
+        }
     }
 
+    /**
+     * A FileInputStream that deletes the file when closed.
+     */
+    private static final class TempFileInputStream extends FileInputStream {
+
+        private File f;
+
+        /**
+         * A constructor whose passed file's content is exposed by this
+         * TempFileInputStream, and which will be deleted when this
+         * InputStream is closed.
+         * @param f
+         * @throws FileNotFoundException
+         */
+        public TempFileInputStream(final File f) throws FileNotFoundException {
+            super(f);
+        }
+
+        @Override
+        public void close() throws IOException {
+            try {
+                super.close();
+            } finally {
+                if (f != null) {
+                    f.delete();
+                    f = null;
+                }
+            }
+        }
+    }
 }
diff --git a/fcrepo-serialization/src/test/java/org/fcrepo/serialization/JcrXmlSerializerTest.java b/fcrepo-serialization/src/test/java/org/fcrepo/serialization/JcrXmlSerializerTest.java
@@ -18,6 +18,8 @@
 import static javax.jcr.ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW;
 import static org.fcrepo.serialization.FedoraObjectSerializer.JCR_XML;
 import static org.junit.Assert.assertEquals;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -97,13 +99,11 @@ public void testSerializeWithOptions() throws Exception {
     }
 
     @Test
-    public void testDeserialize() throws IOException, RepositoryException {
+    public void testDeserialize() throws IOException, RepositoryException, InvalidSerializationFormatException {
+        final InputStream is = getClass().getClassLoader().getResourceAsStream("valid-jcr-xml.xml");
         final Session mockSession = mock(Session.class);
-        try (final InputStream mockIS = mock(InputStream.class)) {
-            new JcrXmlSerializer().deserialize(mockSession, "/objects", mockIS);
-            verify(mockSession).importXML("/objects", mockIS,
-                    IMPORT_UUID_COLLISION_THROW);
-        }
+        new JcrXmlSerializer().deserialize(mockSession, "/objects", is);
+        verify(mockSession).importXML(eq("/objects"), any(InputStream.class), eq(IMPORT_UUID_COLLISION_THROW));
     }
 
     @Test
@@ -115,4 +115,21 @@ public void testGetKey() {
     public void testGetMediaType() {
         assertEquals("application/xml", new JcrXmlSerializer().getMediaType());
     }
+
+    @Test
+    public void testValidJCRXMLValidation() throws IOException,
+            InvalidSerializationFormatException, RepositoryException {
+        final Session mockSession = mock(Session.class);
+        new JcrXmlSerializer().deserialize(mockSession, "/objects",
+                getClass().getClassLoader().getResourceAsStream("valid-jcr-xml.xml"));
+    }
+
+    @Test (expected = InvalidSerializationFormatException.class)
+    public void testInvalidJCRXMLValidation() throws IOException,
+            InvalidSerializationFormatException, RepositoryException {
+        final Session mockSession = mock(Session.class);
+        new JcrXmlSerializer().deserialize(mockSession, "/objects",
+                getClass().getClassLoader().getResourceAsStream("invalid-jcr-xml.xml"));
+    }
+
 }
diff --git a/fcrepo-serialization/src/test/resources/invalid-jcr-xml.xml b/fcrepo-serialization/src/test/resources/invalid-jcr-xml.xml
@@ -0,0 +1,3 @@
+<different>
+    <xml />
+</different>
diff --git a/fcrepo-serialization/src/test/resources/valid-jcr-xml.xml b/fcrepo-serialization/src/test/resources/valid-jcr-xml.xml
@@ -0,0 +1 @@
+<?xml version="1.0" encoding="UTF-8"?><sv:node xmlns:dc="http://purl.org/dc/elements/1.1/" xmlns:nt="http://www.jcp.org/jcr/nt/1.0" xmlns:foaf="http://xmlns.com/foaf/0.1/" xmlns:fedoraconfig="http://fedora.info/definitions/v4/config#" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:image="http://www.modeshape.org/images/1.0" xmlns:premis="http://www.loc.gov/premis/rdf/v1#" xmlns:mix="http://www.jcp.org/jcr/mix/1.0" xmlns:mode="http://www.modeshape.org/1.0" xmlns:jcr="http://www.jcp.org/jcr/1.0" xmlns:rdfs="http://www.w3.org/2000/01/rdf-schema#" xmlns:sv="http://www.jcp.org/jcr/sv/1.0" xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#" xmlns:fedora="http://fedora.info/definitions/v4/repository#" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:ldp="http://www.w3.org/ns/ldp#" sv:name="test"><sv:property sv:name="jcr:primaryType" sv:type="Name"><sv:value>nt:folder</sv:value></sv:property><sv:property sv:name="jcr:mixinTypes" sv:type="Name" sv:multiple="true"><sv:value>fedora:Container</sv:value><sv:value>fedora:Resource</sv:value></sv:property><sv:property sv:name="jcr:uuid" sv:type="String"><sv:value>bfb75d10-b5a1-4422-b137-6199043ef70b</sv:value></sv:property><sv:property sv:name="jcr:created" sv:type="Date"><sv:value>2014-11-14T09:19:01.049-05:00</sv:value></sv:property><sv:property sv:name="jcr:lastModified" sv:type="Date"><sv:value>2014-11-14T09:19:01.049-05:00</sv:value></sv:property><sv:property sv:name="jcr:lastModifiedBy" sv:type="String"><sv:value>bypassAdmin</sv:value></sv:property><sv:property sv:name="jcr:createdBy" sv:type="String"><sv:value>bypassAdmin</sv:value></sv:property></sv:node>
diff --git a/pom.xml b/pom.xml
@@ -348,6 +348,16 @@
         <artifactId>asm</artifactId>
         <version>3.1</version>
       </dependency>
+      <dependency>
+        <groupId>xml-apis</groupId>
+        <artifactId>xml-apis</artifactId>
+        <version>1.4.01</version>
+      </dependency>
+      <dependency>
+        <groupId>commons-io</groupId>
+        <artifactId>commons-io</artifactId>
+        <version>2.4</version>
+      </dependency>
       <!-- test gear -->
       <dependency>
         <groupId>junit</groupId>