Repository: commons-io Updated Branches: refs/heads/master 72d00532a -> 2c30851e4
PR: IO-567 Throw an IllegalArgumentException in FilenameUtils.getExtension(String) for ADS names. Project: http://git-wip-us.apache.org/repos/asf/commons-io/repo Commit: http://git-wip-us.apache.org/repos/asf/commons-io/commit/947c01f6 Tree: http://git-wip-us.apache.org/repos/asf/commons-io/tree/947c01f6 Diff: http://git-wip-us.apache.org/repos/asf/commons-io/diff/947c01f6 Branch: refs/heads/master Commit: 947c01f6d8000122a174da3d668208eec877a799 Parents: 459cebc Author: Jochen Wiedmann (jwi) <[email protected]> Authored: Tue Jan 30 15:24:39 2018 +0100 Committer: Jochen Wiedmann (jwi) <[email protected]> Committed: Tue Jan 30 15:31:09 2018 +0100 ---------------------------------------------------------------------- src/changes/changes.xml | 5 +++ .../org/apache/commons/io/FilenameUtils.java | 43 +++++++++++++++++++- .../apache/commons/io/NtfsAdsNameException.java | 18 ++++++++ .../commons/io/FilenameUtilsTestCase.java | 30 +++++++++++++- 4 files changed, 93 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/commons-io/blob/947c01f6/src/changes/changes.xml ---------------------------------------------------------------------- diff --git a/src/changes/changes.xml b/src/changes/changes.xml index a70b4ee..16a3dd9 100644 --- a/src/changes/changes.xml +++ b/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."> http://git-wip-us.apache.org/repos/asf/commons-io/blob/947c01f6/src/main/java/org/apache/commons/io/FilenameUtils.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/io/FilenameUtils.java b/src/main/java/org/apache/commons/io/FilenameUtils.java index 9cddebb..382df41 100644 --- a/src/main/java/org/apache/commons/io/FilenameUtils.java +++ b/src/main/java/org/apache/commons/io/FilenameUtils.java @@ -716,15 +716,28 @@ public class FilenameUtils { * <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 class FilenameUtils { * </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 class FilenameUtils { } } + 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. http://git-wip-us.apache.org/repos/asf/commons-io/blob/947c01f6/src/main/java/org/apache/commons/io/NtfsAdsNameException.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/io/NtfsAdsNameException.java b/src/main/java/org/apache/commons/io/NtfsAdsNameException.java new file mode 100644 index 0000000..2ca9ab1 --- /dev/null +++ b/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); + } +} http://git-wip-us.apache.org/repos/asf/commons-io/blob/947c01f6/src/test/java/org/apache/commons/io/FilenameUtilsTestCase.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/commons/io/FilenameUtilsTestCase.java b/src/test/java/org/apache/commons/io/FilenameUtilsTestCase.java index 234c25e..ea38b50 100644 --- a/src/test/java/org/apache/commons/io/FilenameUtilsTestCase.java +++ b/src/test/java/org/apache/commons/io/FilenameUtilsTestCase.java @@ -35,6 +35,8 @@ import org.junit.Rule; 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 class FilenameUtilsTestCase { 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 class FilenameUtilsTestCase { 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 class FilenameUtilsTestCase { 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"})))); } - }
