Skip to content

Commit

Permalink
fix(proxy): fix request buffering for HTTP/2.0
Browse files Browse the repository at this point in the history
### Summary

Turning request/response buffering off for HTTP/2.0 is not possible.
There is a check that causes buffering only to be controllable for HTTP/1.1.

This was probably done because of an issue in nginx, which was fixed in
version 1.9.14 (http://nginx.org/en/docs/http/ngx_http_v2_module.html):

> Before version 1.9.14, buffering of a client request body could not be
> disabled regardless of [proxy_request_buffering](http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_request_buffering),
> [fastcgi_request_buffering](http://nginx.org/en/docs/http/ngx_http_fastcgi_module.html#fastcgi_request_buffering),
> [uwsgi_request_buffering](http://nginx.org/en/docs/http/ngx_http_uwsgi_module.html#uwsgi_request_buffering), and
> [scgi_request_buffering](http://nginx.org/en/docs/http/ngx_http_scgi_module.html#scgi_request_buffering) directive values.

Kong now has Nginx > 1.9.14, so the check is not needed any more.

The work was done by @PidgeyBE, thank you very much!

### Issues Resolved

Fix #7418
Close #10204

Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>
  • Loading branch information
PidgeyBE authored and bungle committed Mar 31, 2023
1 parent 887c8d2 commit 9547c53
Show file tree
Hide file tree
Showing 7 changed files with 219 additions and 48 deletions.
3 changes: 2 additions & 1 deletion .devcontainer/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ RUN apt-get install -y \
unzip \
git \
m4 \
libyaml-dev
libyaml-dev \
libcurl4-openssl-dev
8 changes: 4 additions & 4 deletions .devcontainer/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ services:
- ..:/workspace:cached

# Uncomment the next line to use Docker from inside the container. See https://aka.ms/vscode-remote/samples/docker-from-docker-compose for details.
- /var/run/docker.sock:/var/run/docker.sock
- /var/run/docker.sock:/var/run/docker.sock

# Uncomment the next four lines if you will use a ptrace-based debugger like C++, Go, and Rust.
cap_add:
- SYS_PTRACE
Expand All @@ -37,12 +37,12 @@ services:
CRYPTO_DIR: /usr/local/kong

# Overrides default command so things don't shut down after the process ends.
command: /bin/sh -c "while sleep 1000; do :; done"
command: /bin/sh -c "while sleep 1000; do :; done"

# Runs app on the same network as the service container, allows "forwardPorts" in devcontainer.json function.
network_mode: service:db

# Use "forwardPorts" in **devcontainer.json** to forward an app port locally.
# Use "forwardPorts" in **devcontainer.json** to forward an app port locally.
# (Adding the "ports" property to this file will not forward from a Codespace.)

# Uncomment the next line to use a non-root user for all processes - See https://aka.ms/vscode-remote/containers/non-root for details.
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/build_and_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ jobs:

- name: Install packages
if: steps.cache-deps.outputs.cache-hit != 'true'
run: sudo apt update && sudo apt install libyaml-dev valgrind libprotobuf-dev
run: sudo apt update && sudo apt install libyaml-dev valgrind libprotobuf-dev libcurl4-openssl-dev

- name: Build Kong
if: steps.cache-deps.outputs.cache-hit != 'true'
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@
[#10389](https://github.com/Kong/kong/pull/10389)
- Support for configurable Node IDs
[#10385](https://github.com/Kong/kong/pull/10385)
- Request and response buffering options are now enabled for incoming HTTP 2.0 requests too.
Thanks [@PidgeyBE](https://github.com/PidgeyBE) for contributing this change.
[#10595](https://github.com/Kong/kong/pull/10595)
[#10204](https://github.com/Kong/kong/pull/10204)

#### Admin API

Expand Down
6 changes: 4 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
OS := $(shell uname | awk '{print tolower($$0)}')
MACHINE := $(shell uname -m)

DEV_ROCKS = "busted 2.1.2" "busted-htest 1.0.0" "luacheck 1.1.0" "lua-llthreads2 0.1.6" "http 0.4" "ldoc 1.4.6"
DEV_ROCKS = "busted 2.1.2" "busted-htest 1.0.0" "luacheck 1.1.0" "lua-llthreads2 0.1.6" "http 0.4" "ldoc 1.4.6" "Lua-cURL 0.3.13"
WIN_SCRIPTS = "bin/busted" "bin/kong" "bin/kong-health"
BUSTED_ARGS ?= -v
TEST_CMD ?= bin/busted $(BUSTED_ARGS)
Expand All @@ -12,10 +12,12 @@ ifeq ($(OS), darwin)
OPENSSL_DIR ?= $(shell brew --prefix)/opt/openssl
GRPCURL_OS ?= osx
YAML_DIR ?= $(shell brew --prefix)/opt/libyaml
CURL_INCDIR ?= $(shell brew --prefix)/opt/curl/include
else
OPENSSL_DIR ?= /usr
GRPCURL_OS ?= $(OS)
YAML_DIR ?= /usr
CURL_INCDIR ?= /usr/include/x86_64-linux-gnu
endif

ifeq ($(MACHINE), aarch64)
Expand Down Expand Up @@ -77,7 +79,7 @@ install-dev-rocks: build-venv
else \
echo $$rock not found, installing via luarocks... ; \
LIBRARY_PREFIX=$$(pwd)/bazel-bin/build/$(BUILD_NAME)/kong ; \
luarocks install $$rock OPENSSL_DIR=$$LIBRARY_PREFIX CRYPTO_DIR=$$LIBRARY_PREFIX YAML_DIR=$(YAML_DIR) || exit 1; \
luarocks install $$rock OPENSSL_DIR=$$LIBRARY_PREFIX CRYPTO_DIR=$$LIBRARY_PREFIX YAML_DIR=$(YAML_DIR) CURL_INCDIR=$(CURL_INCDIR) || exit 1; \
fi \
done;

Expand Down
18 changes: 8 additions & 10 deletions kong/runloop/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1280,18 +1280,16 @@ return {
return exec("@grpc")
end

if protocol_version == 1.1 then
if route.request_buffering == false then
if route.response_buffering == false then
return exec("@unbuffered")
end

return exec("@unbuffered_request")
end

if route.request_buffering == false then
if route.response_buffering == false then
return exec("@unbuffered_response")
return exec("@unbuffered")
end

return exec("@unbuffered_request")
end

if route.response_buffering == false then
return exec("@unbuffered_response")
end
end
end,
Expand Down
Loading

0 comments on commit 9547c53

Please sign in to comment.