Skip to content

bugfix: revert #2174 and #2237 due to http2 behavior break change#2286

Closed
oowl wants to merge 6 commits into
openresty:masterfrom
oowl:revert-req-body
Closed

bugfix: revert #2174 and #2237 due to http2 behavior break change#2286
oowl wants to merge 6 commits into
openresty:masterfrom
oowl:revert-req-body

Conversation

@oowl

@oowl oowl commented Feb 28, 2024

Copy link
Copy Markdown
Contributor

revert #2174 and #2237 due to http2 behavior break change, context: #2211

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

@oowl oowl changed the title fix: revert #2174 and #2237 due to http2 behavior break change fixs: revert #2174 and #2237 due to http2 behavior break change Feb 28, 2024
@dndx

dndx commented Feb 28, 2024

Copy link
Copy Markdown
Member

Could you also add document for the behavior of client_max_body_size and client_body_timeout and their effect on ngx.req.read_body() as well?

@oowl

oowl commented Feb 28, 2024

Copy link
Copy Markdown
Contributor Author

Could you also add document for the behavior of client_max_body_size and client_body_timeout and their effect on ngx.req.read_body() as well?

On the way

@oowl oowl changed the title fixs: revert #2174 and #2237 due to http2 behavior break change bugfix: revert #2174 and #2237 due to http2 behavior break change Feb 28, 2024

@dndx dndx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We do not need to mention gRPC specifically, the read time out and body size limit applies to all HTTP versions.

Comment thread README.markdown Outdated
Comment thread README.markdown Outdated
Comment thread doc/HttpLuaModule.wiki Outdated
@oowl
oowl requested a review from dndx February 28, 2024 07:21
dndx
dndx previously approved these changes Feb 28, 2024
@oowl

oowl commented Feb 28, 2024

Copy link
Copy Markdown
Contributor Author

@zhuizhuhaomeng @swananan Hi, Can you help me to review this?

dndx pushed a commit to Kong/kong that referenced this pull request Feb 29, 2024
…oding limitation (#12658)

Cherry picked from openresty/lua-nginx-module#2286.

It was acknowledged by OpenResty as a mistaken breaking change, and
we should revert it.

FTI-5766
FTI-5795
github-actions Bot pushed a commit to Kong/kong that referenced this pull request Feb 29, 2024
…oding limitation (#12658)

Cherry picked from openresty/lua-nginx-module#2286.

It was acknowledged by OpenResty as a mistaken breaking change, and
we should revert it.

FTI-5766
FTI-5795

(cherry picked from commit 55358df)
Comment thread t/044-req-body.t
repeat_each(2);

plan tests => repeat_each() * (blocks() * 4 + 56);
plan tests => repeat_each() * (blocks() * 4 + 58 );

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

An extra space, I will remove it on merging.

@zhuizhuhaomeng
zhuizhuhaomeng dismissed dndx’s stale review March 1, 2024 00:58

The merge-base changed after approval.

@mergify

mergify Bot commented Mar 1, 2024

Copy link
Copy Markdown

This pull request is now in conflict :(

@mergify mergify Bot added the conflict label Mar 1, 2024
@zhuizhuhaomeng

Copy link
Copy Markdown
Contributor

I have merged this PR with a small modification.
Please check out the latest master code.

@mergify mergify Bot removed the conflict label Mar 1, 2024
windmgc pushed a commit to Kong/kong that referenced this pull request Mar 1, 2024
…oding limitation (#12658)

Cherry picked from openresty/lua-nginx-module#2286.

It was acknowledged by OpenResty as a mistaken breaking change, and
we should revert it.

FTI-5766
FTI-5795

(cherry picked from commit 55358df)
ogercyprien added a commit to opendatasoft/lua-resty-waf that referenced this pull request Aug 26, 2024
In lua-nginx-module, the changed introduced in PR #2237, which required
an adpatation in lua-resty-waf in commi be3428d, was reverted.
See openresty/lua-nginx-module#2286
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.

4 participants