From 9f043a8343eec5fbecf26a5da8047cb690dd95f4 Mon Sep 17 00:00:00 2001 From: Jarkko Hietaniemi Date: Wed, 5 Apr 2023 19:24:05 +0300 Subject: [PATCH 1/2] Add doublequotes around the URL of X-Effective-URL. URLs can have commas which makes the URLs fragile when multiple URLs are returned, and the values of multiple X-Effective-URL headers are merged. Follow-up on 170e12b4 (Adds new X-Effective-URL header to the xdebug plugin (#7931)) --- plugins/xdebug/xdebug.cc | 6 +++++- .../pluginTest/xdebug/x_effective_url/four.in | 5 +++++ .../xdebug/x_effective_url/out.gold | 19 +++++++++++++++---- .../x_effective_url/x_effective_url.test.py | 12 +++++++++++- 4 files changed, 36 insertions(+), 6 deletions(-) create mode 100644 tests/gold_tests/pluginTest/xdebug/x_effective_url/four.in diff --git a/plugins/xdebug/xdebug.cc b/plugins/xdebug/xdebug.cc index 6bfd35d5045..71384181d3e 100644 --- a/plugins/xdebug/xdebug.cc +++ b/plugins/xdebug/xdebug.cc @@ -433,7 +433,11 @@ InjectEffectiveURLHeader(TSHttpTxn txn, TSMBuffer buffer, TSMLoc hdr) if (strval.ptr != nullptr && strval.len > 0) { TSMLoc dst = FindOrMakeHdrField(buffer, hdr, "X-Effective-URL", lengthof("X-Effective-URL")); if (dst != TS_NULL_MLOC) { - TSReleaseAssert(TSMimeHdrFieldValueStringInsert(buffer, hdr, dst, -1 /* idx */, strval.ptr, strval.len) == TS_SUCCESS); + char buf[16 * 1024]; + int len = snprintf(buf, sizeof(buf), "\"%s\"", strval.ptr); + if (len == strval.len + 2 && len <= static_cast(sizeof(buf)) - 1) { // Only copy back if len expected and within buffer. + TSReleaseAssert(TSMimeHdrFieldValueStringInsert(buffer, hdr, dst, -1 /* idx */, buf, len) == TS_SUCCESS); + } TSHandleMLocRelease(buffer, hdr, dst); } } diff --git a/tests/gold_tests/pluginTest/xdebug/x_effective_url/four.in b/tests/gold_tests/pluginTest/xdebug/x_effective_url/four.in new file mode 100644 index 00000000000..cc043854a22 --- /dev/null +++ b/tests/gold_tests/pluginTest/xdebug/x_effective_url/four.in @@ -0,0 +1,5 @@ +GET /argh,urgh HTTP/1.1 +Host: four +X-Debug: X-effective-url +Connection: close + diff --git a/tests/gold_tests/pluginTest/xdebug/x_effective_url/out.gold b/tests/gold_tests/pluginTest/xdebug/x_effective_url/out.gold index 7af772ad449..19346ca02c3 100644 --- a/tests/gold_tests/pluginTest/xdebug/x_effective_url/out.gold +++ b/tests/gold_tests/pluginTest/xdebug/x_effective_url/out.gold @@ -5,7 +5,7 @@ Server: ATS/`` Cache-Control: no-store Content-Type: text/html Content-Language: en -X-Effective-URL: http://none/argh +X-Effective-URL: "http://none/argh" Content-Length: 391 @@ -32,7 +32,7 @@ Age: `` Transfer-Encoding: chunked Connection: close Server: ATS/`` -X-Effective-URL: http://127.0.0.1:SERVER_PORT/argh +X-Effective-URL: "http://127.0.0.1:SERVER_PORT/argh" 0 @@ -43,7 +43,7 @@ Age: `` Transfer-Encoding: chunked Connection: close Server: ATS/`` -X-Effective-URL: http://127.0.0.1:SERVER_PORT/two/argh +X-Effective-URL: "http://127.0.0.1:SERVER_PORT/two/argh" 0 @@ -54,7 +54,18 @@ Age: `` Transfer-Encoding: chunked Connection: close Server: ATS/`` -X-Effective-URL: http://127.0.0.1:SERVER_PORT/argh +X-Effective-URL: "http://127.0.0.1:SERVER_PORT/argh" + +0 + +====== +HTTP/1.1 200 OK +Date: `` +Age: `` +Transfer-Encoding: chunked +Connection: close +Server: ATS/`` +X-Effective-URL: "http://127.0.0.1:SERVER_PORT/four/argh,urgh" 0 diff --git a/tests/gold_tests/pluginTest/xdebug/x_effective_url/x_effective_url.test.py b/tests/gold_tests/pluginTest/xdebug/x_effective_url/x_effective_url.test.py index 32bd3446f5e..0f01c662a5e 100644 --- a/tests/gold_tests/pluginTest/xdebug/x_effective_url/x_effective_url.test.py +++ b/tests/gold_tests/pluginTest/xdebug/x_effective_url/x_effective_url.test.py @@ -32,6 +32,12 @@ "headers": "GET /two/argh HTTP/1.1\r\nHost: doesnotmatter\r\n\r\n", "timestamp": "1469733493.993", "body": ""} server.addResponse("sessionlog.json", request_header_two, response_header) +# request_header_three would be identical to request_header. + +request_header_four = { + "headers": "GET /four/argh,urgh HTTP/1.1\r\nHost: doesnotmatter\r\n\r\n", "timestamp": "1469733493.993", "body": ""} +server.addResponse("sessionlog.json", request_header_four, response_header) + ts = Test.MakeATSProcess("ts") ts.Disk.records_config.update({ @@ -48,7 +54,10 @@ "map http://two http://127.0.0.1:{0}".format(server.Variables.Port) + "/two" ) ts.Disk.remap_config.AddLine( - "regex_map http://three[0-9]+ http://127.0.0.1:{0}".format(server.Variables.Port) + "regex_map http://three[0-9]+ http://127.0.0.1:{0}".format(server.Variables.Port) # Host: three123 +) +ts.Disk.remap_config.AddLine( + "regex_map http://four http://127.0.0.1:{0}".format(server.Variables.Port) + "/four" ) tr = Test.AddTestRun() @@ -73,6 +82,7 @@ def sendMsg(msgFile): sendMsg('one') sendMsg('two') sendMsg('three') +sendMsg('four') tr = Test.AddTestRun() tr.Processes.Default.Command = "echo test out.gold" From 4bc2179cbe2723315a0db30a5e1592fabc07cb43 Mon Sep 17 00:00:00 2001 From: Jarkko Hietaniemi Date: Mon, 17 Jul 2023 09:45:05 +0300 Subject: [PATCH 2/2] Document the X-Effective-URL. --- doc/admin-guide/plugins/xdebug.en.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/admin-guide/plugins/xdebug.en.rst b/doc/admin-guide/plugins/xdebug.en.rst index cce40d76681..69632fc3cf6 100644 --- a/doc/admin-guide/plugins/xdebug.en.rst +++ b/doc/admin-guide/plugins/xdebug.en.rst @@ -127,6 +127,10 @@ X-Remap If the URL was remapped for a request, this header gives the *to* and *from* field from the line in remap.config that caused the URL to be remapped. +X-Effective-URL + If the URL was remapped for a request, this header gives the URL resulting from the remapping. Note that if there are + multiple remaps, this header aggregates the URLs, space-comma-separated. The URLs are inside doublequotes. + X-ParentSelection-Key The ``X-ParentSelection-Key`` header contains the URL that is used to determine parent selection for an object in the Traffic Server. This