Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

HTTP Headers Case Sensitivity #921

Closed
mikegreiling opened this Issue May 6, 2013 · 3 comments

Comments

Projects
None yet
3 participants
Contributor

mikegreiling commented May 6, 2013

Having recently been playing around a bit with Curl and Lithium's http Request/Response classes, I've run into a few small issues.

One of which is the ambiguity of header case sensitivity. See this stackoverflow thread.

Since there is no enforced standard on header case, I need to check multiple possibilities when looking for headers in a remote server response.

$type = $response->header('Content-Type') ?:
    $response->header('content-type') ?:
    $response->header('CoNtEnT-TyPe')

* granted, the first two account for 99.99% of the world's servers.

I would like to propose that our \net\http\Message::headers() accessor method make this transparent to the user; that $response->header('xxx') produce the corresponding header regardless of letter casing in the parameter or the header it is targeting.

There are two ways to go about this:

  1. Create a protected Message::$_headerFields variable which maps strtolower headers to their canonical key within Message::$headers and is populated as headers are assigned using the accessor method... The "canonical" key will just be whichever key is encountered first. If \net\http\Response::_parseMessage() reads in 'Content-type', then $headers['Content-type'] is set as normal, and $_headerFields['content-type'] is set to 'Content-type'.
    There could be some issues if someone manipulates the public $headers property directly, but for most cases it would work fine.
  2. Store all headers keys in lowercase and convert them with ucwords when re-rendering.
    i.e. str_replace(' ', '-', ucwords(strtolower(str_replace('-', ' ', $name))));

I like the first option, personally. It doesn't mess with the original case of the message headers getting read into \net\http\Response, so you'll get something closer to the true representation of the response with __toString(), but maybe that's just my OCD talking.

The second option is less complicated, but may introduce unnecessary overhead with the string conversion on output (this would effect all \net\http\Request in Socket requests, in addition to \action\Response in all page loads, so I'd like to do whichever requires the least overhead).

I can write the PR for this. I'd just like some input on the solution.

Contributor

jails commented May 6, 2013

Personnaly I would just replace the following if https://github.com/UnionOfRAD/lithium/blob/master/net/http/Message.php#L111 with:

$name = str_replace(' ', '-', ucwords(strtolower(str_replace('-', ' ', $match[1]))));
$this->headers[$name] = trim($match[2]);

Imo the issue is more related of how headers are "imported" from requests and this way we can keep the "ucworded" convention Lithium side.

Contributor

mikegreiling commented May 6, 2013

Ah, that may actually be a better way to do it...

I tend to obsess over making all cases of $message == $response->to('string'); where $response = new Response(array('message' => $message));, but that's perhaps an unnecessary ideal to strive for as long as the representation stays functionally the same.

@davidpersson davidpersson added rfc and removed enhancement labels Sep 25, 2014

Owner

davidpersson commented Sep 25, 2014

The headers() method is already doing a lot of things (https://github.com/UnionOfRAD/lithium/blob/dev/net/http/Message.php#L88) In my opinion adding additional functionality there makes that beast too complex. At the same time this method is called quite often and lies within a performance critical path. Despite this I like the enhancement and understand the reason behind it. As a workaround you may use:

preg_grep('/^content-type\:/i', $response->headers());

Anyway, thanks for opening the issue and describing precisely.

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