Fix: double slashes in redirect URL by zhaohuabing · Pull Request #2998 · envoyproxy/gateway

zhaohuabing · 2024-03-23T05:53:06Z

This PR fix double splashes in the redirect URL

When an HTTPRoute's match is a path prefix and has a redirect filter with a replacePrefixMatch "/",
A redirect routeaction with regex_rewrite is generated to match both the prefix with and without trailing "/" to avoid double "/" in the redirect URL.

For example, given the following HTTPRoute:

 - matches:
   - path:
       type: PathPrefix
       value: /api/foo/
   filters:
   - requestRedirect:
       path:
         replacePrefixMatch: /
         type: ReplacePrefixMatch
     type: RequestRedirect

The following route will be created:

              routes:
              - match:
                  path_separated_prefix: "/api/foo"
                redirect:
                  regex_rewrite:       
                    pattern:
                      regex: "^\/api\/foo\/*"
                    substitution: /

See #2976 for more details.

Fix: #2976

codecov · 2024-03-23T05:56:20Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.58%. Comparing base (31b4058) to head (b2998af).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2998      +/-   ##
==========================================
+ Coverage   64.56%   64.58%   +0.02%     
==========================================
  Files         121      121              
  Lines       21369    21382      +13     
==========================================
+ Hits        13796    13809      +13     
  Misses       6707     6707              
  Partials      866      866

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

arkodg · 2024-03-26T11:09:39Z

hey @zhaohuabing I found some code under URLRewrite,

gateway/internal/xds/translator/route.go

Line 332 in 485edbe

// Circumvent the case of "//" when the replace string is "/"

, can we reuse this ?

internal/xds/translator/route.go

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

arkodg · 2024-03-28T19:48:12Z

internal/xds/translator/route.go

+func useRegexRewriteForPrefixMatchReplace(pathMatch *ir.StringMatch, prefixMatchReplace string) bool {
+	return pathMatch != nil &&
+		pathMatch.Prefix != nil &&
+		*pathMatch.Prefix != "/" &&


this line is new ?

It's an edge case. We don't need to use prefixMatchReplace if the patchMatch prefix is "/". But regex still work anyway. I removed this line.

virtual_hosts: - name: service domains: - "*" routes: - match: prefix: "/" redirect: prefixRewrite: "/"

arkodg · 2024-03-28T19:49:47Z

looks good, thanks for reusing the existing code !
minor comment, else LGTM

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

arkodg

LGTM thanks for fixing this quirk !
and also adding e2es 🚀

* fix: double trailing splashs in redirect URL Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * add e2e tests Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * fix lint Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * add e2e tests Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * revert Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * use regex rewrite to generate the redirect url Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * use regex rewrite to generate the redirect url Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * use regex rewrite to generate the redirect url Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * remove comments Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * extract method Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * address comments Signed-off-by: huabing zhao <zhaohuabing@gmail.com> --------- Signed-off-by: huabing zhao <zhaohuabing@gmail.com> (cherry picked from commit ceb697f) Signed-off-by: Arko Dasgupta <arko@tetrate.io>

* Run certgen when upgrading (#2934) run certgen when upgrading Signed-off-by: huabing zhao <zhaohuabing@gmail.com> (cherry picked from commit 62ecf15) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * Fix: nil secret in resourceversiontable (#2982) * fix nil secret in resourceversiontable Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * check secrets in the xds result Signed-off-by: huabing zhao <zhaohuabing@gmail.com> --------- Signed-off-by: huabing zhao <zhaohuabing@gmail.com> (cherry picked from commit e880439) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix: add missing http filters to the http filter chain (#2970) * fix: add missing http filters to the http filter chain Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * refactor Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * fix lint Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * add comments Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * remove refactor Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * remove refactor Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * fix gen Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * fix lint Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> --------- Signed-off-by: huabing zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> (cherry picked from commit f699edf) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix: allow websockets in url rewrite (#3022) allow websockets in url rewrite Signed-off-by: Jesse Haka <haka.jesse@gmail.com> Co-authored-by: zirain <zirain2009@gmail.com> (cherry picked from commit 3d51933) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * Set host for http health checker explicitly to avoid using the cluster name as host header for http health checking request. (#3057) * Set host for http health checker explictly to avoid using the cluster name as host header for http health checking request Signed-off-by: lemonlinger <lemonlinger@gmail.com> * fix broken tests Signed-off-by: lemonlinger <lemonlinger@gmail.com> * fix health-check test case in xds translation Signed-off-by: lemonlinger <lemonlinger@gmail.com> * Simplify code and concise comments Signed-off-by: lemonlinger <lemonlinger@gmail.com> --------- Signed-off-by: lemonlinger <lemonlinger@gmail.com> (cherry picked from commit 8f450a9) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix: do not create infra resources when missing translated listeners (#3043) * fix: do not create infra resources when missing translated listeners Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> * remove empty line Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> * skip infra creation on empty listeners and log it Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> --------- Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> (cherry picked from commit 36d7141) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * Fix: double slashes in redirect URL (#2998) * fix: double trailing splashs in redirect URL Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * add e2e tests Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * fix lint Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * add e2e tests Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * fix test Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * revert Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * use regex rewrite to generate the redirect url Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * use regex rewrite to generate the redirect url Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * use regex rewrite to generate the redirect url Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * remove comments Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * extract method Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * address comments Signed-off-by: huabing zhao <zhaohuabing@gmail.com> --------- Signed-off-by: huabing zhao <zhaohuabing@gmail.com> (cherry picked from commit ceb697f) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix: Allow Policy to attach to multiple http listeners (#2967) * Fixing the clienttrafficpolicy validation. Signed-off-by: Lior Okman <lior.okman@sap.com> * Make SecurityPolicy validate correctly. Signed-off-by: Lior Okman <lior.okman@sap.com> * Reverted the SecurityPolicy validation - handled differently via another feature. Signed-off-by: Lior Okman <lior.okman@sap.com> * Updated the tests to reflect that this validation isn't required for SecurityPolicy Signed-off-by: Lior Okman <lior.okman@sap.com> * Added some comments to explain the validation being performed. Signed-off-by: Lior Okman <lior.okman@sap.com> * Updated the error message as requested in the review. Signed-off-by: Lior Okman <lior.okman@sap.com> --------- Signed-off-by: Lior Okman <lior.okman@sap.com> (cherry picked from commit f9409e4) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix: set path prefix for http ext auth service (#3018) Signed-off-by: huabing zhao <zhaohuabing@gmail.com> (cherry picked from commit 2882b7c) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * Change route sorting order to Exact > RegularExpression > PathPrefix (#2579) * Change route sorting order to Exact > RegularExpression > PathPrefix kubernetes-sigs/gateway-api#1770 kubernetes-sigs/gateway-api#1855 Signed-off-by: Stéphane Cottin <stephane.cottin@vixns.com> (cherry picked from commit 11f56fd) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix: infraIR duplicate port translation for merged gateways (#3061) * fix: duplicate port translation for merged gateways Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> * refactor to map Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> * rename map Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> * add seperate testcase Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> --------- Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> (cherry picked from commit 29946b0) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * translator: set SpawnUpstreamSpan to true (#3102) * translator: set SpawnUpstreamSpan to true Signed-off-by: zirain <zirain2009@gmail.com> * update Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: zirain <zirain2009@gmail.com> (cherry picked from commit 635ebfc) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * fix: rate limit doesn't work with two(and more) listeners (#3085) * fix: rate limit doesn't work with two listeners Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * add e2e test for rate limit on multiple listeners Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * address comments Signed-off-by: huabing zhao <zhaohuabing@gmail.com> --------- Signed-off-by: huabing zhao <zhaohuabing@gmail.com> Co-authored-by: Xunzhuo <bitliu@tencent.com> (cherry picked from commit a5bedbc) Signed-off-by: Arko Dasgupta <arko@tetrate.io> * rerun make testdata Signed-off-by: Arko Dasgupta <arko@tetrate.io> --------- Signed-off-by: huabing zhao <zhaohuabing@gmail.com> Signed-off-by: Arko Dasgupta <arko@tetrate.io> Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> Signed-off-by: Jesse Haka <haka.jesse@gmail.com> Signed-off-by: lemonlinger <lemonlinger@gmail.com> Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> Signed-off-by: Lior Okman <lior.okman@sap.com> Signed-off-by: Stéphane Cottin <stephane.cottin@vixns.com> Signed-off-by: zirain <zirain2009@gmail.com> Co-authored-by: Huabing Zhao <zhaohuabing@gmail.com> Co-authored-by: Jesse Haka <haka.jesse@gmail.com> Co-authored-by: zirain <zirain2009@gmail.com> Co-authored-by: Meng <lemonlinger@gmail.com> Co-authored-by: Karol Szwaj <karol.szwaj@gmail.com> Co-authored-by: Lior Okman <lior.okman@sap.com> Co-authored-by: vixns <stephane.cottin@vixns.com> Co-authored-by: Xunzhuo <bitliu@tencent.com>

zhaohuabing requested a review from a team as a code owner March 23, 2024 05:53

zhaohuabing force-pushed the fix-redirect-trailling-slash branch from 7799df0 to a11cd5e Compare March 23, 2024 06:02

fix: double trailing splashs in redirect URL

c0b619c

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

zhaohuabing force-pushed the fix-redirect-trailling-slash branch from a11cd5e to c0b619c Compare March 23, 2024 06:04

zhaohuabing changed the title ~~Fix: double trailing splashs in redirect URL~~ Fix: double splashes in redirect URL Mar 23, 2024

zhaohuabing added 5 commits March 23, 2024 14:17

add e2e tests

9329419

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

fix lint

3e89d1d

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

fix test

be5ad08

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

fix test

2c370fc

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

fix test

76cc1c9

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

zhaohuabing force-pushed the fix-redirect-trailling-slash branch from 982eecb to 76cc1c9 Compare March 23, 2024 12:24

zhaohuabing added 2 commits March 24, 2024 09:32

fix test

3cb5340

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

add e2e tests

008c43d

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

zhaohuabing requested a review from arkodg March 24, 2024 01:45

zhaohuabing added 4 commits March 24, 2024 10:47

fix test

498d231

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

Merge branch 'main' into fix-redirect-trailling-slash

ccd717d

Merge branch 'main' into fix-redirect-trailling-slash

ac7a633

Merge branch 'main' into fix-redirect-trailling-slash

3994460

zirain reviewed Mar 26, 2024

View reviewed changes

internal/xds/translator/route.go Outdated Show resolved Hide resolved

zhaohuabing marked this pull request as draft March 27, 2024 07:00

zhaohuabing force-pushed the fix-redirect-trailling-slash branch from 24f7643 to 3994460 Compare March 27, 2024 07:06

zhaohuabing added 5 commits March 27, 2024 15:07

Merge branch 'main' into fix-redirect-trailling-slash

04fef45

revert

5f957bb

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

use regex rewrite to generate the redirect url

cb7a9d3

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

use regex rewrite to generate the redirect url

e0172b1

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

use regex rewrite to generate the redirect url

30d1386

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

zhaohuabing marked this pull request as ready for review March 27, 2024 10:14

zhaohuabing marked this pull request as draft March 27, 2024 10:27

zhaohuabing added 2 commits March 27, 2024 21:35

remove comments

a9958c0

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

Merge branch 'main' into fix-redirect-trailling-slash

1296419

zhaohuabing marked this pull request as ready for review March 27, 2024 13:37

zhaohuabing requested a review from zirain March 27, 2024 13:37

zhaohuabing added 2 commits March 27, 2024 21:49

extract method

2c7c5d1

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

Merge branch 'main' into fix-redirect-trailling-slash

76acf34

arkodg reviewed Mar 28, 2024

View reviewed changes

zhaohuabing added 2 commits March 29, 2024 09:12

address comments

4dcb883

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

Merge branch 'main' into fix-redirect-trailling-slash

b2998af

zhaohuabing requested a review from arkodg March 29, 2024 09:52

arkodg approved these changes Mar 29, 2024

View reviewed changes

arkodg changed the title ~~Fix: double splashes in redirect URL~~ Fix: double slashes in redirect URL Mar 29, 2024

arkodg requested review from a team March 29, 2024 09:58

zirain approved these changes Mar 29, 2024

View reviewed changes

zirain merged commit ceb697f into envoyproxy:main Mar 29, 2024

arkodg added the cherrypick/release-v1.0 label Apr 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: double slashes in redirect URL#2998

Fix: double slashes in redirect URL#2998
zirain merged 23 commits intoenvoyproxy:mainfrom
zhaohuabing:fix-redirect-trailling-slash

zhaohuabing commented Mar 23, 2024 •

edited

Loading

Uh oh!

codecov bot commented Mar 23, 2024 •

edited

Loading

Uh oh!

arkodg commented Mar 26, 2024

Uh oh!

Uh oh!

arkodg Mar 28, 2024

Uh oh!

zhaohuabing Mar 29, 2024 •

edited

Loading

Uh oh!

arkodg commented Mar 28, 2024

Uh oh!

arkodg left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

zhaohuabing commented Mar 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

arkodg commented Mar 26, 2024

Uh oh!

Uh oh!

arkodg Mar 28, 2024

Choose a reason for hiding this comment

Uh oh!

zhaohuabing Mar 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arkodg commented Mar 28, 2024

Uh oh!

arkodg left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zhaohuabing commented Mar 23, 2024 •

edited

Loading

codecov bot commented Mar 23, 2024 •

edited

Loading

zhaohuabing Mar 29, 2024 •

edited

Loading