From 8aee1215fbb869d015145f514133d318dc632d4e Mon Sep 17 00:00:00 2001 From: Dhi Aurrahman Date: Mon, 30 Apr 2018 23:12:15 +0700 Subject: [PATCH] docs: clarify the use of "/" as a prefix_rewrite (#3160) docs: clarify the use of "/" as a prefix_rewrite This patch clarifies the use of "/" as a prefix_rewrite in route and redirect prefix rewriting. And also a note on the use of trailing slashes as match value. Risk Level: Low Testing: add more input samples to RedirectPrefixRewrite test. Docs Changes: Update route.proto doc regarding path_rewrite both for redirect and route. Release Notes: N/A Fixes #2956 Signed-off-by: Dhi Aurrahman Signed-off-by: Rama --- api/envoy/api/v2/route/route.proto | 27 +++++++++++ test/common/router/config_impl_test.cc | 64 ++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/api/envoy/api/v2/route/route.proto b/api/envoy/api/v2/route/route.proto index e2e85e851f9a..7993ba660c2f 100644 --- a/api/envoy/api/v2/route/route.proto +++ b/api/envoy/api/v2/route/route.proto @@ -353,6 +353,28 @@ message RouteAction { // Indicates that during forwarding, the matched prefix (or path) should be // swapped with this value. This option allows application URLs to be rooted // at a different path from those exposed at the reverse proxy layer. + // + // .. attention:: + // + // Pay careful attention to the use of trailing slashes in the + // :ref:`route's match ` prefix value. + // Stripping a prefix from a path requires multiple Routes to handle all cases. For example, + // rewriting */prefix* to */* and */prefix/etc* to */etc* cannot be done in a single + // :ref:`Route `, as shown by the below config entries: + // + // .. code-block:: yaml + // + // - match: + // prefix: "/prefix/" + // route: + // prefix_rewrite: "/" + // - match: + // prefix: "/prefix" + // route: + // prefix_rewrite: "/" + // + // Having above entries in the config, requests to */prefix* will be stripped to */*, while + // requests to */prefix/etc* will be stripped to */etc*. string prefix_rewrite = 5; oneof host_rewrite_specifier { @@ -566,6 +588,11 @@ message RedirectAction { // Indicates that during redirection, the matched prefix (or path) // should be swapped with this value. This option allows redirect URLs be dynamically created // based on the request. + // + // .. attention:: + // + // Pay attention to the use of trailing slashes as mentioned in + // :ref:`RouteAction's prefix_rewrite `. string prefix_rewrite = 5; } diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 20d122afd426..79b625b3addf 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -3925,6 +3925,14 @@ name: AllRedirects redirect: { prefix_rewrite: "/new/regex-prefix/" } - match: { prefix: "/http/prefix"} redirect: { prefix_rewrite: "/https/prefix" , https_redirect: true } + - match: { prefix: "/ignore-this/"} + redirect: { prefix_rewrite: "/" } + - match: { prefix: "/ignore-this"} + redirect: { prefix_rewrite: "/" } + - match: { prefix: "/ignore-substring"} + redirect: { prefix_rewrite: "/" } + - match: { prefix: "/service-hello/"} + redirect: { prefix_rewrite: "/" } )EOF"; NiceMock runtime; @@ -3969,6 +3977,62 @@ name: AllRedirects redirect->rewritePathHeader(headers); EXPECT_EQ("https://redirect.lyft.com/https/prefix/", redirect->newPath(headers)); } + { + // The following matches to the redirect action match value equals to `/ignore-this` instead of + // `/ignore-this/`. + Http::TestHeaderMapImpl headers = + genRedirectHeaders("redirect.lyft.com", "/ignore-this", false, false); + const DirectResponseEntry* redirect = config.route(headers, 0)->directResponseEntry(); + redirect->rewritePathHeader(headers); + EXPECT_EQ("http://redirect.lyft.com/", redirect->newPath(headers)); + } + { + // The following matches to the redirect action match value equals to `/ignore-this/` instead of + // `/ignore-this`. + Http::TestHeaderMapImpl headers = + genRedirectHeaders("redirect.lyft.com", "/ignore-this/", false, false); + const DirectResponseEntry* redirect = config.route(headers, 0)->directResponseEntry(); + redirect->rewritePathHeader(headers); + EXPECT_EQ("http://redirect.lyft.com/", redirect->newPath(headers)); + } + { + // The same as previous test request, the following matches to the redirect action match value + // equals to `/ignore-this/` instead of `/ignore-this`. + Http::TestHeaderMapImpl headers = genRedirectHeaders( + "redirect.lyft.com", "/ignore-this/however/use/the/rest/of/this/path", false, false); + const DirectResponseEntry* redirect = config.route(headers, 0)->directResponseEntry(); + redirect->rewritePathHeader(headers); + EXPECT_EQ("http://redirect.lyft.com/however/use/the/rest/of/this/path", + redirect->newPath(headers)); + } + { + Http::TestHeaderMapImpl headers = + genRedirectHeaders("redirect.lyft.com", "/ignore-this/use/", false, false); + const DirectResponseEntry* redirect = config.route(headers, 0)->directResponseEntry(); + redirect->rewritePathHeader(headers); + EXPECT_EQ("http://redirect.lyft.com/use/", redirect->newPath(headers)); + } + { + Http::TestHeaderMapImpl headers = + genRedirectHeaders("redirect.lyft.com", "/ignore-substringto/use/", false, false); + const DirectResponseEntry* redirect = config.route(headers, 0)->directResponseEntry(); + redirect->rewritePathHeader(headers); + EXPECT_EQ("http://redirect.lyft.com/to/use/", redirect->newPath(headers)); + } + { + Http::TestHeaderMapImpl headers = + genRedirectHeaders("redirect.lyft.com", "/ignore-substring-to/use/", false, false); + const DirectResponseEntry* redirect = config.route(headers, 0)->directResponseEntry(); + redirect->rewritePathHeader(headers); + EXPECT_EQ("http://redirect.lyft.com/-to/use/", redirect->newPath(headers)); + } + { + Http::TestHeaderMapImpl headers = + genRedirectHeaders("redirect.lyft.com", "/service-hello/a/b/c", false, false); + const DirectResponseEntry* redirect = config.route(headers, 0)->directResponseEntry(); + redirect->rewritePathHeader(headers); + EXPECT_EQ("http://redirect.lyft.com/a/b/c", redirect->newPath(headers)); + } } // Test to check Strip Query for redirect messages