w3c-ccg / http-signatures

Signing HTTP Messages specification
https://w3c-dvcg.github.io/http-signatures/
Other
34 stars 9 forks source link

Forbid referring to zero-length header values. #84

Closed aljones15 closed 5 years ago

aljones15 commented 5 years ago

The reason behind this PR is the following:

During HTTP signature test suite creation I found passing an HTTP Message with a header that's value is only white spaces results in the header being dropped. This means that the existing rule to simply use the header field a colon and a white space results in test failure as when it goes to look for the header value there is no value for that header field.

This is a fix for one of the issues from: https://github.com/w3c-dvcg/http-signatures/issues/81

If other implementors could test how their HTTP message parsers treat headers with zero-length string values or white space only values that would help.

I tried this in express this morning and it unfortunately looks like the HTTP parser I am using for the binary in the tests is the outlier. Express did turn headers with zero length strings into what the specification expects.

aljones15 commented 5 years ago

I'm closing this as this does appear to be specific to my implementation and not as varied as I thought it might be. If it turns out your HTTP parser is treating Headers with zero length strings incorrectly then feel free to report it in the existing issue.

liamdennehy commented 5 years ago

Code works fine when the object is correctly created:

$expectedSigningString =
"(request-target): get /path\n" .
"thisisempty: \n" .
"date: Mon, 28 Jul 2014 15:39:13 -0700";

$headerList = new HeaderList(['(request-target)', 'thisisempty', 'date']);
$producedSigningStringObj = new SigningString(
  $headerList,
  $this->message->withHeader('ThisIsEmpty', '')
);
$producedSigningString = $producedSigningStringObj->string();
$this->assertEquals(
    $expectedSigningString,
    $producedSigningString
);

https://github.com/liamdennehy/http-signatures-php/blob/f3a4897e4e4719e2501068926868174ba3a13995/tests/SigningStringTest.php#L137

However it seems the client I am using (curl) refuses to send an empty header.

Simple command pipe:

user@host ~/src/http-signatures-php $ echo -e "GET /\nThisIsFull: Value\nThisIsEmpty: \n" | nc localhost 6789 | grep '{' | jq '.headers'
{
  "Host": [
    "localhost:6789"
  ],
  "ThisIsFull": [
    "Value"
  ],
  "ThisIsEmpty": [
    ""
  ]
}

Using curl:

user@host ~/src/http-signatures-php $ curl -s localhost:6789 -H "ThisIsFull: Value" -H "ThisIsEmpty: " | jq '.headers'
{
  "Host": [
    "localhost:6789"
  ],
  "User-Agent": [
    "curl/7.65.0"
  ],
  "Accept": [
    "*/*"
  ],
  "ThisIsFull": [
    "Value"
  ]
}

You need a weird syntax to force empty header transmission:

user@host ~/src/http-signatures-php $ curl -s localhost:6789 -H "ThisIsFull: Value" -H "ThisIsEmpty;" | jq '.headers'
{
  "Host": [
    "localhost:6789"
  ],
  "User-Agent": [
    "curl/7.65.0"
  ],
  "Accept": [
    "*/*"
  ],
  "ThisIsFull": [
    "Value"
  ],
  "ThisIsEmpty": [
    ""
  ]
}

Sources: https://github.com/alexeyshockov/guzzle/commit/2dd96c9ecbf59d089f9c09f5467f7129799e46f9 https://stackoverflow.com/questions/32909411/send-empty-http-header-with-libcurl https://curl.haxx.se/libcurl/c/httpcustomheader.html

aljones15 commented 5 years ago

@liamdennehy let's take this into the issue: https://github.com/w3c-dvcg/http-signatures/issues/81

locally I am finding that if I pass the HTTP message directly to my parser it does behave correctly, but in the tests it is still being dropped...