Skip to content

escape crlf and quotes in streaming multipart part headers#3418

Merged
velo merged 1 commit into
OpenFeign:14.xfrom
alhudz:multipart-part-encoder-escape
Jun 17, 2026
Merged

escape crlf and quotes in streaming multipart part headers#3418
velo merged 1 commit into
OpenFeign:14.xfrom
alhudz:multipart-part-encoder-escape

Conversation

@alhudz

@alhudz alhudz commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Backport of #3417 to the streaming MultipartFormEncoder added in #3414, as requested there.

Repro: send a streamed multipart request whose part name or FormData filename contains " or a CRLF, e.g. evil"\r\nX-Injected: 1.
Cause: AbstractPartEncoder.createHeaders appends the field name and file name straight into the quoted Content-Disposition parameters, so the value can close the quote and start a new header line or part. Same issue as the legacy writers fixed in #3417, on the new streaming path.
Fix: percent-encode \r, \n and " in those parameters (escapeHeaderParameter), matching the WHATWG multipart/form-data encoding rules. Added AbstractPartEncoderTest covering an injected name and file name.

@velo velo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tightening the streaming multipart header generation. The escaping is covered for both field name and filename, it fixes a real header-injection risk, and I do not see a compatibility issue with the percent-encoded quoted parameter values.

@velo velo merged commit 7492ee6 into OpenFeign:14.x Jun 17, 2026
3 checks passed
velo pushed a commit that referenced this pull request Jun 23, 2026
* Revert "feat: create streamable multipart body encoder (#3414)"

This reverts commit 7163913

* Revert "escape crlf and quotes in streaming multipart part headers (#3418)"

This reverts commit 7492ee6
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.

3 participants