Skip to content
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.

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

🩹 Correctly split headers without leading whitespace #402

Merged
merged 2 commits into from
Sep 24, 2024

Conversation

RafaelKr
Copy link
Contributor

Fixes #401.

According to https://datatracker.ietf.org/doc/html/rfc7230#section-3.2 headers may contain optional leading whitespace before the value. The previous implementation threw an error if there was no whitespace.

According to https://datatracker.ietf.org/doc/html/rfc7230#section-3.2 headers may contain optional leading whitespace before the value. The previous implementation threw an error if there was no whitespace.
Fixes roots#401.
@QWp6t QWp6t changed the title Correctly split headers without leading whitespace 🩹 Correctly split headers without leading whitespace Sep 24, 2024
@QWp6t QWp6t merged commit ef4a8a2 into roots:main Sep 24, 2024
1 check failed
@QWp6t
Copy link
Member

QWp6t commented Sep 24, 2024

Thank you! 🙏

@RafaelKr
Copy link
Contributor Author

@QWp6t No problem!

I built a little Benchmark: https://github.com/RafaelKr/php-header-split-bench
Usage and the output/results I got are documented in the README.md.

As you can see by just using explode the header values are actually modified because one leading space is added.
Usually (and according to RFC) this shouldn't be a problem, but it definitely modifies the current behavior:

{
    "X-With-Leading-Space": " Test: 123",
    "X-With-Leading-Space-And-Colon": " Test: 123",
    "X-Without-Leading-Space": "Test-Header",
    "Content-Type": " application\/x-www-form-urlencoded",
    "Cache-Control": " private, must-revalidate"
}

The same behavior can also be observed in real usage with Laravel/Acorn.

I think using preg_split wouldn't really impact performance but provide consistent output independent of leading whitespace.

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.

ErrorException: Undefined array key 1 with header('x-test:header');
2 participants