Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Commit

Permalink
Bug fix: filter out header sources only if the value is an empty string
Browse files Browse the repository at this point in the history
A previous change checked for a truthy `$value` in SAPI header sources
within the `marshal_headers_from_sapi()` function, which could lead to
problems when the value was `0` or `'0'`.

This patch adds tests to prevent future changes from causing the issue
to re-surface, as well as a fix for the problem.
  • Loading branch information
oauth2-middleware authored and weierophinney committed Jan 5, 2019
1 parent 0c108cc commit c6ea797
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 6 deletions.
10 changes: 6 additions & 4 deletions src/functions/marshal_headers_from_sapi.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,21 @@ function marshalHeadersFromSapi(array $server) : array
}
}

if ($value && strpos($key, 'HTTP_') === 0) {
if ($value === '') {
continue;
}

if (strpos($key, 'HTTP_') === 0) {
$name = strtr(strtolower(substr($key, 5)), '_', '-');
$headers[$name] = $value;
continue;
}

if ($value && strpos($key, 'CONTENT_') === 0) {
if (strpos($key, 'CONTENT_') === 0) {
$name = 'content-' . strtolower(substr($key, 8));
$headers[$name] = $value;
continue;
}

$headers[$key] = $value;
}

return $headers;
Expand Down
38 changes: 36 additions & 2 deletions test/ServerRequestFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,23 @@ public function testMarshalsExpectedHeadersFromServerArray()
$this->assertSame($expected, marshalHeadersFromSapi($server));
}

public function testMarshalInvalidHeadersStrippedFromServerArray()
{
$server = [
'COOKIE' => 'COOKIE',
'HTTP_AUTHORIZATION' => 'token',
'MD5' => 'CONTENT-MD5',
'CONTENT_LENGTH' => 'UNSPECIFIED',
];

//Headers that don't begin with HTTP_ or CONTENT_ will not be returned
$expected = [
'authorization' => 'token',
'content-length' => 'UNSPECIFIED',
];
$this->assertSame($expected, marshalHeadersFromSapi($server));
}

public function testMarshalsVariablesPrefixedByApacheFromServerArray()
{
// Non-prefixed versions will be preferred
Expand Down Expand Up @@ -427,10 +444,27 @@ public function testFromGlobalsUsesCookieHeaderInsteadOfCookieSuperGlobal()
*/
public function testCreateFromGlobalsShouldPreserveKeysWhenCreatedWithAZeroValue()
{
$_SERVER['Accept'] = '0';
$_SERVER['HTTP_ACCEPT'] = '0';
$_SERVER['CONTENT_LENGTH'] = '0';

$request = ServerRequestFactory::fromGlobals();
$this->assertSame('0', $request->getHeaderLine('Accept'));
$this->assertSame('0', $request->getHeaderLine('accept'));
$this->assertSame('0', $request->getHeaderLine('content-length'));
}

/**
* @runInSeparateProcess
* @preserveGlobalState
*/
public function testCreateFromGlobalsShouldNotPreserveKeysWhenCreatedWithAnEmptyValue()
{
$_SERVER['HTTP_ACCEPT'] = '';
$_SERVER['CONTENT_LENGTH'] = '';

$request = ServerRequestFactory::fromGlobals();

$this->assertFalse($request->hasHeader('accept'));
$this->assertFalse($request->hasHeader('content-length'));
}

/**
Expand Down

0 comments on commit c6ea797

Please sign in to comment.