This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 1.x
in repository https://gitbox.apache.org/repos/asf/commons-fileupload.git
The following commit(s) were added to refs/heads/1.x by this push:
new e4739db1 Implement stricter checks for fields using RFC 2231 / RFC 5987
e4739db1 is described below
commit e4739db19d6d2eb4b702db3f1bbc0b2e7d561f99
Author: Mark Thomas <[email protected]>
AuthorDate: Tue May 12 08:02:33 2026 +0100
Implement stricter checks for fields using RFC 2231 / RFC 5987
Invalid characters or %nn encoded values that include non-hex values in
the extended value will cause the value to be ignored.
Includes some changes prompted by a CoPilot review
---
src/changes/changes.xml | 1 +
.../apache/commons/fileupload/ParameterParser.java | 5 ++-
.../apache/commons/fileupload/RFC2231Utility.java | 49 +++++++++++++++++++---
.../commons/fileupload/RFC2231UtilityTestCase.java | 18 ++++++++
4 files changed, 67 insertions(+), 6 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 5b79a01a..dca104d8 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -50,6 +50,7 @@ The <action> type attribute can be add,update,fix,remove.
<action type="fix" dev="ggregory" due-to="Gary Gregory">If
DiskFileItem.delete() can't delete the file, it marks it for deletion on JVM
exit.</action>
<action type="fix" dev="ggregory" due-to="Gary
Gregory">DiskFileItem.delete() and finalize() always ties to delete its backing
file.</action>
<action type="fix" dev="ggregory" due-to="Gary Gregory">Javadoc that you
shouldn't rely on the garbage collection to cause the JVM to call
FileItem.delete() through finalization; see JEP 421: Deprecate Finalization for
Removal.</action>
+ <action type="fix" dev="markt" due-to="Mark
Thomas">Implement stricter checks for fields using RFC 2231 / RFC 5987. Invalid
extended values will be ignored.</action>
<!-- ADD -->
<!-- UPDATE -->
<action type="update" dev="ggregory" due-to="Gary Gregory">Bump
org.apache.commons:commons-parent from 84 to 99.</action>
diff --git a/src/main/java/org/apache/commons/fileupload/ParameterParser.java
b/src/main/java/org/apache/commons/fileupload/ParameterParser.java
index edf8016e..99b4332d 100644
--- a/src/main/java/org/apache/commons/fileupload/ParameterParser.java
+++ b/src/main/java/org/apache/commons/fileupload/ParameterParser.java
@@ -188,7 +188,10 @@ public class ParameterParser {
if (paramValue != null) {
try {
paramValue = RFC2231Utility.hasEncodedValue(paramName)
? RFC2231Utility.decodeText(paramValue) : MimeUtility.decodeText(paramValue);
- } catch (final UnsupportedEncodingException e) {
+ } catch (final IllegalArgumentException iae) {
+ // Treat invalid values as if they were not provided
+ paramValue = null;
+ } catch (final UnsupportedEncodingException ignored) {
// let's keep the original value in this case
}
}
diff --git a/src/main/java/org/apache/commons/fileupload/RFC2231Utility.java
b/src/main/java/org/apache/commons/fileupload/RFC2231Utility.java
index 8375f878..dbe35323 100644
--- a/src/main/java/org/apache/commons/fileupload/RFC2231Utility.java
+++ b/src/main/java/org/apache/commons/fileupload/RFC2231Utility.java
@@ -49,22 +49,54 @@ final class RFC2231Utility {
private static final byte MASK = 0x7f;
/**
- * The Hexadecimal representation of 128.
+ * ASCII DEL character.
*/
- private static final int MASK_128 = 0x80;
+ private static final int DEL = 127;
+
+ /**
+ * Number of ASCII code points.
+ */
+ private static final int ASCII_CODE_POINT_COUNT = 128;
/**
* The Hexadecimal decode value.
*/
- private static final byte[] HEX_DECODE = new byte[MASK_128];
+ private static final byte[] HEX_DECODE = new byte[ASCII_CODE_POINT_COUNT];
+
+ /**
+ * Flags, one for each ASCII code point, that indicate if that code point
is valid for use in an RFC 5987 extended
+ * attribute value.
+ */
+ private static final boolean[] ATTR_CHAR = new
boolean[ASCII_CODE_POINT_COUNT];
+
// create a ASCII decoded array of Hexadecimal values
static {
+ // Initialise all values to invalid
+ for (int i = 0; i < ASCII_CODE_POINT_COUNT; i++) {
+ HEX_DECODE[i] = -1;
+ }
+ // Configure the valid hex digits
for (int i = 0; i < HEX_DIGITS.length; i++) {
HEX_DECODE[HEX_DIGITS[i]] = (byte) i;
HEX_DECODE[Character.toLowerCase(HEX_DIGITS[i])] = (byte) i;
}
+
+ for (int i = 0; i < ASCII_CODE_POINT_COUNT; i++) {
+ // See RFC 5987
+ if (!(i < ' ' || i == ' ' || i == '\"' || i == '%' || i == '\'' ||
i == '(' || i == ')' || i == '*' || i == ','
+ || i == '/' || i == ':' || i == ';' || i == '<' || i ==
'=' || i == '>' || i == '?' || i == '@'
+ || i == '[' || i == '\\' || i == ']' || i == '{' || i ==
'}' || i == DEL)) {
+ ATTR_CHAR[i] = true;
+ }
+ }
}
+
+ static boolean isAttrChar(final char c) {
+ return c < ASCII_CODE_POINT_COUNT && ATTR_CHAR[c];
+ }
+
+
/**
* Decodes a string of text obtained from a HTTP header as per RFC 2231
* <p>
@@ -79,9 +111,10 @@ final class RFC2231Utility {
*
* @param encodedText Text to be decoded has a format of {@code
<charset>'<language>'<encoded_value>} and ASCII only
* @return Decoded text based on charset encoding
+ * @throws IllegalArgumentException The encoded text contained characters
not permitted by RFC 2231
* @throws UnsupportedEncodingException The requested character set wasn't
found.
*/
- static String decodeText(final String encodedText) throws
UnsupportedEncodingException {
+ static String decodeText(final String encodedText) throws
IllegalArgumentException, UnsupportedEncodingException {
final int langDelimitStart = encodedText.indexOf('\'');
if (langDelimitStart == -1) {
// missing charset
@@ -97,6 +130,7 @@ final class RFC2231Utility {
return new String(bytes, getJavaCharset(mimeCharset));
}
+
/**
* Converts {@code text} to their corresponding Hex value.
*
@@ -114,9 +148,14 @@ final class RFC2231Utility {
}
final byte b1 = HEX_DECODE[text.charAt(i++) & MASK];
final byte b2 = HEX_DECODE[text.charAt(i++) & MASK];
+ if (b1 < 0 || b2 < 0) {
+ throw new IllegalArgumentException();
+ }
out.write(b1 << shift | b2);
- } else {
+ } else if (isAttrChar(c)) {
out.write((byte) c);
+ } else {
+ throw new IllegalArgumentException();
}
}
return out.toByteArray();
diff --git
a/src/test/java/org/apache/commons/fileupload/RFC2231UtilityTestCase.java
b/src/test/java/org/apache/commons/fileupload/RFC2231UtilityTestCase.java
index 0a9945d0..9fbd73c4 100644
--- a/src/test/java/org/apache/commons/fileupload/RFC2231UtilityTestCase.java
+++ b/src/test/java/org/apache/commons/fileupload/RFC2231UtilityTestCase.java
@@ -84,4 +84,22 @@ public final class RFC2231UtilityTestCase {
final String nameWithoutAsterisk = "paramname";
assertEquals("paramname",
RFC2231Utility.stripDelimiter(nameWithoutAsterisk));
}
+
+
+ @Test
+ void testDecodeNonTokenCharacters() throws Exception {
+ assertThrows(IllegalArgumentException.class, () ->
RFC2231Utility.decodeText("ISO-8859-1''Not*allowed"));
+ }
+
+
+ @Test
+ void testDecodeUTF8Characters() throws Exception {
+ assertThrows(IllegalArgumentException.class, () ->
RFC2231Utility.decodeText("UTF-8''\\u8a2e"));
+ }
+
+
+ @Test
+ void testDecodeInvalidHex() throws Exception {
+ assertThrows(IllegalArgumentException.class, () ->
RFC2231Utility.decodeText("ISO-8859-1''hello%HHworld"));
+ }
}
