-
Notifications
You must be signed in to change notification settings - Fork 2k
bugfix: first part headers will be discard when using single LF as delimiter #1204
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
Conversation
@spacewander Will you have a look at this? Thanks! |
t/104-req-raw-header.t
Outdated
|
||
|
||
|
||
=== TEST 30: large headers(using single LF as linefeed) |
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.
I suggest to use line break
instead of linefeed
, since linefeed
may be confused with LF
.
t/104-req-raw-header.t
Outdated
=== TEST 30: large headers(using single LF as linefeed) | ||
--- config | ||
location /t { | ||
content_by_lua ' |
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.
New test cases should use content_by_lua_block
instead.
…limiter and headers number is large enough
@spacewander @agentzh The code has been updated. |
@agentzh @spacewander ping. Any plan for this? |
@tokers Will have a look at this. |
|
||
|
||
|
||
=== TEST 31: large headers without request line(using single LF as line break) |
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.
Better add a space before (
for aesthetic reasons.
|
||
|
||
|
||
=== TEST 32: large headers with leading CRLF(using single LF as line break) |
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.
Ditto.
|
||
|
||
|
||
=== TEST 30: large headers(using single LF as line break) |
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.
Like below.
|
||
|
||
|
||
=== TEST 33: large headers without request line but contains leading CRLF(using single LF as line break) |
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.
Ditto.
@tokers Merged with fixes for the aforementioned minor issues. Thanks! |
I hereby granted the copyright of the changes in this pull request
to the authors of this lua-nginx-module project.
When the delimiter is the single LF and the request has large headers,
ngx.req.raw_header
will discard the first part headers.