Skip to content

Header name can't have OWS before semicolon #1147

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

Draft
joanhey wants to merge 3 commits into
base: master
Choose a base branch
from
+17 −9
Draft

Header name can't have OWS before semicolon #1147

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
103134b
Header name can't have OWS before semicolon
joanhey Apr 3, 2026
5595295
Add tests for invalid OWS placement in HTTP headers
joanhey Apr 3, 2026
c43c732
Fix regex to disallow duplicate Content-Length headers in HTTP requests
joanhey Apr 3, 2026
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
18 changes: 9 additions & 9 deletions src/Protocols/Http.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,18 +132,18 @@ public static function input(string $buffer, TcpConnection $connection): int
// The pattern uses case-insensitive modifier (~i) for header name matching.
$headerValidatePattern = '~\A'
// Optional: capture Content-Length value (must be at \A to scan entire header)
. '(?:(?=[\s\S]*\r\nContent-Length[ \t]*:[ \t]*(\d+)[ \t]*\r\n))?'
. '(?:(?=[\s\S]*\r\nContent-Length:[ \t]*(\d+)[ \t]*\r\n))?'
// Disallow Transfer-Encoding header
. '(?![\s\S]*\r\nTransfer-Encoding[ \t]*:)'
. '(?![\s\S]*\r\nTransfer-Encoding:)'
// If Content-Length header exists, its value must be pure digits + optional OWS
. '(?![\s\S]*\r\nContent-Length[ \t]*:(?![ \t]*\d+[ \t]*\r\n)[^\r]*\r\n)'
. '(?![\s\S]*\r\nContent-Length:(?![ \t]*\d+[ \t]*\r\n)[^\r]*\r\n)'
// Disallow duplicate Content-Length headers (adjacent or separated)
. '(?![\s\S]*\r\nContent-Length[ \t]*:[^\r\n]*\r\n(?:[\s\S]*?\r\n)?Content-Length[ \t]*:)'
. '(?![\s\S]*\r\nContent-Length:[^\r\n]*\r\n(?:[\s\S]*?\r\n)?Content-Length:)'
// Match request line: METHOD SP request-target SP HTTP-version CRLF
. '(?:GET|POST|OPTIONS|HEAD|DELETE|PUT|PATCH) +\/[^\x00-\x20\x7f]* +HTTP\/1\.[01]\r\n~i';

if (!preg_match($headerValidatePattern, $header, $matches)) {
if (preg_match('~\r\nTransfer-Encoding[ \t]*:~i', $header)) {
if (preg_match('~\r\nTransfer-Encoding:~i', $header)) {
return static::inputChunked($buffer, $connection, $header, $length);
}
$connection->end(static::HTTP_400, true);
Expand Down Expand Up @@ -181,9 +181,9 @@ public static function input(string $buffer, TcpConnection $connection): int
protected static function inputChunked(string $buffer, TcpConnection $connection, string $header, int $headerLength): int
{
$pattern = '~\A'
. '(?![\s\S]*\r\nContent-Length[ \t]*:)'
. '(?![\s\S]*\r\nTransfer-Encoding[ \t]*:[\s\S]*\r\nTransfer-Encoding[ \t]*:)'
. '(?=[\s\S]*\r\nTransfer-Encoding[ \t]*:[ \t]*chunked[ \t]*\r\n)'
. '(?![\s\S]*\r\nContent-Length:)'
. '(?![\s\S]*\r\nTransfer-Encoding:[\s\S]*\r\nTransfer-Encoding[ \t]*:)'
. '(?=[\s\S]*\r\nTransfer-Encoding:[ \t]*chunked[ \t]*\r\n)'
. '(?:GET|POST|OPTIONS|HEAD|DELETE|PUT|PATCH) +\/[^\x00-\x20\x7f]* +HTTP\/1\.[01]\r\n~i';

if (!preg_match($pattern, $header)) {
Expand Down Expand Up @@ -286,7 +286,7 @@ public static function decode(string $buffer, TcpConnection $connection): mixed
*/
protected static function decodeChunked(string $buffer, int $headerEnd): array
{
$header = preg_replace('~\r\nTransfer-Encoding[ \t]*:[^\r]*~i', '', substr($buffer, 0, $headerEnd), 1);
$header = preg_replace('~\r\nTransfer-Encoding:[^\r]*~i', '', substr($buffer, 0, $headerEnd), 1);
$body = '';
$trailers = [];
$pos = $headerEnd + 4;
Expand Down
8 changes: 8 additions & 0 deletions tests/Unit/Protocols/HttpTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,14 @@
'CRLF injection attempt in request-target' => [
"GET /foo\r\nX: y HTTP/1.1\r\n\r\n",
],
// OWS is not allowed between the field name and colon because it would interfere with header parsing and validation regexes. (OWS is allowed after the colon and around the value, but not before the colon.)
'OWS between Content-Length :' => [
"GET / HTTP/1.1\r\n\Content-Length : 0\r\n\r\n",
],
'OWS between Transfer-Encoding :' => [
"GET / HTTP/1.1\r\n\Transfer-Encoding : chunked\r\n\r\n",
],
// We need more tests about leading OWS before other headers to ensure it is properly rejected and does not interfere with header parsing and validation. For example, if leading OWS before Content-Length is not rejected, it could allow smuggling of a second Content-Length header that would be parsed by the regexes as the only Content-Length header, bypassing the duplicate Content-Length header check and allowing a request with multiple Content-Length headers, which is forbidden by the HTTP spec and can cause security issues.
]);

it('rejects Transfer-Encoding and bad/duplicate Content-Length in ::input', function (string $buffer, ?string $expectedCloseContains = '400 Bad Request') {
Expand Down
Loading