-
-
Notifications
You must be signed in to change notification settings - Fork 142
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
Add max size to request header parser #241
Add max size to request header parser #241
Conversation
tests/RequestHeaderParserTest.php
Outdated
$this->assertInstanceOf('OverflowException', $error); | ||
$this->assertSame('Maximum header size of ' . $newCustomBufferSize . ' exceeded.', $error->getMessage()); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indenting seems off here, otherwise LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for breaking this down into smaller PRs, much appreciated! 👍
{ | ||
if (!is_integer($maxSize)) { | ||
throw new \InvalidArgumentException('Invalid type for maxSize provided. Expected an integer value.'); | ||
} |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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?
Moved this to |
@WyriHaximus waiting for @clue's feedback to continue here. |
Split-off from #217 in order to solve #214. This PR adds a customisable maximum request-header size.