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

Header names are not case-sensitive #51

Closed
marcj opened this issue Mar 19, 2016 · 7 comments · Fixed by #103 or #107
Closed

Header names are not case-sensitive #51

marcj opened this issue Mar 19, 2016 · 7 comments · Fixed by #103 or #107
Assignees
Milestone

Comments

@marcj
Copy link

marcj commented Mar 19, 2016

https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2

Each header field consists of a name followed by a colon (":") and the field value. Field names are case-insensitive

Currently the code heavily relies on case-sensitive names.

Also Content-Type can contain a charset or additional information after ; character. Example: Content-Type: application/x-www-form-urlencoded; charset=utf-8 Such header break currently content parsing because of this.

We tried to fix it in PPM: https://github.com/php-pm/php-pm/blob/master/React/RequestParser.php#L18, but I believe we should fix it directly in reactphp/http.

@WyriHaximus
Copy link
Member

Thanks for reporting and you're absolutely right, this has been on my mind for a while. I'm in favor of strtolower all the header names.

@clue what is your take on this?

@WyriHaximus
Copy link
Member

Or could go for a class that behaves as an array and makes them case-insensitive

@clue
Copy link
Member

clue commented Mar 20, 2016

but I believe we should fix it directly in reactphp/http

Absolutely! 👍

Afaict most occurrences will be replaced with PR #41 anyway, but we should definitely keep an eye on this.

I'm in favor of strtolower all the header names.

Yeah, this should do it for now (http://php.net/array_change_key_case).

Eventually we will look into supporting PSR-7 (#28), by then we will also follow proper message semantics while preserving header case information.

FWIW, RFC 2616 has been replaced with this:

Each header field consists of a case-insensitive field name followed by […]
http://tools.ietf.org/html/rfc7230#section-3.2

@pwhelan
Copy link

pwhelan commented Mar 22, 2016

I actually went ahead and implemented a fix in PR #53 since I needed a fix for a work project. Hopefully #41 gets merged soon so I can drop my fix.

@andig
Copy link
Contributor

andig commented Oct 23, 2016

Will we ever see #41? It's a year in making now (sad).

@WyriHaximus
Copy link
Member

@andig It's nearing completion see #41 (comment) see the issues mentioned in that comment. You're input on those is valued 👍

@clue
Copy link
Member

clue commented Feb 10, 2017

I'm currently looking into this and will file a PR for the upcoming v0.4.4 release. See #103 for the first step here.

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