Skip to content

Commit a3a8138

Browse files
YapWCYapWC
authored
fix(proxy-rewrite): query string discarded when use_real_request_uri_… (#12843)
1 parent 7f67c20 commit a3a8138

3 files changed

Lines changed: 346 additions & 2 deletions

File tree

‎apisix/plugins/proxy-rewrite.lua‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,13 +284,28 @@ function _M.rewrite(conf, ctx)
284284

285285
local upstream_uri = ctx.var.uri
286286
local separator_escaped = false
287+
288+
-- resolve_var() below drops the query string, so keep the original one from
289+
-- real_request_uri to re-append it when conf.uri is set.
290+
local query_string = ""
287291
if conf.use_real_request_uri_unsafe then
288292
upstream_uri = ctx.var.real_request_uri
293+
local index = str_find(upstream_uri, "?")
294+
query_string = index and sub_str(upstream_uri, index) or ""
289295
end
290296

291297
if conf.uri ~= nil then
292298
separator_escaped = true
293299
upstream_uri = core.utils.resolve_var(conf.uri, ctx.var, escape_separator)
300+
if query_string ~= "" then
301+
-- merge with '&' when conf.uri already carries its own query string,
302+
-- otherwise append the original query string as-is
303+
if str_find(upstream_uri, "?") then
304+
upstream_uri = upstream_uri .. "&" .. sub_str(query_string, 2)
305+
else
306+
upstream_uri = upstream_uri .. query_string
307+
end
308+
end
294309

295310
elseif conf.regex_uri ~= nil then
296311
if not str_find(upstream_uri, "?") then

‎t/plugin/proxy-rewrite.t‎

Lines changed: 285 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,3 +1231,288 @@ GET /hello
12311231
uri: /uri
12321232
host: test.com:6443
12331233
x-real-ip: 127.0.0.1
1234+
1235+
1236+
1237+
=== TEST 45: set route(rewrite uri args with unsafe allowed)
1238+
--- config
1239+
location /t {
1240+
content_by_lua_block {
1241+
local t = require("lib.test_admin").test
1242+
local code, body = t('/apisix/admin/routes/1',
1243+
ngx.HTTP_PUT,
1244+
[[{
1245+
"plugins": {
1246+
"proxy-rewrite": {
1247+
"uri": "/plugin_proxy_rewrite_args",
1248+
"use_real_request_uri_unsafe": true
1249+
}
1250+
},
1251+
"upstream": {
1252+
"nodes": {
1253+
"127.0.0.1:1980": 1
1254+
},
1255+
"type": "roundrobin"
1256+
},
1257+
"uri": "/hello"
1258+
}]]
1259+
)
1260+
1261+
if code >= 300 then
1262+
ngx.status = code
1263+
end
1264+
ngx.say(body)
1265+
}
1266+
}
1267+
--- request
1268+
GET /t
1269+
--- response_body
1270+
passed
1271+
1272+
1273+
1274+
=== TEST 46: rewrite uri args
1275+
--- request
1276+
GET /hello?q=apisix&a=iresty HTTP/1.1
1277+
--- response_body
1278+
uri: /plugin_proxy_rewrite_args
1279+
a: iresty
1280+
q: apisix
1281+
1282+
1283+
1284+
=== TEST 47: set route(rewrite uri empty args with unsafe allowed)
1285+
--- config
1286+
location /t {
1287+
content_by_lua_block {
1288+
local t = require("lib.test_admin").test
1289+
local code, body = t('/apisix/admin/routes/1',
1290+
ngx.HTTP_PUT,
1291+
[[{
1292+
"plugins": {
1293+
"proxy-rewrite": {
1294+
"uri": "/plugin_proxy_rewrite_args",
1295+
"use_real_request_uri_unsafe": true
1296+
}
1297+
},
1298+
"upstream": {
1299+
"nodes": {
1300+
"127.0.0.1:1980": 1
1301+
},
1302+
"type": "roundrobin"
1303+
},
1304+
"uri": "/hello"
1305+
}]]
1306+
)
1307+
1308+
if code >= 300 then
1309+
ngx.status = code
1310+
end
1311+
ngx.say(body)
1312+
}
1313+
}
1314+
--- request
1315+
GET /t
1316+
--- response_body
1317+
passed
1318+
1319+
1320+
1321+
=== TEST 48: rewrite uri empty args
1322+
--- request
1323+
GET /hello HTTP/1.1
1324+
--- response_body
1325+
uri: /plugin_proxy_rewrite_args
1326+
1327+
1328+
1329+
=== TEST 49: set route(conf.uri carries its own query, use_real_request_uri_unsafe)
1330+
--- config
1331+
location /t {
1332+
content_by_lua_block {
1333+
local t = require("lib.test_admin").test
1334+
local code, body = t('/apisix/admin/routes/1',
1335+
ngx.HTTP_PUT,
1336+
[[{
1337+
"plugins": {
1338+
"proxy-rewrite": {
1339+
"uri": "/plugin_proxy_rewrite_args?x=1",
1340+
"use_real_request_uri_unsafe": true
1341+
}
1342+
},
1343+
"upstream": {
1344+
"nodes": {
1345+
"127.0.0.1:1980": 1
1346+
},
1347+
"type": "roundrobin"
1348+
},
1349+
"uri": "/hello"
1350+
}]]
1351+
)
1352+
1353+
if code >= 300 then
1354+
ngx.status = code
1355+
end
1356+
ngx.say(body)
1357+
}
1358+
}
1359+
--- request
1360+
GET /t
1361+
--- response_body
1362+
passed
1363+
1364+
1365+
1366+
=== TEST 50: merge conf.uri query with incoming query (use '&', not double '?')
1367+
--- request
1368+
GET /hello?q=apisix HTTP/1.1
1369+
--- response_body
1370+
uri: /plugin_proxy_rewrite_args
1371+
q: apisix
1372+
x: 1
1373+
1374+
1375+
1376+
=== TEST 51: set route(use_real_request_uri_unsafe only, no conf.uri)
1377+
--- config
1378+
location /t {
1379+
content_by_lua_block {
1380+
local t = require("lib.test_admin").test
1381+
local code, body = t('/apisix/admin/routes/1',
1382+
ngx.HTTP_PUT,
1383+
[[{
1384+
"plugins": {
1385+
"proxy-rewrite": {
1386+
"use_real_request_uri_unsafe": true
1387+
}
1388+
},
1389+
"upstream": {
1390+
"nodes": {
1391+
"127.0.0.1:1980": 1
1392+
},
1393+
"type": "roundrobin"
1394+
},
1395+
"uri": "/plugin_proxy_rewrite_args"
1396+
}]]
1397+
)
1398+
1399+
if code >= 300 then
1400+
ngx.status = code
1401+
end
1402+
ngx.say(body)
1403+
}
1404+
}
1405+
--- request
1406+
GET /t
1407+
--- response_body
1408+
passed
1409+
1410+
1411+
1412+
=== TEST 52: query string reaches upstream exactly once without conf.uri
1413+
--- request
1414+
GET /plugin_proxy_rewrite_args?q=apisix HTTP/1.1
1415+
--- response_body
1416+
uri: /plugin_proxy_rewrite_args
1417+
q: apisix
1418+
1419+
1420+
1421+
=== TEST 53: set route(conf.uri echoing raw request_uri, use_real_request_uri_unsafe)
1422+
--- config
1423+
location /t {
1424+
content_by_lua_block {
1425+
local t = require("lib.test_admin").test
1426+
local code, body = t('/apisix/admin/routes/1',
1427+
ngx.HTTP_PUT,
1428+
[[{
1429+
"plugins": {
1430+
"proxy-rewrite": {
1431+
"uri": "/print_uri_detailed",
1432+
"use_real_request_uri_unsafe": true
1433+
}
1434+
},
1435+
"upstream": {
1436+
"nodes": {
1437+
"127.0.0.1:1980": 1
1438+
},
1439+
"type": "roundrobin"
1440+
},
1441+
"uri": "/hello"
1442+
}]]
1443+
)
1444+
1445+
if code >= 300 then
1446+
ngx.status = code
1447+
end
1448+
ngx.say(body)
1449+
}
1450+
}
1451+
--- request
1452+
GET /t
1453+
--- response_body
1454+
passed
1455+
1456+
1457+
1458+
=== TEST 54: query with special characters passed through unchanged (unsafe)
1459+
--- request
1460+
GET /hello?name=a%20b&x=%E4%B8%AD HTTP/1.1
1461+
--- response_body
1462+
ngx.var.uri: /print_uri_detailed
1463+
ngx.var.request_uri: /print_uri_detailed?name=a%20b&x=%E4%B8%AD
1464+
1465+
1466+
1467+
=== TEST 55: query with multiple '?' passed through unchanged (unsafe)
1468+
--- request
1469+
GET /hello?a=1?b=2 HTTP/1.1
1470+
--- response_body
1471+
ngx.var.uri: /print_uri_detailed
1472+
ngx.var.request_uri: /print_uri_detailed?a=1?b=2
1473+
1474+
1475+
1476+
=== TEST 56: set route(same conf.uri with use_real_request_uri_unsafe disabled)
1477+
--- config
1478+
location /t {
1479+
content_by_lua_block {
1480+
local t = require("lib.test_admin").test
1481+
local code, body = t('/apisix/admin/routes/1',
1482+
ngx.HTTP_PUT,
1483+
[[{
1484+
"plugins": {
1485+
"proxy-rewrite": {
1486+
"uri": "/print_uri_detailed",
1487+
"use_real_request_uri_unsafe": false
1488+
}
1489+
},
1490+
"upstream": {
1491+
"nodes": {
1492+
"127.0.0.1:1980": 1
1493+
},
1494+
"type": "roundrobin"
1495+
},
1496+
"uri": "/hello"
1497+
}]]
1498+
)
1499+
1500+
if code >= 300 then
1501+
ngx.status = code
1502+
end
1503+
ngx.say(body)
1504+
}
1505+
}
1506+
--- request
1507+
GET /t
1508+
--- response_body
1509+
passed
1510+
1511+
1512+
1513+
=== TEST 57: query preserved with use_real_request_uri_unsafe disabled (both variants consistent)
1514+
--- request
1515+
GET /hello?name=a%20b&x=1 HTTP/1.1
1516+
--- response_body
1517+
ngx.var.uri: /print_uri_detailed
1518+
ngx.var.request_uri: /print_uri_detailed?name=a%20b&x=1

‎t/plugin/proxy-rewrite3.t‎

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ plugin_proxy_rewrite get method: POST
199199
200200
201201
202-
=== TEST 8: set route(unsafe uri not normalized at request)
202+
=== TEST 8: set route(unsafe uri not normalized at request with unsafe allowed)
203203
--- config
204204
location /t {
205205
content_by_lua_block {
@@ -243,7 +243,7 @@ ngx.var.request_uri: /print%5Furi%5Fdetailed
243243
244244
245245
246-
=== TEST 10: set route(safe uri not normalized at request)
246+
=== TEST 10: set route(safe uri not normalized at request with unsafe allowed)
247247
--- config
248248
location /t {
249249
content_by_lua_block {
@@ -999,3 +999,47 @@ GET /hello/%ED%85%8C%EC%8A%A4%ED%8A%B8 HTTP/1.1
999999
}
10001000
--- response_body
10011001
/hello?unsafe_variable=%ED%85%8C%EC%8A%A4%ED%8A%B8
1002+
1003+
1004+
1005+
=== TEST 42: set route(unsafe uri normalized at request with unsafe not allowed)
1006+
--- config
1007+
location /t {
1008+
content_by_lua_block {
1009+
local t = require("lib.test_admin").test
1010+
local code, body = t('/apisix/admin/routes/1',
1011+
ngx.HTTP_PUT,
1012+
[[{
1013+
"methods": ["GET"],
1014+
"plugins": {
1015+
"proxy-rewrite": {
1016+
"use_real_request_uri_unsafe": false
1017+
}
1018+
},
1019+
"upstream": {
1020+
"nodes": {
1021+
"127.0.0.1:1980": 1
1022+
},
1023+
"type": "roundrobin"
1024+
},
1025+
"uri": "/print_uri_detailed"
1026+
}]]
1027+
)
1028+
1029+
if code >= 300 then
1030+
ngx.status = code
1031+
end
1032+
ngx.say(body)
1033+
}
1034+
}
1035+
--- response_body
1036+
passed
1037+
1038+
1039+
1040+
=== TEST 43: unsafe uri normalized at request
1041+
--- request
1042+
GET /print%5Furi%5Fdetailed HTTP/1.1
1043+
--- response_body
1044+
ngx.var.uri: /print_uri_detailed
1045+
ngx.var.request_uri: /print_uri_detailed

0 commit comments

Comments
 (0)