Skip to content

(doc) [IO-484] Fix incorrect FilenameUtils documentation for null bytes #310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
garydgregory merged 3 commits into from
Jan 30, 2022
Merged

(doc) [IO-484] Fix incorrect FilenameUtils documentation for null bytes #310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
511e77e
(doc) [IO-484] Fix incorrect FilenameUtils documentation for null bytes
Marcono1234 Dec 6, 2021
045998f
(doc) Consistently use term "null character"
Marcono1234 Dec 6, 2021
ad4e622
(doc) Remove TODO comments for null character handling
Marcono1234 Jan 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 35 additions & 22 deletions src/main/java/org/apache/commons/io/FilenameUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,8 @@ public class FilenameUtils {
*
* @param basePath the base path to attach to, always treated as a path
* @param fullFileNameToAdd the fileName (or path) to attach to the base
* @return the concatenated path, or null if invalid. Null bytes inside string will be removed
* @return the concatenated path, or null if invalid
* @throws IllegalArgumentException if the result path contains the null character ({@code U+0000})
*/
public static String concat(final String basePath, final String fullFileNameToAdd) {
final int prefix = getPrefixLength(fullFileNameToAdd);
Expand Down Expand Up @@ -260,6 +261,7 @@ public static boolean directoryContains(final String canonicalParent, final Stri
* @param fileName the fileName
* @param includeSeparator true to include the end separator
* @return the path
* @throws IllegalArgumentException if the result path contains the null character ({@code U+0000})
*/
private static String doGetFullPath(final String fileName, final boolean includeSeparator) {
if (fileName == null) {
Expand Down Expand Up @@ -291,7 +293,8 @@ private static String doGetFullPath(final String fileName, final boolean include
*
* @param fileName the fileName
* @param separatorAdd 0 to omit the end separator, 1 to return it
* @return the path. Null bytes inside string will be removed
* @return the path
* @throws IllegalArgumentException if the result path contains the null character ({@code U+0000})
*/
private static String doGetPath(final String fileName, final int separatorAdd) {
if (fileName == null) {
Expand All @@ -315,7 +318,8 @@ private static String doGetPath(final String fileName, final int separatorAdd) {
* @param fileName the fileName
* @param separator The separator character to use
* @param keepSeparator true to keep the final separator
* @return the normalized fileName. Null bytes inside string will be removed.
* @return the normalized fileName
* @throws IllegalArgumentException if the fileName contains the null character ({@code U+0000})
*/
private static String doNormalize(final String fileName, final char separator, final boolean keepSeparator) {
if (fileName == null) {
Expand Down Expand Up @@ -565,8 +569,8 @@ private static int getAdsCriticalOffset(final String fileName) {
* </p>
*
* @param fileName the fileName to query, null returns null
* @return the name of the file without the path, or an empty string if none exists. Null bytes inside string
* will be removed
* @return the name of the file without the path, or an empty string if none exists
* @throws IllegalArgumentException if the fileName contains the null character ({@code U+0000})
*/
public static String getBaseName(final String fileName) {
return removeExtension(getName(fileName));
Expand Down Expand Up @@ -639,6 +643,7 @@ public static String getExtension(final String fileName) throws IllegalArgumentE
*
* @param fileName the fileName to query, null returns null
* @return the path of the file, an empty string if none exists, null if invalid
* @throws IllegalArgumentException if the result path contains the null character ({@code U+0000})
*/
public static String getFullPath(final String fileName) {
return doGetFullPath(fileName, true);
Expand Down Expand Up @@ -671,6 +676,7 @@ public static String getFullPath(final String fileName) {
*
* @param fileName the fileName to query, null returns null
* @return the path of the file, an empty string if none exists, null if invalid
* @throws IllegalArgumentException if the result path contains the null character ({@code U+0000})
*/
public static String getFullPathNoEndSeparator(final String fileName) {
return doGetFullPath(fileName, false);
Expand All @@ -693,8 +699,8 @@ public static String getFullPathNoEndSeparator(final String fileName) {
* </p>
*
* @param fileName the fileName to query, null returns null
* @return the name of the file without the path, or an empty string if none exists.
* Null bytes inside string will be removed
* @return the name of the file without the path, or an empty string if none exists
* @throws IllegalArgumentException if the fileName contains the null character ({@code U+0000})
*/
public static String getName(final String fileName) {
if (fileName == null) {
Expand Down Expand Up @@ -726,8 +732,8 @@ public static String getName(final String fileName) {
* </p>
*
* @param fileName the fileName to query, null returns null
* @return the path of the file, an empty string if none exists, null if invalid.
* Null bytes inside string will be removed
* @return the path of the file, an empty string if none exists, null if invalid
* @throws IllegalArgumentException if the result path contains the null character ({@code U+0000})
*/
public static String getPath(final String fileName) {
return doGetPath(fileName, 1);
Expand Down Expand Up @@ -757,8 +763,8 @@ public static String getPath(final String fileName) {
* </p>
*
* @param fileName the fileName to query, null returns null
* @return the path of the file, an empty string if none exists, null if invalid.
* Null bytes inside string will be removed
* @return the path of the file, an empty string if none exists, null if invalid
* @throws IllegalArgumentException if the result path contains the null character ({@code U+0000})
*/
public static String getPathNoEndSeparator(final String fileName) {
return doGetPath(fileName, 0);
Expand Down Expand Up @@ -793,7 +799,8 @@ public static String getPathNoEndSeparator(final String fileName) {
* </p>
*
* @param fileName the fileName to query, null returns null
* @return the prefix of the file, null if invalid. Null bytes inside string will be removed
* @return the prefix of the file, null if invalid
* @throws IllegalArgumentException if the result contains the null character ({@code U+0000})
*/
public static String getPrefix(final String fileName) {
if (fileName == null) {
Expand Down Expand Up @@ -838,7 +845,7 @@ public static String getPrefix(final String fileName) {
* ~user --&gt; 6 --&gt; named user (slash added)
* //server/a/b/c.txt --&gt; 9
* ///a/b/c.txt --&gt; -1 --&gt; error
* C: --&gt; 0 --&gt; valid filename as only null byte and / are reserved characters
* C: --&gt; 0 --&gt; valid filename as only null character and / are reserved characters
* </pre>
* <p>
* The output will be the same irrespective of the machine that the code is running on.
Expand Down Expand Up @@ -987,7 +994,7 @@ private static boolean isEmpty(final String string) {
* @param fileName the fileName to query, null returns false
* @param extensions the extensions to check for, null checks for no extension
* @return true if the fileName is one of the extensions
* @throws java.lang.IllegalArgumentException if the supplied fileName contains null bytes
* @throws IllegalArgumentException if the fileName contains the null character ({@code U+0000})
*/
public static boolean isExtension(final String fileName, final Collection<String> extensions) {
if (fileName == null) {
Expand Down Expand Up @@ -1017,7 +1024,7 @@ public static boolean isExtension(final String fileName, final Collection<String
* @param fileName the fileName to query, null returns false
* @param extension the extension to check for, null or empty checks for no extension
* @return true if the fileName has the specified extension
* @throws java.lang.IllegalArgumentException if the supplied fileName contains null bytes
* @throws IllegalArgumentException if the fileName contains the null character ({@code U+0000})
*/
public static boolean isExtension(final String fileName, final String extension) {
if (fileName == null) {
Expand All @@ -1041,7 +1048,7 @@ public static boolean isExtension(final String fileName, final String extension)
* @param fileName the fileName to query, null returns false
* @param extensions the extensions to check for, null checks for no extension
* @return true if the fileName is one of the extensions
* @throws java.lang.IllegalArgumentException if the supplied fileName contains null bytes
* @throws IllegalArgumentException if the fileName contains the null character ({@code U+0000})
*/
public static boolean isExtension(final String fileName, final String... extensions) {
if (fileName == null) {
Expand Down Expand Up @@ -1253,7 +1260,8 @@ private static boolean isValidHostName(final String name) {
* (Note the file separator returned will be correct for Windows/Unix)
*
* @param fileName the fileName to normalize, null returns null
* @return the normalized fileName, or null if invalid. Null bytes inside string will be removed
* @return the normalized fileName, or null if invalid
* @throws IllegalArgumentException if the fileName contains the null character ({@code U+0000})
*/
public static String normalize(final String fileName) {
return doNormalize(fileName, SYSTEM_NAME_SEPARATOR, true);
Expand Down Expand Up @@ -1300,7 +1308,8 @@ public static String normalize(final String fileName) {
* @param fileName the fileName to normalize, null returns null
* @param unixSeparator {@code true} if a unix separator should
* be used or {@code false} if a windows separator should be used.
* @return the normalized fileName, or null if invalid. Null bytes inside string will be removed
* @return the normalized fileName, or null if invalid
* @throws IllegalArgumentException if the fileName contains the null character ({@code U+0000})
* @since 2.0
*/
public static String normalize(final String fileName, final boolean unixSeparator) {
Expand Down Expand Up @@ -1346,7 +1355,8 @@ public static String normalize(final String fileName, final boolean unixSeparato
* (Note the file separator returned will be correct for Windows/Unix)
*
* @param fileName the fileName to normalize, null returns null
* @return the normalized fileName, or null if invalid. Null bytes inside string will be removed
* @return the normalized fileName, or null if invalid
* @throws IllegalArgumentException if the fileName contains the null character ({@code U+0000})
*/
public static String normalizeNoEndSeparator(final String fileName) {
return doNormalize(fileName, SYSTEM_NAME_SEPARATOR, false);
Expand Down Expand Up @@ -1392,7 +1402,8 @@ public static String normalizeNoEndSeparator(final String fileName) {
* @param fileName the fileName to normalize, null returns null
* @param unixSeparator {@code true} if a unix separator should
* be used or {@code false} if a windows separator should be used.
* @return the normalized fileName, or null if invalid. Null bytes inside string will be removed
* @return the normalized fileName, or null if invalid
* @throws IllegalArgumentException if the fileName contains the null character ({@code U+0000})
* @since 2.0
*/
public static String normalizeNoEndSeparator(final String fileName, final boolean unixSeparator) {
Expand All @@ -1415,6 +1426,7 @@ public static String normalizeNoEndSeparator(final String fileName, final boolea
*
* @param fileName the fileName to query, null returns null
* @return the fileName minus the extension
* @throws IllegalArgumentException if the fileName contains the null character ({@code U+0000})
*/
public static String removeExtension(final String fileName) {
if (fileName == null) {
Expand All @@ -1430,17 +1442,18 @@ public static String removeExtension(final String fileName) {
}

/**
* Checks the input for null bytes, a sign of unsanitized data being passed to to file level functions.
* Checks the input for null characters ({@code U+0000}), a sign of unsanitized data being passed to to file level functions.
*
* This may be used for poison byte attacks.
*
* @param path the path to check
* @return The input
* @throws IllegalArgumentException if path contains the null character ({@code U+0000})
*/
private static String requireNonNullChars(final String path) {
if (path.indexOf(0) >= 0) {
throw new IllegalArgumentException(
"Null byte present in file/path name. There are no known legitimate use cases for such data, but several injection attacks may use it");
"Null character present in file/path name. There are no known legitimate use cases for such data, but several injection attacks may use it");
}
return path;
}
Expand Down
10 changes: 5 additions & 5 deletions src/test/java/org/apache/commons/io/FilenameUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ public void testGetBaseName() {
}

@Test
public void testGetBaseName_with_nullByte() {
public void testGetBaseName_with_null_character() {
try {
assertEquals("file.txt", FilenameUtils.getBaseName("fil\u0000e.txt.bak"));
} catch (final IllegalArgumentException ignore) {
Expand Down Expand Up @@ -419,7 +419,7 @@ public void testGetPath() {


@Test
public void testGetPath_with_nullbyte() {
public void testGetPath_with_null_character() {
assertThrows(IllegalArgumentException.class, () -> FilenameUtils.getPath("~user/a/\u0000b/c.txt"));
}

Expand Down Expand Up @@ -461,7 +461,7 @@ public void testGetPathNoEndSeparator() {
}

@Test
public void testGetPathNoEndSeparator_with_null_byte() {
public void testGetPathNoEndSeparator_with_null_character() {
try {
assertEquals("a/b", FilenameUtils.getPathNoEndSeparator("~user/a\u0000/b/c.txt"));
} catch (final IllegalArgumentException ignore) {
Expand Down Expand Up @@ -515,7 +515,7 @@ public void testGetPrefix() {
}

@Test
public void testGetPrefix_with_nullbyte() {
public void testGetPrefix_with_null_character() {
try {
assertEquals("~user\\", FilenameUtils.getPrefix("~u\u0000ser\\a\\b\\c.txt"));
} catch (final IllegalArgumentException ignore) {
Expand Down Expand Up @@ -957,7 +957,7 @@ public void testNormalize() {
/**
*/
@Test
public void testNormalize_with_nullbytes() {
public void testNormalize_with_null_character() {
try {
assertEquals("a" + SEP + "b" + SEP + "c.txt", FilenameUtils.normalize("a\\b/c\u0000.txt"));
} catch (final IllegalArgumentException ignore) {
Expand Down