PR: IO-567

Throw an IllegalArgumentException in FilenameUtils.getExtension(String)
for ADS names.

Author: jochenw

src/changes/changes.xml

@@ -140,6 +140,11 @@ The <action> type attribute can be add,update,fix,remove.
       <action issue="IO-514" dev="pschumacher" type="remove">
         Remove org.apache.commons.io.Java7Support
       </action>
+      <action issue="IO-567" dev="jochen" type="fix">
+        Implement special case handling for NTFS ADS names: FilenameUtils.getExtension(String),
+        and FilenameUtils.indexOfExtension(String) are now throwing an IllegalArgumentException,
+        if the file name in question appears to identify an alternate data stream (Windows only).
+      </action>
     </release>
 
     <release version="2.5" date="2016-04-22" description="New features and bug fixes.">

src/main/java/org/apache/commons/io/FilenameUtils.java

@@ -716,15 +716,28 @@ public static int indexOfLastSeparator(final String filename) {
      * <p>
      * The output will be the same irrespective of the machine that the code is running on.
      * </p>
+     * <b>Note:</b> This method used to have a hidden problem for names like "foo.exe:bar.txt".
+     * In this case, the name wouldn't be the name of a file, but the identifier of an
+     * alternate data stream (bar.txt) on the file foo.exe. The method used to return
+     * ".txt" here, which would be misleading. Commons IO 2.7, and later versions, are throwing
+     * at {@link NtfsAdsNameException} for names like this.
      *
      * @param filename
      *            the filename to find the last extension separator in, null returns -1
      * @return the index of the last extension separator character, or -1 if there is no such character
+     * @throws NtfsAdsNameException The filename argument 
      */
-    public static int indexOfExtension(final String filename) {
+    public static int indexOfExtension(final String filename) throws NtfsAdsNameException {
         if (filename == null) {
             return NOT_FOUND;
         }
+        if (isSystemWindows()) {
+        	// Special handling for NTFS ADS: Don't accept colon in the filename.
+        	final int offset = filename.indexOf(':', getAdsCriticalOffset(filename));
+        	if (offset != -1) {
+        		throw new NtfsAdsNameException("NTFS ADS separator (':') in filename is forbidden.");
+        	}
+        }
         final int extensionPos = filename.lastIndexOf(EXTENSION_SEPARATOR);
         final int lastSeparator = indexOfLastSeparator(filename);
         return lastSeparator > extensionPos ? NOT_FOUND : extensionPos;
@@ -1027,12 +1040,19 @@ public static String getBaseName(final String filename) {
      * </pre>
      * <p>
      * The output will be the same irrespective of the machine that the code is running on.
+     * <b>Note:</b> This method used to have a hidden problem for names like "foo.exe:bar.txt".
+     * In this case, the name wouldn't be the name of a file, but the identifier of an
+     * alternate data stream (bar.txt) on the file foo.exe. The method used to return
+     * ".txt" here, which would be misleading. Commons IO 2.7, and later versions, are throwing
+     * at {@link NtfsAdsNameException} for names like this.
      *
      * @param filename the filename to retrieve the extension of.
      * @return the extension of the file or an empty string if none exists or {@code null}
      * if the filename is {@code null}.
+     * @throws NtfsAdsNameException <b>Windows only:/b> The filename parameter is, in fact,
+     * the identifier of an Alternate Data Stream, for example "foo.exe:bar.txt".
      */
-    public static String getExtension(final String filename) {
+    public static String getExtension(final String filename) throws NtfsAdsNameException {
         if (filename == null) {
             return null;
         }
@@ -1044,6 +1064,25 @@ public static String getExtension(final String filename) {
         }
     }
 
+    private static int getAdsCriticalOffset(String filename) {
+    	// Step 1: Remove leading path segments.
+    	int offset1 = filename.lastIndexOf(SYSTEM_SEPARATOR);
+    	int offset2 = filename.lastIndexOf(OTHER_SEPARATOR);
+    	if (offset1 == -1) {
+    		if (offset2 == -1) {
+    			return 0;
+    		} else {
+    			return offset2 + 1;
+    		}
+    	} else {
+    		if (offset2 == -1) {
+    			return offset1+1;
+    		} else {
+    			return Math.max(offset1, offset2)+1;
+    		}
+    	}
+    }
+    
     //-----------------------------------------------------------------------
     /**
      * Removes the extension from a filename.

src/main/java/org/apache/commons/io/NtfsAdsNameException.java

@@ -0,0 +1,18 @@
+package org.apache.commons.io;
+
+
+/**
+ * This exception is thrown, if an NTFS ADS name is passed to certain methods,
+ * where it might affect the result. For example, if you pass a name like
+ * "foo.exe:bar.txt" to {@link FilenameUtils#getExtension(String)}, then it
+ * might return ".txt", which would be misleading, because the actual extension
+ * would be ".txt".
+ */
+public class NtfsAdsNameException extends IllegalArgumentException {
+
+	private static final long serialVersionUID = -9158109384797441214L;
+
+	public NtfsAdsNameException(String pMessage) {
+		super(pMessage);
+	}
+}

src/test/java/org/apache/commons/io/FilenameUtilsTestCase.java

@@ -35,6 +35,8 @@
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
+import org.junit.Assert;
+
 /**
  * This is used to test FilenameUtils for correctness.
  *
@@ -580,6 +582,20 @@ public void testIndexOfExtension() {
         assertEquals(-1, FilenameUtils.indexOfExtension("a\\b\\c"));
         assertEquals(-1, FilenameUtils.indexOfExtension("a/b.notextension/c"));
         assertEquals(-1, FilenameUtils.indexOfExtension("a\\b.notextension\\c"));
+
+        if (FilenameUtils.isSystemWindows()) {
+            // Special case handling for NTFS ADS names
+        	try {
+        		FilenameUtils.indexOfExtension("foo.exe:bar.txt");
+        		throw new AssertionError("Expected Exception");
+        	} catch (IllegalArgumentException e) {
+        		assertEquals("NTFS ADS separator (':') in filename is forbidden.", e.getMessage());
+        	}
+        } else {
+        	// Upwards compatibility on other systems
+        	assertEquals(11, FilenameUtils.indexOfExtension("foo.exe:bar.txt"));
+        }
+
     }
 
     //-----------------------------------------------------------------------
@@ -862,6 +878,19 @@ public void testGetExtension() {
         assertEquals("", FilenameUtils.getExtension("a\\b\\c"));
         assertEquals("", FilenameUtils.getExtension("C:\\temp\\foo.bar\\README"));
         assertEquals("ext", FilenameUtils.getExtension("../filename.ext"));
+
+        if (FilenameUtils.isSystemWindows()) {
+            // Special case handling for NTFS ADS names
+        	try {
+        		FilenameUtils.getExtension("foo.exe:bar.txt");
+        		throw new AssertionError("Expected Exception");
+        	} catch (IllegalArgumentException e) {
+        		assertEquals("NTFS ADS separator (':') in filename is forbidden.", e.getMessage());
+        	}
+        } else {
+        	// Upwards compatibility:
+        	assertEquals("txt", FilenameUtils.getExtension("foo.exe:bar.txt"));
+        }
     }
 
     @Test
@@ -1082,5 +1111,4 @@ public void testIsExtensionCollection() {
         assertFalse(FilenameUtils.isExtension("a.b\\file.txt", new ArrayList<>(Arrays.asList(new String[]{"TXT"}))));
         assertFalse(FilenameUtils.isExtension("a.b\\file.txt", new ArrayList<>(Arrays.asList(new String[]{"TXT", "RTF"}))));
     }
-
 }