Follow-on to: https://www.pivotaltracker.com/story/show/72296804

- "Handle spaces in path names" - Use URLDecoder instead of String.replaceAll
fcrepo · Jul 28, 2014 · 3279475 · 3279475
1 parent 494ab59
commit 3279475
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 15 deletions.
diff --git a/...-http-commons/src/main/java/org/fcrepo/http/commons/api/rdf/HttpIdentifierTranslator.java b/...-http-commons/src/main/java/org/fcrepo/http/commons/api/rdf/HttpIdentifierTranslator.java
@@ -26,6 +26,7 @@
 
 import static com.google.common.base.Throwables.propagate;
 import static com.hp.hpl.jena.rdf.model.ResourceFactory.createResource;
+import static java.net.URLDecoder.decode;
 import static java.util.Collections.singletonList;
 import static java.util.Collections.singletonMap;
 import static javax.jcr.PropertyType.PATH;
@@ -36,6 +37,7 @@
 import static org.modeshape.jcr.api.JcrConstants.JCR_CONTENT;
 import static org.slf4j.LoggerFactory.getLogger;
 
+import java.io.UnsupportedEncodingException;
 import java.net.URI;
 import java.util.List;
 import java.util.Map;
@@ -152,11 +154,11 @@ public boolean isCanonical() {
     @Override
     public Resource getSubject(final String absPath) throws RepositoryException {
         resetTranslationChain();
-        if ( absPath != null && absPath.indexOf("%20") != -1 ) {
-            LOGGER.debug("Creating RDF subject from identifier with spaces: {}", absPath);
-            return doForward(absPath.replaceAll("%20"," "));
-        } else {
-            LOGGER.debug("Creating RDF subject from identifier: {}", absPath);
+        try {
+            LOGGER.debug("Creating RDF subject from identifier: {}", decode(absPath, "UTF-8"));
+            return doForward(decode(absPath, "UTF-8"));
+        } catch ( UnsupportedEncodingException ex ) {
+            LOGGER.warn("Required encoding (UTF-8) not supported, trying undecoded path",ex);
             return doForward(absPath);
         }
     }

diff --git a/...-kernel-impl/src/main/java/org/fcrepo/kernel/impl/utils/JcrPropertyStatementListener.java b/...-kernel-impl/src/main/java/org/fcrepo/kernel/impl/utils/JcrPropertyStatementListener.java
@@ -16,11 +16,13 @@
 package org.fcrepo.kernel.impl.utils;
 
 import static com.google.common.base.Throwables.propagate;
+import static java.net.URLDecoder.decode;
 import static java.util.UUID.randomUUID;
 import static org.fcrepo.kernel.RdfLexicon.COULD_NOT_STORE_PROPERTY;
 import static org.fcrepo.kernel.RdfLexicon.LDP_NAMESPACE;
 import static org.slf4j.LoggerFactory.getLogger;
 
+import java.io.UnsupportedEncodingException;
 import javax.jcr.NamespaceException;
 import javax.jcr.Node;
 import javax.jcr.RepositoryException;
@@ -123,12 +125,14 @@ public void addedStatement(final Statement s) {
                     skolemizedBnodeMap.put(subject.getId(), subjectNode);
                 }
             } else {
-                final String path = subjects.getPathFromSubject(subject);
-                if ( path != null && path.indexOf("%20") != -1 ) {
-                    subjectNode = session.getNode(path.replaceAll("%20"," "));
-                } else {
-                    subjectNode = session.getNode(path);
+                String path = null;
+                try {
+                    path = decode(subjects.getPathFromSubject(subject), "UTF-8");
+                } catch ( UnsupportedEncodingException ex ) {
+                    LOGGER.warn("Required encoding (UTF-8) not supported, trying undecoded path",ex);
+                    path = subjects.getPathFromSubject(subject);
                 }
+                subjectNode = session.getNode(path);
             }
 
             // special logic for handling rdf:type updates.

diff --git a/...nel-impl/src/test/java/org/fcrepo/kernel/impl/utils/JcrPropertyStatementListenerTest.java b/...nel-impl/src/test/java/org/fcrepo/kernel/impl/utils/JcrPropertyStatementListenerTest.java
@@ -142,8 +142,8 @@ public void testAddedIrrelevantStatement() {
     public void testAddedProhibitedStatement() throws RepositoryException {
         mockStatic(JcrRdfTools.class);
         when(mockSubjects.isFedoraGraphSubject(mockSubject)).thenReturn(true);
-        when(mockSession.getNode(mockSubjects.getPathFromSubject(mockSubject)))
-                .thenReturn(mockSubjectNode);
+        when(mockSubjects.getPathFromSubject(mockSubject)).thenReturn("/some/path");
+        when(mockSession.getNode("/some/path")).thenReturn(mockSubjectNode);
         when(mockJcrRdfTools.isInternalProperty(mockSubjectNode, mockPredicate)).thenReturn(true);
 
         when(mockPredicate.getURI()).thenReturn("x");
@@ -155,7 +155,8 @@ public void testAddedProhibitedStatement() throws RepositoryException {
     public void testAddedStatement() throws RepositoryException {
         mockStatic(JcrRdfTools.class);
         when(mockSubjects.isFedoraGraphSubject(mockSubject)).thenReturn(true);
-        when(mockSession.getNode(mockSubjects.getPathFromSubject(mockSubject))).thenReturn(mockSubjectNode);
+        when(mockSubjects.getPathFromSubject(mockSubject)).thenReturn("/some/path");
+        when(mockSession.getNode("/some/path")).thenReturn(mockSubjectNode);
         final String mockPropertyName = "mock:property";
         when(
                 mockJcrRdfTools.getPropertyNameFromPredicate(mockSubjectNode,
@@ -241,7 +242,8 @@ public void testAddRdfType() throws RepositoryException {
 
         final Resource resource = createResource("xyz");
         when(mockSubjects.isFedoraGraphSubject(resource)).thenReturn(true);
-        when(mockSession.getNode(mockSubjects.getPathFromSubject(resource))).thenReturn(mockSubjectNode);
+        when(mockSubjects.getPathFromSubject(resource)).thenReturn("/xyz");
+        when(mockSession.getNode("/xyz")).thenReturn(mockSubjectNode);
 
         when(mockSubjectNode.getSession()).thenReturn(mockSession);
         when(mockSession.getWorkspace()).thenReturn(mockWorkspace);
@@ -282,7 +284,8 @@ public void testAddRdfTypeForNonMixin() throws RepositoryException {
 
         final Resource resource = createResource("xyz");
         when(mockSubjects.isFedoraGraphSubject(resource)).thenReturn(true);
-        when(mockSession.getNode(mockSubjects.getPathFromSubject(resource))).thenReturn(mockSubjectNode);
+        when(mockSubjects.getPathFromSubject(resource)).thenReturn("/xyz");
+        when(mockSession.getNode("/xyz")).thenReturn(mockSubjectNode);
 
         when(mockSubjectNode.getSession()).thenReturn(mockSession);
         when(mockSession.getWorkspace()).thenReturn(mockWorkspace);