From 688f76684657d35ec25f43d165b46f27143966da Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Thu, 14 Sep 2023 00:24:59 +0800 Subject: [PATCH 1/6] fix: graphql request route matching exception --- apisix/http/route.lua | 8 ++++---- apisix/http/router/radixtree_host_uri.lua | 12 ++++++------ apisix/http/router/radixtree_uri.lua | 4 +--- apisix/http/router/radixtree_uri_with_parameter.lua | 4 +--- apisix/init.lua | 4 ---- 5 files changed, 12 insertions(+), 20 deletions(-) diff --git a/apisix/http/route.lua b/apisix/http/route.lua index d475646b56c6..a157eea4345d 100644 --- a/apisix/http/route.lua +++ b/apisix/http/route.lua @@ -84,7 +84,7 @@ function _M.create_radixtree_uri_router(routes, uri_routes, with_parameter) handler = function (api_ctx, match_opts) api_ctx.matched_params = nil api_ctx.matched_route = route - api_ctx.curr_req_matched = match_opts.matched + api_ctx.curr_req_matched = core.table.deepcopy(match_opts.matched) end }) @@ -103,15 +103,15 @@ function _M.create_radixtree_uri_router(routes, uri_routes, with_parameter) end -function _M.match_uri(uri_router, match_opts, api_ctx) - core.table.clear(match_opts) +function _M.match_uri(uri_router, api_ctx) + local match_opts = core.tablepool.fetch("route_match_opts", 0, 16) match_opts.method = api_ctx.var.request_method match_opts.host = api_ctx.var.host match_opts.remote_addr = api_ctx.var.remote_addr match_opts.vars = api_ctx.var - match_opts.matched = core.tablepool.fetch("matched_route_record", 0, 4) local ok = uri_router:dispatch(api_ctx.var.uri, match_opts, api_ctx, match_opts) + core.tablepool.release("route_match_opts", match_opts) return ok end diff --git a/apisix/http/router/radixtree_host_uri.lua b/apisix/http/router/radixtree_host_uri.lua index 532576e53d4a..891d7c283b78 100644 --- a/apisix/http/router/radixtree_host_uri.lua +++ b/apisix/http/router/radixtree_host_uri.lua @@ -78,10 +78,11 @@ local function push_host_router(route, host_routes, only_uri_routes) vars = route.value.vars, filter_fun = filter_fun, handler = function (api_ctx, match_opts) + local matched = core.table.deepcopy(match_opts.matched) api_ctx.matched_params = nil api_ctx.matched_route = route - api_ctx.curr_req_matched = match_opts.matched - api_ctx.real_curr_req_matched_path = match_opts.matched._path + api_ctx.curr_req_matched = matched + api_ctx.real_curr_req_matched_path = matched._path end } @@ -142,8 +143,6 @@ local function create_radixtree_router(routes) return true end - - local match_opts = {} function _M.match(api_ctx) local user_routes = _M.user_routes local _, service_version = get_services() @@ -162,12 +161,11 @@ end function _M.matching(api_ctx) core.log.info("route match mode: radixtree_host_uri") - core.table.clear(match_opts) + local match_opts = core.tablepool.fetch("route_match_opts", 0, 16) match_opts.method = api_ctx.var.request_method match_opts.remote_addr = api_ctx.var.remote_addr match_opts.vars = api_ctx.var match_opts.host = api_ctx.var.host - match_opts.matched = core.tablepool.fetch("matched_route_record", 0, 4) if host_router then local host_uri = api_ctx.var.host @@ -181,11 +179,13 @@ function _M.matching(api_ctx) api_ctx.curr_req_matched._host = api_ctx.real_curr_req_matched_host:reverse() api_ctx.real_curr_req_matched_host = nil end + core.tablepool.release("route_match_opts", match_opts) return true end end local ok = only_uri_router:dispatch(api_ctx.var.uri, match_opts, api_ctx, match_opts) + core.tablepool.release("route_match_opts", match_opts) return ok end diff --git a/apisix/http/router/radixtree_uri.lua b/apisix/http/router/radixtree_uri.lua index 6e546364ac14..7c1b5c0c147a 100644 --- a/apisix/http/router/radixtree_uri.lua +++ b/apisix/http/router/radixtree_uri.lua @@ -27,7 +27,6 @@ local _M = {version = 0.2} local uri_routes = {} local uri_router - local match_opts = {} function _M.match(api_ctx) local user_routes = _M.user_routes local _, service_version = get_services() @@ -51,8 +50,7 @@ end function _M.matching(api_ctx) core.log.info("route match mode: radixtree_uri") - - return base_router.match_uri(uri_router, match_opts, api_ctx) + return base_router.match_uri(uri_router, api_ctx) end diff --git a/apisix/http/router/radixtree_uri_with_parameter.lua b/apisix/http/router/radixtree_uri_with_parameter.lua index 4bf7f3ebee5f..3f10f4fcac49 100644 --- a/apisix/http/router/radixtree_uri_with_parameter.lua +++ b/apisix/http/router/radixtree_uri_with_parameter.lua @@ -27,7 +27,6 @@ local _M = {} local uri_routes = {} local uri_router - local match_opts = {} function _M.match(api_ctx) local user_routes = _M.user_routes local _, service_version = get_services() @@ -51,8 +50,7 @@ end function _M.matching(api_ctx) core.log.info("route match mode: radixtree_uri_with_parameter") - - return base_router.match_uri(uri_router, match_opts, api_ctx) + return base_router.match_uri(uri_router, api_ctx) end diff --git a/apisix/init.lua b/apisix/init.lua index 86b68cf62208..17cce08a5cad 100644 --- a/apisix/init.lua +++ b/apisix/init.lua @@ -883,10 +883,6 @@ function _M.http_log_phase() core.tablepool.release("plugins", api_ctx.plugins) end - if api_ctx.curr_req_matched then - core.tablepool.release("matched_route_record", api_ctx.curr_req_matched) - end - core.tablepool.release("api_ctx", api_ctx) end From 7999699057c3f9b0d9abc26367a2668233ce326f Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Thu, 14 Sep 2023 06:06:05 +0800 Subject: [PATCH 2/6] add test cases --- t/cli/test_route_match_with_graphql.sh | 98 ++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 t/cli/test_route_match_with_graphql.sh diff --git a/t/cli/test_route_match_with_graphql.sh b/t/cli/test_route_match_with_graphql.sh new file mode 100644 index 000000000000..3d94e721a830 --- /dev/null +++ b/t/cli/test_route_match_with_graphql.sh @@ -0,0 +1,98 @@ +#!/usr/bin/env bash + +# +# 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. +# + +. ./t/cli/common.sh + +echo ' +deployment: + role: data_plane + role_data_plane: + config_provider: yaml + +apisix: + router: + http: radixtree_uri + +nginx_config: + worker_processes: 1 + +' > conf/config.yaml + +echo ' +routes: + - uri: "/hello" + hosts: + - test.com + vars: + - - "graphql_name" + - "==" + - "createAccount" + priority: 30 + id: "graphql1" + upstream_id: "invalid" + + - uri: "/hello" + hosts: + - test.com + plugins: + echo: + body: "test server\n" + priority: 20 + id: "graphql2" + upstream_id: "invalid" + + - uri: "/hello" + hosts: + - test2.com + plugins: + echo: + body: "test2\n" + priority: 20 + id: "graphql3" + upstream_id: "invalid" + +upstreams: + - nodes: + 127.0.0.1:1999: 1 + id: "invalid" +#END +' > conf/apisix.yaml + +make run + +dd if=/dev/urandom of=tmp_data.json bs=300K count=1 + +for i in {1..100}; do + curl -s http://127.0.0.1:9080/hello -H "Host: test.com" -H "Content-Type: application/json" -X POST -d @tmp_data.json > /tmp/graphql_request1.txt & + curl -s http://127.0.0.1:9080/hello -H "Host: test2.com" -H "Content-Type: application/json" -X POST -d @tmp_data.json > /tmp/graphql_request2.txt & + + wait + + if diff /tmp/graphql_request1.txt /tmp/graphql_request2.txt > /dev/null; then + make stop + echo "failed: route match error in GraphQL requests, route should not be the same" + exit 1 + fi +done + +make stop + +rm tmp_data.json /tmp/graphql_request1.txt /tmp/graphql_request2.txt + +echo "passed: GraphQL requests can be correctly matched to the route" \ No newline at end of file From f8a4593c18d3f3f71b59f169ae583246c5565c48 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Thu, 14 Sep 2023 09:12:17 +0800 Subject: [PATCH 3/6] update match_opts.matched --- apisix/http/route.lua | 1 + apisix/http/router/radixtree_host_uri.lua | 1 + 2 files changed, 2 insertions(+) diff --git a/apisix/http/route.lua b/apisix/http/route.lua index a157eea4345d..1bb99052fae9 100644 --- a/apisix/http/route.lua +++ b/apisix/http/route.lua @@ -109,6 +109,7 @@ function _M.match_uri(uri_router, api_ctx) match_opts.host = api_ctx.var.host match_opts.remote_addr = api_ctx.var.remote_addr match_opts.vars = api_ctx.var + match_opts.matched = {} local ok = uri_router:dispatch(api_ctx.var.uri, match_opts, api_ctx, match_opts) core.tablepool.release("route_match_opts", match_opts) diff --git a/apisix/http/router/radixtree_host_uri.lua b/apisix/http/router/radixtree_host_uri.lua index 891d7c283b78..b812dd62970e 100644 --- a/apisix/http/router/radixtree_host_uri.lua +++ b/apisix/http/router/radixtree_host_uri.lua @@ -166,6 +166,7 @@ function _M.matching(api_ctx) match_opts.remote_addr = api_ctx.var.remote_addr match_opts.vars = api_ctx.var match_opts.host = api_ctx.var.host + match_opts.matched = {} if host_router then local host_uri = api_ctx.var.host From 526ccfc8c657609a52e2c01ccfedf28b608d6f64 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Thu, 14 Sep 2023 09:16:15 +0800 Subject: [PATCH 4/6] update sh --- t/cli/test_route_match_with_graphql.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/cli/test_route_match_with_graphql.sh b/t/cli/test_route_match_with_graphql.sh index 3d94e721a830..917d5b4162e4 100644 --- a/t/cli/test_route_match_with_graphql.sh +++ b/t/cli/test_route_match_with_graphql.sh @@ -56,7 +56,7 @@ routes: priority: 20 id: "graphql2" upstream_id: "invalid" - + - uri: "/hello" hosts: - test2.com @@ -95,4 +95,4 @@ make stop rm tmp_data.json /tmp/graphql_request1.txt /tmp/graphql_request2.txt -echo "passed: GraphQL requests can be correctly matched to the route" \ No newline at end of file +echo "passed: GraphQL requests can be correctly matched to the route" From cfd93b9a053554ffb73f1259e7e42d312110691c Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Thu, 14 Sep 2023 09:32:13 +0800 Subject: [PATCH 5/6] update route match sh --- t/cli/test_route_match_with_graphql.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) mode change 100644 => 100755 t/cli/test_route_match_with_graphql.sh diff --git a/t/cli/test_route_match_with_graphql.sh b/t/cli/test_route_match_with_graphql.sh old mode 100644 new mode 100755 index 917d5b4162e4..c67027748146 --- a/t/cli/test_route_match_with_graphql.sh +++ b/t/cli/test_route_match_with_graphql.sh @@ -52,7 +52,7 @@ routes: - test.com plugins: echo: - body: "test server\n" + body: "test server" priority: 20 id: "graphql2" upstream_id: "invalid" @@ -62,7 +62,7 @@ routes: - test2.com plugins: echo: - body: "test2\n" + body: "test2" priority: 20 id: "graphql3" upstream_id: "invalid" From 2372e00259a4b7eae4ef91aad0e5971059bbad8f Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Thu, 14 Sep 2023 11:45:30 +0800 Subject: [PATCH 6/6] perf table reuse --- apisix/http/route.lua | 6 +++--- apisix/http/router/radixtree_host_uri.lua | 7 +++---- apisix/init.lua | 4 ++++ 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/apisix/http/route.lua b/apisix/http/route.lua index 1bb99052fae9..dbf11abf5e28 100644 --- a/apisix/http/route.lua +++ b/apisix/http/route.lua @@ -84,7 +84,7 @@ function _M.create_radixtree_uri_router(routes, uri_routes, with_parameter) handler = function (api_ctx, match_opts) api_ctx.matched_params = nil api_ctx.matched_route = route - api_ctx.curr_req_matched = core.table.deepcopy(match_opts.matched) + api_ctx.curr_req_matched = match_opts.matched end }) @@ -104,12 +104,12 @@ end function _M.match_uri(uri_router, api_ctx) - local match_opts = core.tablepool.fetch("route_match_opts", 0, 16) + local match_opts = core.tablepool.fetch("route_match_opts", 0, 4) match_opts.method = api_ctx.var.request_method match_opts.host = api_ctx.var.host match_opts.remote_addr = api_ctx.var.remote_addr match_opts.vars = api_ctx.var - match_opts.matched = {} + match_opts.matched = core.tablepool.fetch("matched_route_record", 0, 4) local ok = uri_router:dispatch(api_ctx.var.uri, match_opts, api_ctx, match_opts) core.tablepool.release("route_match_opts", match_opts) diff --git a/apisix/http/router/radixtree_host_uri.lua b/apisix/http/router/radixtree_host_uri.lua index b812dd62970e..680a04fbe815 100644 --- a/apisix/http/router/radixtree_host_uri.lua +++ b/apisix/http/router/radixtree_host_uri.lua @@ -78,11 +78,10 @@ local function push_host_router(route, host_routes, only_uri_routes) vars = route.value.vars, filter_fun = filter_fun, handler = function (api_ctx, match_opts) - local matched = core.table.deepcopy(match_opts.matched) api_ctx.matched_params = nil api_ctx.matched_route = route - api_ctx.curr_req_matched = matched - api_ctx.real_curr_req_matched_path = matched._path + api_ctx.curr_req_matched = match_opts.matched + api_ctx.real_curr_req_matched_path = match_opts.matched._path end } @@ -166,7 +165,7 @@ function _M.matching(api_ctx) match_opts.remote_addr = api_ctx.var.remote_addr match_opts.vars = api_ctx.var match_opts.host = api_ctx.var.host - match_opts.matched = {} + match_opts.matched = core.tablepool.fetch("matched_route_record", 0, 4) if host_router then local host_uri = api_ctx.var.host diff --git a/apisix/init.lua b/apisix/init.lua index 17cce08a5cad..86b68cf62208 100644 --- a/apisix/init.lua +++ b/apisix/init.lua @@ -883,6 +883,10 @@ function _M.http_log_phase() core.tablepool.release("plugins", api_ctx.plugins) end + if api_ctx.curr_req_matched then + core.tablepool.release("matched_route_record", api_ctx.curr_req_matched) + end + core.tablepool.release("api_ctx", api_ctx) end