Skip to content

Disallow newlines in CURLOPT_HTTPHEADER#22651

Open
Sjord wants to merge 4 commits into
php:masterfrom
Sjord:headers-without-newlines
Open

Disallow newlines in CURLOPT_HTTPHEADER#22651
Sjord wants to merge 4 commits into
php:masterfrom
Sjord:headers-without-newlines

Conversation

@Sjord

@Sjord Sjord commented Jul 9, 2026

Copy link
Copy Markdown

It was possible to have multiple lines in a HTTP headers in a cURL request, which then get interpreted as multiple headers. This commit changes that to throw a ValueError when using newlines or carriage returns in a string passed as header.

It was possible to have multiple lines in a HTTP headers in a cURL
request, which then get interpreted as multiple headers. This commit
changes that to throw a ValueError when using newlines or carriage
returns in a string passed as header.
@Sjord Sjord requested a review from adoy as a code owner July 9, 2026 09:38
@Sjord

Sjord commented Jul 9, 2026

Copy link
Copy Markdown
Author

@TimWolla TimWolla 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.

Thank you!

Comment thread ext/curl/interface.c Outdated
Comment thread ext/curl/interface.c Outdated
Comment thread ext/curl/interface.c Outdated
Sjord added 2 commits July 9, 2026 12:23
Based on PR review comments. I'll do the NUL byte check separately.
This is already done for string options in php_curl_option_str
(see bug #68089). Similarly, array arguments should also not contain NUL
bytes, so we throw the same error for these.
Comment thread ext/curl/interface.c Outdated

@TimWolla TimWolla 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.

LGTM, but there were some concerns raised on the mailing list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants