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

Malformed request causes 500 response #183

Closed
pavattt opened this issue Feb 16, 2024 · 7 comments
Closed

Malformed request causes 500 response #183

pavattt opened this issue Feb 16, 2024 · 7 comments
Labels
Bug Something isn't working Duplicate This issue or pull request already exists

Comments

@pavattt
Copy link

pavattt commented Feb 16, 2024

Bug Report

Q A
Version(s) 2.26

Summary

Malformed request causes 500 response when sending integers in header names.

Current behavior

Returns response with http code 500 for requests with invalid headers.

How to reproduce

Send random integer in header name in request.

Expected behavior

Returns response with http code 400 for requests with invalid headers.

If this exception code was set to 400 it would be used as http response code in ServerRequestErrorResponseGenerator.

https://github.com/laminas/laminas-diactoros/blob/2.26.x/src/HeaderSecurity.php#L152-L155

@pavattt pavattt added the Bug Something isn't working label Feb 16, 2024
@gsteel
Copy link
Member

gsteel commented Feb 16, 2024

It's pretty easy to trigger this exception, new Request('/foo', 'GET', 'php://temp', ['123' => 'Bar']); - PHP of course casts numeric keys to an integer, at which point HeaderSecurity::assertValidName() throws for the non-string header name.

From what I can tell, RFC 7230 does not prohibit numeric header names.

@Xerkus
Copy link
Member

Xerkus commented Feb 16, 2024

Duplicate of #11

@Xerkus Xerkus marked this as a duplicate of #11 Feb 16, 2024
@Xerkus Xerkus closed this as not planned Won't fix, can't repro, duplicate, stale Feb 16, 2024
@Xerkus Xerkus added the Duplicate This issue or pull request already exists label Feb 16, 2024
@gsteel
Copy link
Member

gsteel commented Feb 16, 2024

Further reference: #159

@Xerkus - has anything changed WRT PSR-7 at fig?

@Xerkus
Copy link
Member

Xerkus commented Feb 16, 2024

Not that I am aware of. I don't expect anything to change except some errata may be.

This really boils down to a php bug that php wont fix and it can only be mitigated by type casting each and every time array key is read. Not really feasible.

@gsteel
Copy link
Member

gsteel commented Feb 16, 2024

Internally, headers could be list<array{name: non-empty-string, value: list<non-empty-string>}> - slower perhaps… at this point, users cannot expect an integer for a header name, so "fixing" the problem couldn't be considered a BC break right?

@Xerkus
Copy link
Member

Xerkus commented Feb 16, 2024

Please read original issue comments and look at closed PR attempting to solve it.

@Xerkus
Copy link
Member

Xerkus commented Feb 16, 2024

It is impossible to fix or work around:
public function getHeaders(): array;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

3 participants