Copilot commented on code in PR #13642:
URL: https://github.com/apache/apisix/pull/13642#discussion_r3506167447
##########
apisix/plugins/forward-auth.lua:
##########
@@ -121,9 +121,9 @@ function _M.access(conf, ctx)
}
if conf.request_method == "POST" then
- auth_headers["Content-Length"] = core.request.header(ctx,
"content-length")
- auth_headers["Expect"] = core.request.header(ctx, "expect")
- auth_headers["Transfer-Encoding"] = core.request.header(ctx,
"transfer-encoding")
+ -- body is buffered and re-framed below, so only keep content-encoding.
+ -- forwarding client transfer-encoding/content-length/expect would not
+ -- match the buffered body.
auth_headers["Content-Encoding"] = core.request.header(ctx,
"content-encoding")
end
Review Comment:
When request_method is POST, this change stops implicitly forwarding framing
headers, but a user can still reintroduce the same inconsistency by configuring
`request_headers` or `extra_headers` to include `Transfer-Encoding`,
`Content-Length`, or `Expect`. Consider explicitly filtering/forbidding these
headers on the POST path (case-insensitive) so the auth request framing cannot
be made inconsistent via configuration.
##########
t/plugin/forward-auth.t:
##########
@@ -595,3 +595,89 @@ POST /withinlimit
--- response_body_like eval
qr/i-am-an-user/
--- error_code: 200
+
+
+
+=== TEST 20: setup route that reflects the framing seen by the auth service
+--- config
+ location /t {
+ content_by_lua_block {
+ local data = {
+ {
+ url = "/apisix/admin/routes/auth-frame",
+ data = [[{
+ "plugins": {
+ "serverless-pre-function": {
+ "phase": "rewrite",
+ "functions": [
+ "return function (conf, ctx)
+ local core = require(\"apisix.core\");
+ local te = core.request.header(ctx,
\"transfer-encoding\") or \"none\";
+
core.response.set_header(\"X-Auth-Saw-Te\", te);
+ core.response.exit(200);
+ end"
+ ]
+ }
+ },
+ "uri": "/auth-frame"
+ }]],
+ },
+ {
+ url = "/apisix/admin/routes/pingframe",
+ data = [[{
+ "plugins": {
+ "forward-auth": {
+ "uri": "http://127.0.0.1:1984/auth-frame";,
+ "request_method": "POST",
+ "upstream_headers": ["X-Auth-Saw-Te"]
+ },
+ "proxy-rewrite": {
+ "uri": "/echo"
+ }
+ },
+ "upstream_id": "u1",
+ "uri": "/pingframe"
+ }]],
+ },
+ }
+
+ local t = require("lib.test_admin").test
+ for _, data in ipairs(data) do
+ local code, body = t(data.url, ngx.HTTP_PUT, data.data)
+ ngx.say(body)
+ end
+ }
+ }
+--- request
+GET /t
+--- response_body eval
+"passed\n" x 2
+
+
+
+=== TEST 21: chunked POST is re-framed, auth service does not see chunked
+--- config
+ location /t {
+ content_by_lua_block {
+ local sock = ngx.socket.tcp()
+ sock:settimeout(2000)
+ local ok, err = sock:connect("127.0.0.1", 1984)
+ if not ok then
+ ngx.say("connect failed: ", err)
+ return
+ end
+ local req = "POST /pingframe HTTP/1.1\r\n"
+ .. "Host: 127.0.0.1\r\n"
+ .. "Transfer-Encoding: chunked\r\n"
+ .. "Connection: close\r\n\r\n"
+ .. "5\r\nhello\r\n0\r\n\r\n"
+ sock:send(req)
+ local resp = sock:receive("*a")
+ sock:close()
+ ngx.print(resp)
Review Comment:
This test uses a raw TCP socket but doesn't check `sock:send` /
`sock:receive` errors. If either fails, the test output can be empty or raise
an error, making failures harder to diagnose. Adding explicit error checks (and
printing the error) will make CI failures more actionable.
--
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]
