close #370: replace all whitespace chars and horizontal whitespace ch…

…ars with a standard whitespace. Trim the filename
torakiki · Dec 2, 2019 · cccdc2b · cccdc2b
1 parent 8d3d965
commit cccdc2b
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 14 deletions.
diff --git a/sejda-core/src/main/java/org/sejda/core/support/prefix/NameGenerator.java b/sejda-core/src/main/java/org/sejda/core/support/prefix/NameGenerator.java
@@ -22,7 +22,6 @@
 
 import static java.util.Optional.ofNullable;
 import static org.sejda.core.support.prefix.model.NameGenerationRequest.nameRequest;
-import static org.sejda.core.support.prefix.processor.PrefixUtils.toSafeFilename;
 
 import org.apache.commons.lang3.StringUtils;
 import org.sejda.core.support.prefix.model.NameGenerationRequest;
@@ -60,7 +59,7 @@ public String generate(NameGenerationRequest request) {
         if (request == null) {
             throw new IllegalArgumentException("Unable to generate a name for a null request.");
         }
-        return toSafeFilename(prefixTypesChain.process(prefix, ofNullable(request).orElseGet(() -> nameRequest())));
+        return prefixTypesChain.process(prefix, ofNullable(request).orElseGet(() -> nameRequest()));
     }
 
 }
diff --git a/sejda-core/src/main/java/org/sejda/core/support/prefix/processor/PrefixTypesChain.java b/sejda-core/src/main/java/org/sejda/core/support/prefix/processor/PrefixTypesChain.java
@@ -21,6 +21,7 @@
 package org.sejda.core.support.prefix.processor;
 
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
+import static org.sejda.core.support.prefix.processor.PrefixUtils.toSafeFilename;
 
 import java.util.HashSet;
 import java.util.Set;
@@ -79,7 +80,7 @@ public String process(String prefix, NameGenerationRequest request) {
         if (!ensureUniqueness) {
             retVal = pageNumberPrepend.process(retVal, request);
         }
-        return extensionProcessor.process(retVal, request);
+        return extensionProcessor.process(toSafeFilename(retVal), request);
     }
 
     private String processChain(String prefix, NameGenerationRequest request, Set<PrefixType> chain) {

diff --git a/sejda-core/src/main/java/org/sejda/core/support/prefix/processor/PrefixUtils.java b/sejda-core/src/main/java/org/sejda/core/support/prefix/processor/PrefixUtils.java
@@ -18,25 +18,28 @@
  */
 package org.sejda.core.support.prefix.processor;
 
-import org.apache.commons.lang3.StringUtils;
+import static org.apache.commons.lang3.StringUtils.defaultIfBlank;
 
 public final class PrefixUtils {
     private PrefixUtils() {
         // hide
     }
 
     /**
-     * Strips characters deemed usafe for a filename
+     * @return A string where all the variations of whitespace and horizontal whitespace are replace with a standard whitespace, all characters deemed unsafe for a filename are
+     *         stripped and the resulting filename is trimmed
      */
     public static String toSafeFilename(String input) {
-        return StringUtils.defaultIfBlank(input, "").replaceAll("[`\0\f\t\n\r\\\\/:*?\\\"<>|]", "");
+        // TODO maybe we should do the trimming when we ensure a proper file name length to make sure we do it for every generated file? Or we shouldn't do it at all?
+        return defaultIfBlank(input, "").replaceAll("\\s|\\h", " ").replaceAll("[`\0\f\t\n\r\\\\/:*?\\\"<>|]", "")
+                .trim();
     }
 
     /**
      * Strips all but characters that are known to be safe: alphanumerics for now.
      */
     public static String toStrictFilename(String input) {
-        String safe = StringUtils.defaultIfBlank(input, "").replaceAll("[^A-Za-z0-9_ .-]", "");
+        String safe = defaultIfBlank(input, "").replaceAll("[^A-Za-z0-9_ .-]", "");
         if (safe.length() > 255) {
             safe = safe.substring(0, 255);
         }

diff --git a/sejda-core/src/test/java/org/sejda/core/support/prefix/processor/PrefixTypesChainTest.java b/sejda-core/src/test/java/org/sejda/core/support/prefix/processor/PrefixTypesChainTest.java
@@ -77,4 +77,18 @@ public void filenumberAndExtension() {
         assertEquals("prefix_3_name.txt", new PrefixTypesChain(prefix).process(prefix,
                 nameRequest("txt").originalName("name.pdf").fileNumber(3)));
     }
+
+    @Test
+    public void invalidCharsText() {
+        String prefix = "[TEXT] [BASENAME]";
+        assertEquals("This   has  some  name.txt", new PrefixTypesChain(prefix).process(prefix,
+                nameRequest("txt").originalName("name.pdf").text("  This \n \u00A0 has \t some $§°éç")));
+    }
+
+    @Test
+    public void invalidCharsBookmarks() {
+        String prefix = "[BOOKMARK_NAME] [BASENAME]";
+        assertEquals("This     has some $§°éç name.txt", new PrefixTypesChain(prefix).process(prefix,
+                nameRequest("txt").originalName("name.pdf").bookmark("  This \n \u00A0 has\tsome $§°éç ")));
+    }
 }
diff --git a/sejda-core/src/test/java/org/sejda/core/support/prefix/processor/PrefixUtilsTest.java b/sejda-core/src/test/java/org/sejda/core/support/prefix/processor/PrefixUtilsTest.java
@@ -25,23 +25,37 @@
 
 public class PrefixUtilsTest {
 
+    @Test
+    public void nullSafe() {
+        assertEquals("", PrefixUtils.toSafeFilename(null));
+    }
+
     @Test
     public void testSafeFilename() {
-        assertEquals("1_Invoice#0001.pdf", PrefixUtils.toSafeFilename("1_Invoice#0001:*<>/\\.pdf"));
-        assertEquals("..test.pdf", PrefixUtils.toSafeFilename("../test.pdf"));
-        assertEquals("..test.pdf", PrefixUtils.toSafeFilename("..\\test.pdf"));
-        assertEquals(".test.pdf", PrefixUtils.toSafeFilename("./test.pdf"));
-        assertEquals("rest.pdf", PrefixUtils.toSafeFilename("\r\n\t\f`rest.pdf"));
+        assertEquals("1_Invoice#0001", PrefixUtils.toSafeFilename("1_Invoice#0001:*<>/\\"));
+        assertEquals("..test", PrefixUtils.toSafeFilename("../test"));
+        assertEquals("..test", PrefixUtils.toSafeFilename("..\\test"));
+        assertEquals(".test", PrefixUtils.toSafeFilename("./test"));
+        assertEquals("rest", PrefixUtils.toSafeFilename("\r\n\t\f`rest"));
+    }
+
+    @Test
+    public void safeFilenameWhitespaces() {
+        assertEquals("Chuck Norris", PrefixUtils.toSafeFilename("Chuck\tNorris"));
+        assertEquals("Chuck Norris", PrefixUtils.toSafeFilename("\u00A0Chuck\u00A0Norris\u00A0"));
+        assertEquals("Chuck Norris", PrefixUtils.toSafeFilename("\u00A0\n\t\u000B\f\rChuck\nNorris\u202f"));
+        assertEquals("This is a Chuck Norris roundkick, will Steven Segal survive Nope!", PrefixUtils.toSafeFilename(
+                "This\u1680is\u180ea\u2000Chuck\u200aNorris\u202froundkick,\u205fwill\u3000Steven\fSegal\rsurvive?\u000BNope!"));
     }
 
     @Test
     public void testStrictFilename() {
-        assertEquals("1_Invoice0001.pdf", PrefixUtils.toStrictFilename("1_Invoice#0001:*<>/\\.pdf"));
+        assertEquals("1_Invoice0001", PrefixUtils.toStrictFilename("1_Invoice#0001:*<>/\\"));
         assertEquals(StringUtils.repeat('a', 255), PrefixUtils.toStrictFilename(StringUtils.repeat('a', 256)));
     }
 
     @Test
-    public void testNulls(){
+    public void testNulls() {
         assertEquals("", PrefixUtils.toSafeFilename(null));
         assertEquals("", PrefixUtils.toStrictFilename(null));
     }