diff --git a/plugins/cachekey/pattern.cc b/plugins/cachekey/pattern.cc index 515cd8f4f0b..45743815e27 100644 --- a/plugins/cachekey/pattern.cc +++ b/plugins/cachekey/pattern.cc @@ -152,7 +152,7 @@ Pattern::process(const String &subject, StringVector &result) } else { /* Replacement was not provided so return all capturing groups except the group zero. */ StringVector captures; - if (capture(subject, captures)) { + if (capture(subject, captures) && !captures.empty()) { if (captures.size() == 1) { result.push_back(captures[0]); } else { @@ -210,7 +210,7 @@ Pattern::capture(const String &subject, StringVector &result) return false; } - RegexMatches matches; + RegexMatches matches(_captureCount + 1); int matchCount = _re.exec(subject, matches, RE_NOTEMPTY); if (matchCount < 0) { if (matchCount != RE_ERROR_NOMATCH) { @@ -219,7 +219,7 @@ Pattern::capture(const String &subject, StringVector &result) return false; } - for (int i = 0; i < matchCount; i++) { + for (int i = 0; i < matches.size(); i++) { std::string_view capture = matches[i]; String dst(capture.data(), capture.length()); @@ -246,7 +246,7 @@ Pattern::replace(const String &subject, String &result) return false; } - RegexMatches matches; + RegexMatches matches(_captureCount + 1); int matchCount = _re.exec(subject, matches, RE_NOTEMPTY); if (matchCount < 0) { if (matchCount != RE_ERROR_NOMATCH) { @@ -255,18 +255,11 @@ Pattern::replace(const String &subject, String &result) return false; } - /* Verify the replacement has the right number of matching groups */ - for (int i = 0; i < _tokenCount; i++) { - if (_tokens[i] >= matchCount) { - CacheKeyError("invalid reference in replacement string: $%d", _tokens[i]); - return false; - } - } - int previous = 0; for (int i = 0; i < _tokenCount; i++) { - int replIndex = _tokens[i]; - std::string_view capture = matches[replIndex]; + int replIndex = _tokens[i]; + // Trailing optional groups may not participate in this match. + std::string_view capture = (replIndex < matches.size()) ? matches[replIndex] : std::string_view{""}; String src(_replacement, _tokenOffset[i], 2); String dst(capture.data(), capture.length()); @@ -304,6 +297,12 @@ Pattern::compile() return false; } + _captureCount = _re.get_capture_count(); + if (_captureCount < 0) { + CacheKeyError("failed to get capture count for pattern '%s'", _pattern.c_str()); + return false; + } + if (!_replace) { /* No replacement necessary - we are done. */ return true; @@ -336,6 +335,16 @@ Pattern::compile() } } + if (success) { + for (int i = 0; i < _tokenCount; i++) { + if (_tokens[i] > _captureCount) { + CacheKeyError("invalid reference $%d in replacement '%s': pattern defines only %d group(s)", _tokens[i], + _replacement.c_str(), _captureCount); + return false; + } + } + } + return success; } diff --git a/plugins/cachekey/pattern.h b/plugins/cachekey/pattern.h index e3f441d27ab..1392849b371 100644 --- a/plugins/cachekey/pattern.h +++ b/plugins/cachekey/pattern.h @@ -50,7 +50,8 @@ class Pattern private: bool compile(); - Regex _re; /**< @brief Regex compiled object */ + Regex _re; /**< @brief Regex compiled object */ + int32_t _captureCount = 0; ///< Number of capture groups defined by the compiled pattern. String _pattern; /**< @brief Regex pattern string, containing regex patterns and capturing groups. */ String diff --git a/plugins/cachekey/unit_tests/pattern_test.cc b/plugins/cachekey/unit_tests/pattern_test.cc index 2b9adb90d42..d0637e0d35d 100644 --- a/plugins/cachekey/unit_tests/pattern_test.cc +++ b/plugins/cachekey/unit_tests/pattern_test.cc @@ -202,13 +202,47 @@ TEST_CASE("Pattern compile and match behavior", "[cachekey][pattern]") CHECK(res == "num=123;"); } - SECTION("Replacement with invalid group reference") + SECTION("Replacement with invalid group reference fails at initialization") { Pattern p; - REQUIRE(p.init("(\\w+)", "$5", true)); // only 2 groups (0 and 1) + + CHECK_FALSE(p.init("(\\w+)", "$5", true)); + CHECK_FALSE(p.init("(a)(b)?", "$3", true)); + CHECK_FALSE(p.init("literal", "$1", true)); + } + + SECTION("Replacement with optional capture groups") + { + Pattern p; + + REQUIRE(p.init("^(a)(b)?(c)?$", "$1-$2-$3", true)); + for (const auto &[subject, expected] : { + std::pair{"a", "a--" }, + {"ab", "a-b-" }, + {"ac", "a--c" }, + {"abc", "a-b-c"} + }) { + String res; + + REQUIRE(p.replace(subject, res)); + CHECK(res == expected); + } + } + + SECTION("Capture and replacement beyond the inline match buffer") + { + Pattern p; + StringVector result; + + REQUIRE(p.init("(a)(b)(c)(d)(e)(f)(g)(h)(i)(j)(k)(l)")); + REQUIRE(p.process("abcdefghijkl", result)); + CHECK(result == StringVector{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l"}); + + REQUIRE(p.init("(a)(b)(c)(d)(e)(f)(g)(h)(i)(j)(k)(l)", "$9$1", true)); String res; - // Should fail because $5 doesn't exist - CHECK(p.replace("test", res) == false); + + REQUIRE(p.replace("abcdefghijkl", res)); + CHECK(res == "ia"); } SECTION("process() method - capture mode (no replacement)") diff --git a/tests/gold_tests/pluginTest/cachekey/cachekey_capture.test.py b/tests/gold_tests/pluginTest/cachekey/cachekey_capture.test.py new file mode 100644 index 00000000000..72451b04403 --- /dev/null +++ b/tests/gold_tests/pluginTest/cachekey/cachekey_capture.test.py @@ -0,0 +1,19 @@ +# 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. + +Test.Summary = 'Verify cachekey patterns preserve all capture groups.' +Test.SkipUnless(Condition.PluginExists('cachekey.so'), Condition.PluginExists('xdebug.so')) +Test.ATSReplayTest(replay_file='capture.replay.yaml') diff --git a/tests/gold_tests/pluginTest/cachekey/capture.replay.yaml b/tests/gold_tests/pluginTest/cachekey/capture.replay.yaml new file mode 100644 index 00000000000..3c5f668f231 --- /dev/null +++ b/tests/gold_tests/pluginTest/cachekey/capture.replay.yaml @@ -0,0 +1,294 @@ +# 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. + +meta: + version: '1.0' + +autest: + description: 'Cachekey captures beyond the default regex match capacity' + server: + name: server + client: + name: client + ats: + name: ts + records_config: + proxy.config.diags.debug.enabled: 1 + proxy.config.diags.debug.tags: cachekey + plugin_config: + - 'xdebug.so --enable=x-cache-key' + remap_config: + - from: 'http://nine.example.com/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + plugins: + - name: cachekey.so + args: + - '--static-prefix=capture' + - '--capture-path=(a)(b)(c)(d)(e)(f)(g)(h)(i)' + - from: 'http://ten.example.com/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + plugins: + - name: cachekey.so + args: + - '--static-prefix=capture' + - '--capture-path=(a)(b)(c)(d)(e)(f)(g)(h)(i)(j)' + - from: 'http://twelve.example.com/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + plugins: + - name: cachekey.so + args: + - '--static-prefix=capture' + - '--capture-path=(a)(b)(c)(d)(e)(f)(g)(h)(i)(j)(k)(l)' + - from: 'http://replace.example.com/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + plugins: + - name: cachekey.so + args: + - '--static-prefix=capture' + - '--capture-path=/(a)(b)(c)(d)(e)(f)(g)(h)(i)(j)(k)(l)/$9$1/' + - from: 'http://whole.example.com/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + plugins: + - name: cachekey.so + args: + - '--static-prefix=capture' + - '--capture-path=abcdef' + - from: 'http://no-match.example.com/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + plugins: + - name: cachekey.so + args: + - '--static-prefix=capture' + - '--capture-path=(z)' + - from: 'http://optional.example.com/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + plugins: + - name: cachekey.so + args: + - '--static-prefix=capture' + - '--capture-path=/(a)(b)?(c)?/$1-$2-$3/' + +sessions: +- transactions: + - client-request: + method: GET + url: /abcdefghi + version: '1.1' + headers: + fields: + - [Host, nine.example.com] + - [uuid, nine] + - [X-Debug, x-cache-key] + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + proxy-response: + status: 200 + headers: + fields: + - [X-Cache-Key, {value: '/capture/a/b/c/d/e/f/g/h/i', as: equal}] + + - client-request: + method: GET + url: /abcdefghij + version: '1.1' + headers: + fields: + - [Host, ten.example.com] + - [uuid, ten] + - [X-Debug, x-cache-key] + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + proxy-response: + status: 200 + headers: + fields: + - [X-Cache-Key, {value: '/capture/a/b/c/d/e/f/g/h/i/j', as: equal}] + + - client-request: + method: GET + url: /abcdefghijkl + version: '1.1' + headers: + fields: + - [Host, twelve.example.com] + - [uuid, twelve] + - [X-Debug, x-cache-key] + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + proxy-response: + status: 200 + headers: + fields: + - [X-Cache-Key, {value: '/capture/a/b/c/d/e/f/g/h/i/j/k/l', as: equal}] + + - client-request: + method: GET + url: /abcdefghijkl + version: '1.1' + headers: + fields: + - [Host, replace.example.com] + - [uuid, replace] + - [X-Debug, x-cache-key] + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + proxy-response: + status: 200 + headers: + fields: + - [X-Cache-Key, {value: '/capture/ia', as: equal}] + + - client-request: + method: GET + url: /abcdef + version: '1.1' + headers: + fields: + - [Host, whole.example.com] + - [uuid, whole] + - [X-Debug, x-cache-key] + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + proxy-response: + status: 200 + headers: + fields: + - [X-Cache-Key, {value: '/capture/abcdef', as: equal}] + + - client-request: + method: GET + url: /abcdef + version: '1.1' + headers: + fields: + - [Host, no-match.example.com] + - [uuid, no-match] + - [X-Debug, x-cache-key] + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + proxy-response: + status: 200 + headers: + fields: + - [X-Cache-Key, {value: '/capture', as: equal}] + + - client-request: + method: GET + url: /a + version: '1.1' + headers: + fields: + - [Host, optional.example.com] + - [uuid, optional-a] + - [X-Debug, x-cache-key] + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + proxy-response: + status: 200 + headers: + fields: + - [X-Cache-Key, {value: '/capture/a--', as: equal}] + + - client-request: + method: GET + url: /ab + version: '1.1' + headers: + fields: + - [Host, optional.example.com] + - [uuid, optional-ab] + - [X-Debug, x-cache-key] + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + proxy-response: + status: 200 + headers: + fields: + - [X-Cache-Key, {value: '/capture/a-b-', as: equal}] + + - client-request: + method: GET + url: /ac + version: '1.1' + headers: + fields: + - [Host, optional.example.com] + - [uuid, optional-ac] + - [X-Debug, x-cache-key] + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + proxy-response: + status: 200 + headers: + fields: + - [X-Cache-Key, {value: '/capture/a--c', as: equal}] + + - client-request: + method: GET + url: /abc + version: '1.1' + headers: + fields: + - [Host, optional.example.com] + - [uuid, optional-abc] + - [X-Debug, x-cache-key] + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + proxy-response: + status: 200 + headers: + fields: + - [X-Cache-Key, {value: '/capture/a-b-c', as: equal}]