From b420294ffcaab9ea21b71d780f120f0b384e908a Mon Sep 17 00:00:00 2001 From: jiangfucheng Date: Tue, 21 Feb 2023 16:04:07 -0500 Subject: [PATCH 1/8] fix: invalidate cache in core.request.add_haeder and fix some calls --- apisix/core/request.lua | 37 +++++++++++++++-------- apisix/plugins/proxy-rewrite.lua | 6 ++-- t/core/request.t | 44 +++++++++++++++++++++++++++ t/plugin/proxy-rewrite3.t | 52 +++++++++++++++++++++++++++++++- 4 files changed, 123 insertions(+), 16 deletions(-) diff --git a/apisix/core/request.lua b/apisix/core/request.lua index 173fafc56928..5f0c95b85300 100644 --- a/apisix/core/request.lua +++ b/apisix/core/request.lua @@ -42,6 +42,7 @@ local req_get_body_file = ngx.req.get_body_file local req_get_post_args = ngx.req.get_post_args local req_get_uri_args = ngx.req.get_uri_args local req_set_uri_args = ngx.req.set_uri_args +local table_insert = table.insert local _M = {} @@ -108,16 +109,13 @@ function _M.header(ctx, name) return _headers(ctx)[name] end - -function _M.set_header(ctx, header_name, header_value) +local function modify_header(ctx, header_name, header_value, update_func, override) if type(ctx) == "string" then -- It would be simpler to keep compatibility if we put 'ctx' -- after 'header_value', but the style is too ugly! header_value = header_name header_name = ctx ctx = nil - - log.warn("DEPRECATED: use set_header(ctx, header_name, header_value) instead") end local err @@ -131,26 +129,41 @@ function _M.set_header(ctx, header_name, header_value) changed = a6_request.is_request_header_set() end - ngx.req.set_header(header_name, header_value) + update_func(header_name, header_value) if is_apisix_or and not changed then -- if the headers are not changed before, -- we can only update part of the cache instead of invalidating the whole a6_request.clear_request_header() if ctx and ctx.headers then - ctx.headers[header_name] = header_value + if override or not ctx.headers[header_name] then + ctx.headers[header_name] = header_value + else + local values = ctx.headers[header_name] + if type(values) == "table" then + table_insert(values, header_value) + else + ctx.headers[header_name] = {values, header_value} + end + end end end end -function _M.add_header(header_name, header_value) - local err - header_name, err = _validate_header_name(header_name) - if err then - error(err) +function _M.set_header(ctx, header_name, header_value) + if type(ctx) == "string" then + log.warn("DEPRECATED: use set_header(ctx, header_name, header_value) instead") + end + + modify_header(ctx, header_name, header_value, ngx.req.set_header, true) +end + +function _M.add_header(ctx, header_name, header_value) + if type(ctx) == "string" then + log.warn("DEPRECATED: use add_header(ctx, header_name, header_value) instead") end - req_add_header(header_name, header_value) + modify_header(ctx, header_name, header_value, req_add_header, false) end -- return the remote address of client which directly connecting to APISIX. diff --git a/apisix/plugins/proxy-rewrite.lua b/apisix/plugins/proxy-rewrite.lua index 65ffdf3abd4d..0308557ee44c 100644 --- a/apisix/plugins/proxy-rewrite.lua +++ b/apisix/plugins/proxy-rewrite.lua @@ -327,18 +327,18 @@ function _M.rewrite(conf, ctx) for i = 1, field_cnt, 2 do local val = core.utils.resolve_var(hdr_op.add[i + 1], ctx.var) local header = hdr_op.add[i] - core.request.add_header(header, val) + core.request.add_header(ctx, header, val) end local field_cnt = #hdr_op.set for i = 1, field_cnt, 2 do local val = core.utils.resolve_var(hdr_op.set[i + 1], ctx.var) - core.request.set_header(hdr_op.set[i], val) + core.request.set_header(ctx, hdr_op.set[i], val) end local field_cnt = #hdr_op.remove for i = 1, field_cnt do - core.request.set_header(hdr_op.remove[i], nil) + core.request.set_header(ctx, hdr_op.remove[i], nil) end end diff --git a/t/core/request.t b/t/core/request.t index eab9986d48f3..6286b8fa9c59 100644 --- a/t/core/request.t +++ b/t/core/request.t @@ -439,3 +439,47 @@ while ($i <= 101) { $s --- response_body 101 + + + +=== TEST 15: add header +--- config + location /t { + content_by_lua_block { + local core = require("apisix.core") + ngx.ctx.api_ctx = {} + local ctx = ngx.ctx.api_ctx + core.request.add_header(ctx, "test_header", "test") + local h = core.request.header(ctx, "test_header") + ngx.say(h) + core.request.add_header(ctx, "test_header", "t2") + local h2 = core.request.header(ctx, "test_header") + ngx.say("[" .. table.concat(h2, ", ") .. "]") + core.request.add_header(ctx, "test_header", "t3") + local h3 = core.request.header(ctx, "test_header") + ngx.say("[" .. table.concat(h3, ", ") .. "]") + } + } +--- response_body +test +[test, t2] +[test, t2, t3] + + + +=== TEST 16: call add_header with deprecated way +--- config + location /t { + content_by_lua_block { + local core = require("apisix.core") + ngx.ctx.api_ctx = {} + local ctx = ngx.ctx.api_ctx + core.request.add_header("test_header", "test") + local h = core.request.header(ctx, "test_header") + ngx.say(h) + } + } +--- response_body +test +--- error_log +DEPRECATED: use add_header(ctx, header_name, header_value) instead diff --git a/t/plugin/proxy-rewrite3.t b/t/plugin/proxy-rewrite3.t index 613bd1b1add0..ef98dfb79914 100644 --- a/t/plugin/proxy-rewrite3.t +++ b/t/plugin/proxy-rewrite3.t @@ -450,11 +450,14 @@ passed -=== TEST 20: set and test priority test +=== TEST 20: set and test priority test & deprecated calls test --- request GET /echo HTTP/1.1 --- response_headers test: test_in_set +--- no_error_log +DEPRECATED: use add_header(ctx, header_name, header_value) instead +DEPRECATED: use set_header(ctx, header_name, header_value) instead @@ -619,3 +622,50 @@ GET /test/plugin/proxy/rewrite HTTP/1.1 } --- response_body /plugin_proxy_rewrite?a=c + + + +=== TEST 27: set route +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "methods": ["GET"], + "plugins": { + "proxy-rewrite": { + "headers": { + "add":{"test": "t1"}, + "set":{"test": "t2"} + } + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/echo" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 28: deprecated calls test +--- request +GET /echo HTTP/1.1 +--- no_error_log +DEPRECATED: use add_header(ctx, header_name, header_value) instead +DEPRECATED: use set_header(ctx, header_name, header_value) instead \ No newline at end of file From 1280cde05370c8ba1e12ec99f800d8f982613b61 Mon Sep 17 00:00:00 2001 From: jiangfucheng Date: Tue, 21 Feb 2023 16:09:50 -0500 Subject: [PATCH 2/8] fix --- t/plugin/proxy-rewrite3.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/plugin/proxy-rewrite3.t b/t/plugin/proxy-rewrite3.t index ef98dfb79914..a8d10c1934bb 100644 --- a/t/plugin/proxy-rewrite3.t +++ b/t/plugin/proxy-rewrite3.t @@ -668,4 +668,4 @@ passed GET /echo HTTP/1.1 --- no_error_log DEPRECATED: use add_header(ctx, header_name, header_value) instead -DEPRECATED: use set_header(ctx, header_name, header_value) instead \ No newline at end of file +DEPRECATED: use set_header(ctx, header_name, header_value) instead From b8b86223c90e05847865e18993e49dd6b8deaab1 Mon Sep 17 00:00:00 2001 From: jiangfucheng Date: Tue, 21 Feb 2023 16:18:57 -0500 Subject: [PATCH 3/8] fix --- t/plugin/proxy-rewrite3.t | 1 + 1 file changed, 1 insertion(+) diff --git a/t/plugin/proxy-rewrite3.t b/t/plugin/proxy-rewrite3.t index a8d10c1934bb..53c21091cb21 100644 --- a/t/plugin/proxy-rewrite3.t +++ b/t/plugin/proxy-rewrite3.t @@ -669,3 +669,4 @@ GET /echo HTTP/1.1 --- no_error_log DEPRECATED: use add_header(ctx, header_name, header_value) instead DEPRECATED: use set_header(ctx, header_name, header_value) instead + From 90f2ee28a072f96926e572074a134d02ad38cc7d Mon Sep 17 00:00:00 2001 From: jiangfucheng Date: Tue, 21 Feb 2023 16:19:05 -0500 Subject: [PATCH 4/8] fix --- t/plugin/proxy-rewrite3.t | 1 - 1 file changed, 1 deletion(-) diff --git a/t/plugin/proxy-rewrite3.t b/t/plugin/proxy-rewrite3.t index 53c21091cb21..a8d10c1934bb 100644 --- a/t/plugin/proxy-rewrite3.t +++ b/t/plugin/proxy-rewrite3.t @@ -669,4 +669,3 @@ GET /echo HTTP/1.1 --- no_error_log DEPRECATED: use add_header(ctx, header_name, header_value) instead DEPRECATED: use set_header(ctx, header_name, header_value) instead - From 243569041956abc813f6cce78e3e63f9502a82e5 Mon Sep 17 00:00:00 2001 From: jiangfucheng Date: Thu, 23 Feb 2023 04:47:28 -0500 Subject: [PATCH 5/8] fix --- apisix/core/request.lua | 26 ++++++++++++++------------ t/core/request.t | 8 ++++---- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/apisix/core/request.lua b/apisix/core/request.lua index 5f0c95b85300..5279033f9739 100644 --- a/apisix/core/request.lua +++ b/apisix/core/request.lua @@ -109,13 +109,19 @@ function _M.header(ctx, name) return _headers(ctx)[name] end -local function modify_header(ctx, header_name, header_value, update_func, override) +local function modify_header(ctx, header_name, header_value, override) if type(ctx) == "string" then -- It would be simpler to keep compatibility if we put 'ctx' -- after 'header_value', but the style is too ugly! header_value = header_name header_name = ctx ctx = nil + + if override then + log.warn("DEPRECATED: use set_header(ctx, header_name, header_value) instead") + else + log.warn("DEPRECATED: use add_header(ctx, header_name, header_value) instead") + end end local err @@ -129,7 +135,11 @@ local function modify_header(ctx, header_name, header_value, update_func, overri changed = a6_request.is_request_header_set() end - update_func(header_name, header_value) + if override then + ngx.req.set_header(header_name, header_value) + else + req_add_header(header_name, header_value) + end if is_apisix_or and not changed then -- if the headers are not changed before, @@ -151,19 +161,11 @@ local function modify_header(ctx, header_name, header_value, update_func, overri end function _M.set_header(ctx, header_name, header_value) - if type(ctx) == "string" then - log.warn("DEPRECATED: use set_header(ctx, header_name, header_value) instead") - end - - modify_header(ctx, header_name, header_value, ngx.req.set_header, true) + modify_header(ctx, header_name, header_value, true) end function _M.add_header(ctx, header_name, header_value) - if type(ctx) == "string" then - log.warn("DEPRECATED: use add_header(ctx, header_name, header_value) instead") - end - - modify_header(ctx, header_name, header_value, req_add_header, false) + modify_header(ctx, header_name, header_value, false) end -- return the remote address of client which directly connecting to APISIX. diff --git a/t/core/request.t b/t/core/request.t index 6286b8fa9c59..19e89e7076ed 100644 --- a/t/core/request.t +++ b/t/core/request.t @@ -454,16 +454,16 @@ $s ngx.say(h) core.request.add_header(ctx, "test_header", "t2") local h2 = core.request.header(ctx, "test_header") - ngx.say("[" .. table.concat(h2, ", ") .. "]") + ngx.say(core.json.encode(h2)) core.request.add_header(ctx, "test_header", "t3") local h3 = core.request.header(ctx, "test_header") - ngx.say("[" .. table.concat(h3, ", ") .. "]") + ngx.say(core.json.encode(h3)) } } --- response_body test -[test, t2] -[test, t2, t3] +["test","t2"] +["test","t2","t3"] From 4a1486239d4d567d917c8609c8f8ae0aa3c2df62 Mon Sep 17 00:00:00 2001 From: jiangfucheng Date: Fri, 24 Feb 2023 04:56:32 -0500 Subject: [PATCH 6/8] fix test --- t/core/request.t | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/t/core/request.t b/t/core/request.t index 19e89e7076ed..9bf48ddbae8d 100644 --- a/t/core/request.t +++ b/t/core/request.t @@ -449,15 +449,16 @@ $s local core = require("apisix.core") ngx.ctx.api_ctx = {} local ctx = ngx.ctx.api_ctx + local json = require("toolkit.json") core.request.add_header(ctx, "test_header", "test") local h = core.request.header(ctx, "test_header") ngx.say(h) core.request.add_header(ctx, "test_header", "t2") local h2 = core.request.header(ctx, "test_header") - ngx.say(core.json.encode(h2)) + ngx.say(json.encode(h2)) core.request.add_header(ctx, "test_header", "t3") local h3 = core.request.header(ctx, "test_header") - ngx.say(core.json.encode(h3)) + ngx.say(json.encode(h3)) } } --- response_body From 151893f151ae6ada09d953e4ffb0c3067b8d600c Mon Sep 17 00:00:00 2001 From: Tristan <33349046+jiangfucheng@users.noreply.github.com> Date: Wed, 1 Mar 2023 13:09:23 +0800 Subject: [PATCH 7/8] fix\ --- t/plugin/proxy-rewrite3.t | 47 --------------------------------------- 1 file changed, 47 deletions(-) diff --git a/t/plugin/proxy-rewrite3.t b/t/plugin/proxy-rewrite3.t index a8d10c1934bb..eaea849cd432 100644 --- a/t/plugin/proxy-rewrite3.t +++ b/t/plugin/proxy-rewrite3.t @@ -622,50 +622,3 @@ GET /test/plugin/proxy/rewrite HTTP/1.1 } --- response_body /plugin_proxy_rewrite?a=c - - - -=== TEST 27: set route ---- config - location /t { - content_by_lua_block { - local t = require("lib.test_admin").test - local code, body = t('/apisix/admin/routes/1', - ngx.HTTP_PUT, - [[{ - "methods": ["GET"], - "plugins": { - "proxy-rewrite": { - "headers": { - "add":{"test": "t1"}, - "set":{"test": "t2"} - } - } - }, - "upstream": { - "nodes": { - "127.0.0.1:1980": 1 - }, - "type": "roundrobin" - }, - "uri": "/echo" - }]] - ) - - if code >= 300 then - ngx.status = code - end - ngx.say(body) - } - } ---- response_body -passed - - - -=== TEST 28: deprecated calls test ---- request -GET /echo HTTP/1.1 ---- no_error_log -DEPRECATED: use add_header(ctx, header_name, header_value) instead -DEPRECATED: use set_header(ctx, header_name, header_value) instead From ef49e7b68121dea057ed8ec1ee68d53b42013a3a Mon Sep 17 00:00:00 2001 From: Tristan <33349046+jiangfucheng@users.noreply.github.com> Date: Thu, 2 Mar 2023 14:15:47 +0800 Subject: [PATCH 8/8] fix --- apisix/core/request.lua | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apisix/core/request.lua b/apisix/core/request.lua index 5279033f9739..aa9dd03bf7bc 100644 --- a/apisix/core/request.lua +++ b/apisix/core/request.lua @@ -43,6 +43,7 @@ local req_get_post_args = ngx.req.get_post_args local req_get_uri_args = ngx.req.get_uri_args local req_set_uri_args = ngx.req.set_uri_args local table_insert = table.insert +local req_set_header = ngx.req.set_header local _M = {} @@ -136,7 +137,7 @@ local function modify_header(ctx, header_name, header_value, override) end if override then - ngx.req.set_header(header_name, header_value) + req_set_header(header_name, header_value) else req_add_header(header_name, header_value) end