Copilot commented on code in PR #13348:
URL: https://github.com/apache/apisix/pull/13348#discussion_r3232574478
##########
t/certs/sse_server.key:
##########
@@ -0,0 +1,28 @@
+-----BEGIN PRIVATE KEY-----
+MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDFGBdWQztEX6GW
+YNi0zhPyNG9RIsrLGJI3iUcAiyQrZnkSvORVEKzt6vwoEbhChCQbZcyLWW95a/rz
+K+4TXfZvX520iBf3hmcHK8+em0WW9BUttKOkjlAGZxi6RUIkUV6HE76C0vm5fX+e
+awBZSDlwg+rd8DcfyGcrrMwvAwiftmFMpMN5QZEtmchwGFVmN8T8JWmiC3w+50nj
+O4mfovjiXFaxBZ+X2T5pZLFhFXHPqRMtaX6khFqK/s1BwMuZ2BkKQOVBEkQZMN5w
+5HtyxhanrNId8EB8ss5ijG0+a0BpSGQ0FLQlWREqPUpwNdO2nPEHzxLJMr5bEwYi
+uqki/k1HAgMBAAECggEALSIUqgDQUBp0FssLpO+x+ptOSG6msLZqOUR62WGDgVrA
+a+2MffxJFVxjrMtN/hFjcVCw89IhqFT1TP0o0g+I0L09EGu/zUNeUXKTYzccSvKO
+7P36IUMjiSvPqkwU1ts5QcZgMHYekH7wG/dVx5w15xGWVYdeIC2UjphN05Amx+el
+XkWJsRg5zQadfgGpX875J86sdY/KBWYeA2lxpxZ33EbEfHbuJvLFPqJ69FN8mOAf
+6SFcyjShP8ovpuhX/DT/g3hi7dQtRR6YoKb7LBiNwFkpdNkEA8rWMkcA1F0NHlf+
+KEs9O9iD+im/v62KYJtZTCQoPNuFVArPVUH0i0HMKQKBgQDlw+ygKV5pdUDTHuhA
+lVlEokh3oa7MFGywh1T5OUVvUqCSJ5NPX5L+3cAENS4uLje/3hM5gCIyVAfN0ri7
+OiXpeR+D2fq1js5ccKXvJNQy6knxDnB2BLAlPipSp4Iih6PMVF0hKvyzBl2p3nTT
+S7fqFaDc6sIOBsWWRJcghS7XrwKBgQDbmS6XG+VBTZLRtMmkBPv+2XGcaH9s9lI/
+4UmYDvUZi4bFqtSrKKi0WkpNCP6x4TXzH41v/bJqmQqs40Y1ZctraEt/rV6nR0E5
+IgSAkTFXB2T7Cjhcvt9Xj0QfU01+4mIRBkDQnbjrcCu1QOtDISGi5nQtK0QAd6UD
+/aykToSx6QKBgAwFjUrv/y2bYfHp6xL9/Xa22v3Patrosqsl2Y9UrMpfU2FySqXb
+hVBqf9J4idsGtgoG75CRoLhrZyEgxmOdbkBiAwEeFZ0MRMXXawcxMR0c3xOKwt2Z
+7zFzqDk85HU0DaDyRREoM6KWUa5ConAvxQatbQZCDjc3qXzsR8/+x+2nAoGBAMA0
+uVTFs8mOrl0ikgMf4bjUdd5ikHW8u4zyEUoofVsYhqPovC/7bH4/MR1wLA1hg6kD
+Cvbk5Q7sWS2t17vRF1UxejOMeXaMpYfuQGaPrtHvxPD9pwt2fWHUIdoRPZk7aH5i
+LMTr5/kauwbwhXrCOwCsGS+X2PNXxXVSyZMeroJRAoGBAL5d2VhMevFerYzuPA8f
+yNMT4H5IOW9XeB/fxJGqOdVyn3aY3TBoNXFSQKLWnK/0DuKB7nwB/klQ9hL8OW9u
+QfDXwtXRk0XznUDcxjcx723KdRRipEZd4QaZNEPfhaadmB1jZN8rNDSXJFqhmN4g
+ydkZABH8wRc0Bc1qI/Gr/bRe
+-----END PRIVATE KEY-----
Review Comment:
Committing a private key into the repository is a sensitive security risk
even if intended only for tests (it can be reused elsewhere, gets copied into
forks, and triggers secret-scanning). Prefer generating a self-signed cert/key
during the test run, or reuse an existing non-sensitive test certificate
fixture already present in the repo (and document that it’s for tests only).
##########
t/test_sse.py:
##########
@@ -0,0 +1,74 @@
+from threading import Thread
+import asyncio
+from aiohttp import web
+from aiohttp.web import Response
+from aiohttp_sse import sse_response
+from aiohttp_sse import EventSourceResponse
+from datetime import datetime
+import time
+import pprint
+import sseclient
+import sys
+import os
+import ssl
+
+
+test_ssl = len(sys.argv) == 2 and sys.argv[1] == "ssl"
+
+
+class Response(EventSourceResponse):
+ def __init__(self, **args):
+ super().__init__(**args)
+ del self.headers["X-Accel-Buffering"]
Review Comment:
`del self.headers["X-Accel-Buffering"]` will raise `KeyError` if that header
is not present (implementation details of `aiohttp_sse` can change). Use a safe
removal (e.g., `pop` with a default) so the test doesn’t fail due to missing
header rather than proxy behavior.
##########
t/test_sse.py:
##########
@@ -0,0 +1,74 @@
+from threading import Thread
+import asyncio
+from aiohttp import web
+from aiohttp.web import Response
+from aiohttp_sse import sse_response
+from aiohttp_sse import EventSourceResponse
+from datetime import datetime
+import time
+import pprint
+import sseclient
+import sys
+import os
+import ssl
+
+
+test_ssl = len(sys.argv) == 2 and sys.argv[1] == "ssl"
+
+
+class Response(EventSourceResponse):
Review Comment:
The local class `Response` shadows the imported `aiohttp.web.Response`,
which makes the module harder to read/debug and can confuse type expectations.
Rename the subclass to something specific (e.g.,
`SseResponse`/`NoAccelBufferingResponse`) and remove/adjust the conflicting
import accordingly.
##########
t/test_sse.py:
##########
@@ -0,0 +1,74 @@
+from threading import Thread
+import asyncio
+from aiohttp import web
+from aiohttp.web import Response
Review Comment:
The local class `Response` shadows the imported `aiohttp.web.Response`,
which makes the module harder to read/debug and can confuse type expectations.
Rename the subclass to something specific (e.g.,
`SseResponse`/`NoAccelBufferingResponse`) and remove/adjust the conflicting
import accordingly.
##########
t/APISIX.pm:
##########
@@ -224,6 +224,43 @@ $grpc_location .= <<_EOC_;
}
_EOC_
+my $disable_proxy_buffering_location = <<_EOC_;
+ location \@disable_proxy_buffering {
+ access_by_lua_block {
+ apisix.disable_proxy_buffering_access_phase()
+ }
+
+ proxy_http_version 1.1;
+ proxy_set_header Host \$upstream_host;
+ proxy_set_header Upgrade \$upstream_upgrade;
+ proxy_set_header Connection \$upstream_connection;
+ proxy_set_header X-Real-IP \$remote_addr;
+ proxy_pass_header Date;
+
+ proxy_set_header X-Forwarded-For
\$proxy_add_x_forwarded_for;
+ proxy_set_header X-Forwarded-Proto \$var_x_forwarded_proto;
+ proxy_set_header X-Forwarded-Host \$var_x_forwarded_host;
+ proxy_set_header X-Forwarded-Port \$var_x_forwarded_port;
+
+ proxy_pass
\$upstream_scheme://apisix_backend\$upstream_uri;
+ mirror /proxy_mirror;
Review Comment:
The test harness Nginx template always adds `mirror /proxy_mirror;` in
`@disable_proxy_buffering`, while the runtime template
(`apisix/cli/ngx_tpl.lua`) only adds `mirror` when `proxy-mirror` is enabled.
This divergence can cause test-only behavior and makes the template harder to
keep consistent; align the conditional behavior in `t/APISIX.pm` with
`ngx_tpl.lua` (or ensure `proxy_mirror` location is always configured in tests
when the directive is present).
##########
apisix/plugins/proxy-buffering.lua:
##########
@@ -0,0 +1,53 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements. See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local require = require
+local ngx = ngx
+local core = require("apisix.core")
+
+
+local schema = {
+ type = "object",
+ properties = {
+ disable_proxy_buffering = {
+ type = "boolean",
+ default = false,
+ },
+ },
+}
Review Comment:
The schema does not restrict unknown fields, so typos in configuration
(e.g., `disable_proxy_bufferin`) may be silently accepted depending on schema
defaults in `core.schema.check`. If the project’s plugin schemas typically
reject unknown properties, add `additionalProperties = false` to make config
validation stricter and errors easier to catch.
##########
.ignore_words:
##########
@@ -9,3 +9,4 @@ nulll
smove
aks
nin
+bre
Review Comment:
Adding a very short, common token like `bre` to the global ignore list can
mask real typos across the repo (e.g., truncated words). If this is intended
for a specific domain term, consider using a more specific ignore entry (or fix
the underlying spelling issue where it appears) to avoid reducing spell-check
signal.
##########
apisix/cli/ngx_tpl.lua:
##########
@@ -947,6 +947,46 @@ http {
}
{% end %}
+ {% if enabled_plugins["proxy-buffering"] then %}
+ location @disable_proxy_buffering {
+ access_by_lua_block {
+ apisix.disable_proxy_buffering_access_phase()
+ }
+
+ proxy_http_version 1.1;
+ proxy_set_header Host $upstream_host;
+ proxy_set_header Upgrade $upstream_upgrade;
+ proxy_set_header Connection $upstream_connection;
+ proxy_set_header X-Real-IP $remote_addr;
+ proxy_pass_header Date;
+
+ proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
+ proxy_set_header X-Forwarded-Proto $var_x_forwarded_proto;
+ proxy_set_header X-Forwarded-Host $var_x_forwarded_host;
+ proxy_set_header X-Forwarded-Port $var_x_forwarded_port;
+
+ proxy_pass $upstream_scheme://apisix_backend$upstream_uri;
+
+ {% if enabled_plugins["proxy-mirror"] then %}
+ mirror /proxy_mirror;
+ {% end %}
+
+ header_filter_by_lua_block {
+ apisix.http_header_filter_phase()
+ }
+
+ body_filter_by_lua_block {
+ apisix.http_body_filter_phase()
+ }
+
+ log_by_lua_block {
+ apisix.http_log_phase()
+ }
+
+ proxy_buffering off;
+ }
Review Comment:
This introduces a second proxying location that largely duplicates the
standard HTTP proxy location’s directives. That duplication tends to drift over
time (e.g., missing future header directives, TLS/proxy settings, timeouts,
etc.). Consider factoring shared proxy directives into a common
include/template fragment used by both locations, so only the `proxy_buffering
off;` delta is unique.
##########
docs/en/latest/plugins/proxy-buffering.md:
##########
@@ -0,0 +1,109 @@
+---
+title: proxy-buffering
+keywords:
+ - Apache APISIX
+ - API Gateway
+ - Proxy Buffering
+description: The proxy-buffering Plugin disables nginx proxy buffering per
route to enable streaming responses such as Server-Sent Events (SSE).
+---
+
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+<head>
+ <link rel="canonical" href="https://docs.api7.ai/hub/proxy-buffering"; />
+</head>
+
+## Description
+
+The `proxy-buffering` Plugin disables nginx proxy buffering for the configured
route. When proxy buffering is disabled, nginx streams the upstream response
directly to the client without accumulating it in memory or on disk first.
+
+This is particularly useful for:
+
+- **Server-Sent Events (SSE)**: Clients must receive events in real time;
buffering would delay or break the stream.
+- **Streaming APIs**: Large or indefinite response bodies must flow
continuously without waiting for the full body.
+- **Real-time data delivery**: Any use case requiring low-latency delivery of
partial responses.
+
+## Attributes
+
+| Name | Type | Required | Default | Description
|
+| ------------------------- | ------- | -------- | ------- |
---------------------------------------------------------------------------------------------
|
+| disable_proxy_buffering | boolean | False | false | When set to
`true`, disables
[`proxy_buffering`](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffering)
for this route, enabling streaming responses. |
Review Comment:
In the attributes table, the “Required” column uses `False` (capitalized
boolean) which is inconsistent with typical docs conventions (`Yes/No` or
`true/false`). Standardizing this improves clarity and consistency across
plugin docs.
##########
t/plugin/test_proxy_buffering.sh:
##########
@@ -0,0 +1,52 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+# Admin API curl wrapper
+# c [get|put|post|delete|...] <resource path> <any curl args> ...
+c() {
+ method=${1^^}
+ resource=$2
+ shift 2
+ curl
${ADMIN_SCHEME:-http}://${ADMIN_IP:-127.0.0.1}:${ADMIN_PORT:-9180}/apisix/admin${resource}
\
+ -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X $method "$@"
Review Comment:
`curl` exits with status 0 on many HTTP error responses (e.g., 400/409/500),
so with `set -e` this script can silently proceed after a failed Admin API
call. Add a failing option (`--fail`/`--fail-with-body`) and quote `"$method"`
to make failures reliably break the test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
