Skip to content

Add max size to request header parser #241

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.

Sign up for GitHub

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

Closed
christoph-kluge wants to merge 5 commits into from
Closed

Add max size to request header parser #241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5b07ac9
Added option to allow a custom buffer size for the request header
christoph-kluge Sep 7, 2017
44a2b9e
Usage of maxSize in RequestHeaderParse should fail early
christoph-kluge Sep 8, 2017
5f303c2
Provide php backwards compatibility for tests with exceptions
christoph-kluge Oct 15, 2017
abe8e00
Moved indention from 2 to 4 spaces to be more consistent with existin…
christoph-kluge Oct 15, 2017
475803b
Moved indention from 2 to 4 spaces to be more consistent with existin…
christoph-kluge Oct 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/RequestHeaderParser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,20 @@
class RequestHeaderParser extends EventEmitter
{
private $buffer = '';
private $maxSize = 4096;
private $maxSize;

private $localSocketUri;
private $remoteSocketUri;

public function __construct($localSocketUri = null, $remoteSocketUri = null)
public function __construct($localSocketUri = null, $remoteSocketUri = null, $maxSize = 4096)
{
if (!is_integer($maxSize)) {
throw new \InvalidArgumentException('Invalid type for maxSize provided. Expected an integer value.');
}
Copy link
Member

@clue clue Nov 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how much value this check provides? I would vote to remove this and replace this with a docblock (which can be replaced by PHP 7+ typehints eventually).

Copy link
Contributor Author

@christoph-kluge christoph-kluge Nov 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good spot! 👍 Since I will need to change this anyway for #242 would it be ok to keep it like that for now and remove it in #242?

Copy link
Contributor Author

@christoph-kluge christoph-kluge Nov 17, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clue how do we want to continue here?


$this->localSocketUri = $localSocketUri;
$this->remoteSocketUri = $remoteSocketUri;
$this->maxSize = $maxSize;
}

public function feed($data)
Expand Down
31 changes: 31 additions & 0 deletions tests/RequestHeaderParserTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@

class RequestHeaderParserTest extends TestCase
{

public function testMaxSizeParameterShouldFailOnWrongType()
{
$this->setExpectedException('InvalidArgumentException', 'Invalid type for maxSize provided. Expected an integer value.');

new RequestHeaderParser(null, null, 'abc');
}

public function testSplitShouldHappenOnDoubleCrlf()
{
$parser = new RequestHeaderParser();
Expand Down Expand Up @@ -124,6 +132,29 @@ public function testHeaderEventViaHttpsShouldApplySchemeFromConstructor()
$this->assertEquals('example.com', $request->getHeaderLine('Host'));
}

public function testCustomBufferSizeOverflowShouldEmitError()
{
$error = null;
$passedParser = null;
$newCustomBufferSize = 1024 * 16;

$parser = new RequestHeaderParser(null, null, $newCustomBufferSize);
$parser->on('headers', $this->expectCallableNever());
$parser->on('error', function ($message, $parser) use (&$error, &$passedParser) {
$error = $message;
$passedParser = $parser;
});

$this->assertSame(1, count($parser->listeners('headers')));
$this->assertSame(1, count($parser->listeners('error')));

$data = str_repeat('A', $newCustomBufferSize + 1);
$parser->feed($data);

$this->assertInstanceOf('OverflowException', $error);
$this->assertSame('Maximum header size of ' . $newCustomBufferSize . ' exceeded.', $error->getMessage());
}

public function testHeaderOverflowShouldEmitError()
{
$error = null;
Expand Down