Skip to content

bugfix: disable http2 in body read due to http2 stream processing bug#2174

Merged
zhuizhuhaomeng merged 8 commits into
openresty:masterfrom
oowl:fix/read_body_api_http2_disable
May 11, 2023
Merged

bugfix: disable http2 in body read due to http2 stream processing bug#2174
zhuizhuhaomeng merged 8 commits into
openresty:masterfrom
oowl:fix/read_body_api_http2_disable

Conversation

@oowl

@oowl oowl commented Mar 29, 2023

Copy link
Copy Markdown
Contributor

I hereby granted the copyright of the changes in this pull request
to the authors of this lua-nginx-module project.

@oowl

oowl commented Mar 29, 2023

Copy link
Copy Markdown
Contributor Author

PR context #2172

@oowl
oowl force-pushed the fix/read_body_api_http2_disable branch from b30c569 to a411a42 Compare March 29, 2023 03:16
@oowl

oowl commented Mar 29, 2023

Copy link
Copy Markdown
Contributor Author

I don't know how to test this, because test-nginx can not request by HTTP2 protocol

@oowl oowl changed the title dbugfix: disable http2 in body read due to http2 stream processing bug bugfix: disable http2 in body read due to http2 stream processing bug Mar 29, 2023
@oowl

oowl commented Mar 31, 2023

Copy link
Copy Markdown
Contributor Author

@zhuizhuhaomeng Hi, Can you help me review this PR?

Comment thread src/ngx_http_lua_contentby.c Outdated
Comment thread src/ngx_http_lua_req_body.c Outdated
Comment thread src/ngx_http_lua_req_body.c Outdated
Comment thread src/ngx_http_lua_req_body.c Outdated
Comment thread src/ngx_http_lua_rewriteby.c Outdated
Comment thread src/ngx_http_lua_server_rewriteby.c Outdated
Comment thread README.markdown
@zhuizhuhaomeng

Copy link
Copy Markdown
Contributor

we need to add test cases for this PR @attenuation

@oowl
oowl requested a review from zhuizhuhaomeng April 17, 2023 07:45
@oowl

oowl commented Apr 23, 2023

Copy link
Copy Markdown
Contributor Author

@zhuizhuhaomeng Can you take a look of this PR?

Comment thread t/044-req-body.t Outdated
Comment thread t/024-access/request_body.t Outdated
Comment thread t/024-access/request_body.t
Comment thread t/023-rewrite/request_body.t Outdated
Comment thread t/023-rewrite/request_body.t
@oowl
oowl requested a review from zhuizhuhaomeng April 23, 2023 13:43
@oowl

oowl commented May 4, 2023

Copy link
Copy Markdown
Contributor Author

@zhuizhuhaomeng Hi, Can you help me look at this PR again?

Comment thread t/044-req-body.t Outdated
@oowl
oowl force-pushed the fix/read_body_api_http2_disable branch from d08da97 to 54c5762 Compare May 10, 2023 14:44
@oowl
oowl requested a review from zhuizhuhaomeng May 10, 2023 15:09
@zhuizhuhaomeng
zhuizhuhaomeng merged commit 0090f3f into openresty:master May 11, 2023

/* http2 read body may break http2 stream process */
#if (NGX_HTTP_V2)
if (r->main->stream) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Does this check are really needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have explain it in #2172 (comment)

@oowl
oowl deleted the fix/read_body_api_http2_disable branch September 15, 2023 03:19
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.

3 participants