Skip to content

Ignore invalid RFC 2231 extended value so it cannot erase a valid filename#479

Merged
garydgregory merged 1 commit into
apache:masterfrom
alhudz:rfc2231-invalid-ext-value
Jul 18, 2026
Merged

Ignore invalid RFC 2231 extended value so it cannot erase a valid filename#479
garydgregory merged 1 commit into
apache:masterfrom
alhudz:rfc2231-invalid-ext-value

Conversation

@alhudz

@alhudz alhudz commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

When ParameterParser.parse decodes an RFC 2231 / RFC 5987 extended value (such as filename*) and the decode throws IllegalArgumentException, it set the value to null and then unconditionally stored it. Because filename and filename* both strip to the key filename, a malformed filename* clobbers a valid filename for the same part. Repro: parse("form-data; name=\"file\"; filename=\"safe.txt\"; filename*=UTF-8'bad%2", new char[]{',',';'}). Expected: filename stays safe.txt. Actual: filename is null, so getFileName returns an empty name. A client controlling the multipart Content-Disposition header can therefore blank the filename of an uploaded file and slip past filename-based checks in callers.

The existing comment already states the intent (Treat invalid values as if they were not provided), but setting paramValue = null and putting it does the opposite for a parameter that also has a valid sibling value. The fix flags the invalid decode and skips storing that parameter, so an invalid extended value is genuinely ignored and cannot overwrite a valid one, regardless of ordering. Keeping this in parse means every caller (getFileName, getFieldName, getBoundary, charset lookup) gets the corrected behaviour without its own guard. Covered by a new ParameterParserTest case; the core suite passes (72 tests).

@garydgregory garydgregory changed the title ignore invalid RFC 2231 extended value so it cannot erase a valid filename Ignore invalid RFC 2231 extended value so it cannot erase a valid filename Jul 18, 2026
@garydgregory
garydgregory merged commit e45f383 into apache:master Jul 18, 2026
9 checks passed
@garydgregory

Copy link
Copy Markdown
Member

Thank you @alhudz , merged 🚀 Please port to 1.x if applicable.

@alhudz

alhudz commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for merging. 1.x has the same pattern in ParameterParser.parse (null value still stored after stripDelimiter), so it does apply; ported in #480 with the same regression test.

garydgregory pushed a commit that referenced this pull request Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants