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

refactor(plugins) removing ngx.req.raw_header() for HTTP/2 compatibility #2540

Merged
merged 1 commit into from
Jun 21, 2017

Conversation

subnetmarco
Copy link
Member

@subnetmarco subnetmarco commented May 17, 2017

  • Enables HTTP/2 compatibility by removing ngx.req.raw_header() invocation calls.

return "GET /request/path HTTP/1.1\r\n"..
"Host: mockbin.com\r\n"..
"Accept: application/json\r\n"..
"Accept: application/x-www-form-urlencoded\r\n\r\n\r\n"
Copy link
Member Author

Choose a reason for hiding this comment

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

Required to avoid double escaping of \r.

size = size + #k + 2 + #tostring(y) + 2 --First 2 is semicolon + space
end
else
size = size + #k + 1 + 1 + #tostring(v) + 2 --First 2 is semicolon + space
Copy link
Contributor

Choose a reason for hiding this comment

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

why 1 + 1 instead 2? what makes it different from line 90?

size = size + #k + 1 + 1 + #tostring(v) + 2 --First 2 is semicolon + space
end
end
return #request_line + 2 + size + 4 -- 2 it's \r\n, 4 it's trailing \r\n\r\n
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you already counting one \r\n when you calculating the size of last header, so should't the final sum be like

#request_line + 2 + size + 2

@shashiranjan84 shashiranjan84 added pr/changes requested Changes were requested to this PR by a maintainer. Please address them and ping back once done. and removed pr/status/please review labels May 18, 2017
@thibaultcha thibaultcha modified the milestone: 0.11.0 May 19, 2017
@subnetmarco subnetmarco added pr/status/please review and removed pr/changes requested Changes were requested to this PR by a maintainer. Please address them and ping back once done. labels May 22, 2017
@subnetmarco
Copy link
Member Author

Updated.

@subnetmarco subnetmarco removed the request for review from thibaultcha June 7, 2017 18:07
shashiranjan84
shashiranjan84 previously approved these changes Jun 7, 2017
@p0pr0ck5
Copy link
Contributor

(fixed merge conflict and squashed)

@p0pr0ck5 p0pr0ck5 merged commit a89c0c6 into master Jun 21, 2017
@p0pr0ck5 p0pr0ck5 deleted the refactor/raw-header branch June 21, 2017 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants