Skip to content
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 limits to HTTP request parsing #4428

Merged
merged 1 commit into from Jun 2, 2017

Conversation

RX14
Copy link
Contributor

@RX14 RX14 commented May 18, 2017

Fixes #1043.

A lot of servers simply use a 8KiB limit to the whole header, but it's easier not to here. We use larger limits than most servers, so we should be safe.

@akzhan
Copy link
Contributor

akzhan commented May 19, 2017

Any benchmarks before and after?

Reason: Changes to http request parsing touch a lot of users, so we need any benchmark tool/results for it.

@RX14
Copy link
Contributor Author

RX14 commented May 19, 2017

Before

HTTP Parse 682.26k (  1.47µs) (± 3.70%) fastest

After

HTTP Parse 679.46k (  1.47µs) (± 4.41%) fastest

Running the benchmark multiple times, it doesn't seem that there's any noticeable difference (the results are quite variable)

@akzhan
Copy link
Contributor

akzhan commented May 19, 2017

Thanks, @RX14 . Will glad to see automation on it :)

@mverzilli mverzilli added this to the Next milestone Jun 2, 2017
@mverzilli mverzilli merged commit 2c8fead into crystal-lang:master Jun 2, 2017
@mverzilli
Copy link

Thanks @RX14 !

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

Successfully merging this pull request may close these issues.

None yet

4 participants