diff options
author | Stan Hu <stanhu@gmail.com> | 2024-01-10 03:07:49 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2024-01-10 03:07:49 +0300 |
commit | 419134ead25372663c0482fe2e58e8a1caa46cfe (patch) | |
tree | 2ca08c39ebbe6ac2e685690535f6ed48f3dc3a86 | |
parent | 9dc70599a0696ad3e08eb78a787ee2fcc412375c (diff) | |
parent | 852bde8a974154b34f8c3f0bbf42014e69f5a240 (diff) |
Merge remote-tracking branch 'origin/master' into security-master
-rw-r--r-- | go.mod | 20 | ||||
-rw-r--r-- | go.sum | 40 | ||||
-rw-r--r-- | internal/feature/feature.go | 28 | ||||
-rw-r--r-- | internal/redirects/matching.go | 107 | ||||
-rw-r--r-- | internal/redirects/matching_test.go | 355 | ||||
-rw-r--r-- | internal/redirects/redirects.go | 5 | ||||
-rw-r--r-- | internal/redirects/utils.go | 36 | ||||
-rw-r--r-- | internal/redirects/utils_test.go | 83 | ||||
-rw-r--r-- | internal/redirects/validations.go | 108 | ||||
-rw-r--r-- | internal/redirects/validations_test.go | 325 | ||||
-rw-r--r-- | internal/serving/disk/helpers.go | 10 | ||||
-rw-r--r-- | internal/serving/disk/reader.go | 7 | ||||
-rw-r--r-- | shared/pages/group.redirects/project-redirects/public.zip | bin | 2452 -> 6034 bytes | |||
-rw-r--r-- | shared/pages/group.redirects/project-redirects/public/_redirects | 1 | ||||
-rw-r--r-- | test/acceptance/redirects_test.go | 104 |
15 files changed, 1106 insertions, 123 deletions
@@ -19,7 +19,7 @@ require ( github.com/namsral/flag v1.7.4-pre github.com/patrickmn/go-cache v2.1.0+incompatible github.com/pires/go-proxyproto v0.7.0 - github.com/prometheus/client_golang v1.17.0 + github.com/prometheus/client_golang v1.18.0 github.com/rs/cors v1.7.0 github.com/sirupsen/logrus v1.9.3 github.com/stretchr/testify v1.8.4 @@ -27,10 +27,10 @@ require ( gitlab.com/feistel/go-contentencoding v1.0.0 gitlab.com/gitlab-org/go-mimedb v1.52.0 gitlab.com/gitlab-org/labkit v1.21.0 - golang.org/x/crypto v0.16.0 - golang.org/x/net v0.19.0 - golang.org/x/sync v0.5.0 - golang.org/x/sys v0.15.0 + golang.org/x/crypto v0.18.0 + golang.org/x/net v0.20.0 + golang.org/x/sync v0.6.0 + golang.org/x/sys v0.16.0 golang.org/x/time v0.3.0 ) @@ -53,18 +53,18 @@ require ( github.com/googleapis/gax-go/v2 v2.11.0 // indirect github.com/hashicorp/errwrap v1.0.0 // indirect github.com/kr/text v0.2.0 // indirect - github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect + github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 // indirect github.com/oklog/ulid/v2 v2.0.2 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/prometheus/client_model v0.4.1-0.20230718164431-9a2bf3000d16 // indirect - github.com/prometheus/common v0.44.0 // indirect - github.com/prometheus/procfs v0.11.1 // indirect + github.com/prometheus/client_model v0.5.0 // indirect + github.com/prometheus/common v0.45.0 // indirect + github.com/prometheus/procfs v0.12.0 // indirect github.com/rogpeppe/go-internal v1.11.0 // indirect github.com/sebest/xff v0.0.0-20210106013422-671bd2870b3a // indirect github.com/tj/assert v0.0.3 // indirect go.opencensus.io v0.24.0 // indirect - golang.org/x/oauth2 v0.11.0 // indirect + golang.org/x/oauth2 v0.12.0 // indirect golang.org/x/text v0.14.0 // indirect google.golang.org/api v0.126.0 // indirect google.golang.org/appengine v1.6.7 // indirect @@ -209,8 +209,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= -github.com/matttproud/golang_protobuf_extensions v1.0.4 h1:mmDVorXM7PCGKw94cs5zkfA9PSy5pEvNWRP0ET0TIVo= -github.com/matttproud/golang_protobuf_extensions v1.0.4/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4= +github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 h1:jWpvCLoY8Z/e3VKvlsiIGKtc+UG6U5vzxaoagmhXfyg= +github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0/go.mod h1:QUyp042oQthUoa9bqDv0ER0wrtXnBruoNd7aNjkbP+k= github.com/namsral/flag v1.7.4-pre h1:b2ScHhoCUkbsq0d2C15Mv+VU8bl8hAXV8arnWiOHNZs= github.com/namsral/flag v1.7.4-pre/go.mod h1:OXldTctbM6SWH1K899kPZcf65KxJiD7MsceFUpB5yDo= github.com/oklog/ulid/v2 v2.0.2 h1:r4fFzBm+bv0wNKNh5eXTwU7i85y5x+uwkxCUTNVQqLc= @@ -225,15 +225,15 @@ github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/prometheus/client_golang v1.17.0 h1:rl2sfwZMtSthVU752MqfjQozy7blglC+1SOtjMAMh+Q= -github.com/prometheus/client_golang v1.17.0/go.mod h1:VeL+gMmOAxkS2IqfCq0ZmHSL+LjWfWDUmp1mBz9JgUY= +github.com/prometheus/client_golang v1.18.0 h1:HzFfmkOzH5Q8L8G+kSJKUx5dtG87sewO+FoDDqP5Tbk= +github.com/prometheus/client_golang v1.18.0/go.mod h1:T+GXkCk5wSJyOqMIzVgvvjFDlkOQntgjkJWKrN5txjA= github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= -github.com/prometheus/client_model v0.4.1-0.20230718164431-9a2bf3000d16 h1:v7DLqVdK4VrYkVD5diGdl4sxJurKJEMnODWRJlxV9oM= -github.com/prometheus/client_model v0.4.1-0.20230718164431-9a2bf3000d16/go.mod h1:oMQmHW1/JoDwqLtg57MGgP/Fb1CJEYF2imWWhWtMkYU= -github.com/prometheus/common v0.44.0 h1:+5BrQJwiBB9xsMygAB3TNvpQKOwlkc25LbISbrdOOfY= -github.com/prometheus/common v0.44.0/go.mod h1:ofAIvZbQ1e/nugmZGz4/qCb9Ap1VoSTIO7x0VV9VvuY= -github.com/prometheus/procfs v0.11.1 h1:xRC8Iq1yyca5ypa9n1EZnWZkt7dwcoRPQwX/5gwaUuI= -github.com/prometheus/procfs v0.11.1/go.mod h1:eesXgaPo1q7lBpVMoMy0ZOFTth9hBn4W/y0/p/ScXhY= +github.com/prometheus/client_model v0.5.0 h1:VQw1hfvPvk3Uv6Qf29VrPF32JB6rtbgI6cYPYQjL0Qw= +github.com/prometheus/client_model v0.5.0/go.mod h1:dTiFglRmd66nLR9Pv9f0mZi7B7fk5Pm3gvsjB5tr+kI= +github.com/prometheus/common v0.45.0 h1:2BGz0eBc2hdMDLnO/8n0jeB3oPrt2D08CekT0lneoxM= +github.com/prometheus/common v0.45.0/go.mod h1:YJmSTw9BoKxJplESWWxlbyttQR4uaEcGyv9MZjVOJsY= +github.com/prometheus/procfs v0.12.0 h1:jluTpSng7V9hY0O2R9DzzJHYb2xULk9VTR1V1R/k6Bo= +github.com/prometheus/procfs v0.12.0/go.mod h1:pcuDEFsWDnvcgNzo4EEweacyhjeA9Zk3cnaOZAZEfOo= github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M= @@ -289,8 +289,8 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.0.0-20220314234659-1baeb1ce4c0b/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= -golang.org/x/crypto v0.16.0 h1:mMMrFzRSCF0GvB7Ne27XVtVAaXLrPmgPC7/v0tkwHaY= -golang.org/x/crypto v0.16.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= +golang.org/x/crypto v0.18.0 h1:PGVlW0xEltQnzFZ55hkuX5+KLyrMYhHld1YHO4AKcdc= +golang.org/x/crypto v0.18.0/go.mod h1:R0j02AL6hcrfOiy9T4ZYp/rcWeMxM3L6QYxlOuEG1mg= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8= @@ -364,8 +364,8 @@ golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96b golang.org/x/net v0.0.0-20210503060351-7fd8e65b6420/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= -golang.org/x/net v0.19.0 h1:zTwKpTd2XuCqf8huc7Fo2iSy+4RHPd10s4KzeTnVr1c= -golang.org/x/net v0.19.0/go.mod h1:CfAk/cbD4CthTvqiEl8NpboMuiuOYsAr/7NOjZJtv1U= +golang.org/x/net v0.20.0 h1:aCL9BSgETF1k+blQaYUBx9hJ9LOGP3gAVemcZlf1Kpo= +golang.org/x/net v0.20.0/go.mod h1:z8BVo6PvndSri0LbOE3hAn0apkU+1YvI6E70E9jsnvY= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -380,8 +380,8 @@ golang.org/x/oauth2 v0.0.0-20210313182246-cd4f82c27b84/go.mod h1:KelEdhl1UZF7XfJ golang.org/x/oauth2 v0.0.0-20210514164344-f6687ab2804c/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A= golang.org/x/oauth2 v0.0.0-20210628180205-a41e5a781914/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A= golang.org/x/oauth2 v0.0.0-20210805134026-6f1e6394065a/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A= -golang.org/x/oauth2 v0.11.0 h1:vPL4xzxBM4niKCW6g9whtaWVXTJf1U5e4aZxxFx/gbU= -golang.org/x/oauth2 v0.11.0/go.mod h1:LdF7O/8bLR/qWK9DrpXmbHLTouvRHK0SgJl0GmDBchk= +golang.org/x/oauth2 v0.12.0 h1:smVPGxink+n1ZI5pkQa8y6fZT0RW0MgCO5bFpepy4B4= +golang.org/x/oauth2 v0.12.0/go.mod h1:A74bZ3aGXgCY0qaIC9Ahg6Lglin4AMAco8cIv9baba4= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -394,8 +394,8 @@ golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.5.0 h1:60k92dhOjHxJkrqnwsfl8KuaHbn/5dl0lUPUklKo3qE= -golang.org/x/sync v0.5.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= +golang.org/x/sync v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ= +golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -443,8 +443,8 @@ golang.org/x/sys v0.0.0-20210806184541-e5e7981a1069/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc= -golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.16.0 h1:xWw16ngr6ZMtmxDyKyIgsE93KNKz5HKmMa3b8ALHidU= +golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/internal/feature/feature.go b/internal/feature/feature.go index daf5f7c9..b01e9c7a 100644 --- a/internal/feature/feature.go +++ b/internal/feature/feature.go @@ -7,6 +7,19 @@ type Feature struct { defaultEnabled bool } +// Enabled reads the environment variable responsible for the feature flag +// if FF is disabled by default, the environment variable needs to be "true" to explicitly enable it +// if FF is enabled by default, variable needs to be "false" to explicitly disable it +func (f Feature) Enabled() bool { + env := os.Getenv(f.EnvVariable) + + if f.defaultEnabled { + return env != "false" + } + + return env == "true" +} + // RedirectsPlaceholders enables support for placeholders in redirects file // TODO: remove https://gitlab.com/gitlab-org/gitlab-pages/-/issues/620 var RedirectsPlaceholders = Feature{ @@ -25,15 +38,8 @@ var ProjectPrefixCookiePath = Feature{ defaultEnabled: false, } -// Enabled reads the environment variable responsible for the feature flag -// if FF is disabled by default, the environment variable needs to be "true" to explicitly enable it -// if FF is enabled by default, variable needs to be "false" to explicitly disable it -func (f Feature) Enabled() bool { - env := os.Getenv(f.EnvVariable) - - if f.defaultEnabled { - return env != "false" - } - - return env == "true" +// DomainRedirects enables support for domain level redirects +var DomainRedirects = Feature{ + EnvVariable: "FF_ENABLE_DOMAIN_REDIRECT", + defaultEnabled: false, } diff --git a/internal/redirects/matching.go b/internal/redirects/matching.go index d52863ee..e5cb8a48 100644 --- a/internal/redirects/matching.go +++ b/internal/redirects/matching.go @@ -2,6 +2,7 @@ package redirects import ( "fmt" + "net/url" "regexp" "strings" @@ -12,11 +13,12 @@ import ( ) var ( - regexMultipleSlashes = regexp.MustCompile(`//+`) + regexMultipleSlashes = regexp.MustCompile(`([^:])//+`) regexPlaceholderOrSplats = regexp.MustCompile(`(?i)\*|:[a-z]+`) ) // matchesRule returns `true` if the rule's "from" pattern matches the requested URL. +// This internally calls matchesRuleWithPlaceholderOrSplats to match rules. // // For example, given a "from" URL like this: // @@ -30,37 +32,56 @@ var ( // If the first return value is `true`, the second return value is the path that this // rule should redirect/rewrite to. This path is effectively the rule's "to" path that // has been templated with all the placeholders (if any) from the originally requested URL. -// -// TODO: Likely these should include host comparison once we have domain-level redirects -// https://gitlab.com/gitlab-org/gitlab-pages/-/issues/601 -func matchesRule(rule *netlifyRedirects.Rule, path string) (bool, string) { +func matchesRule(rule *netlifyRedirects.Rule, originalURL *url.URL) (bool, string) { + hostMatches, fromPath := matchHost(originalURL, rule.From) + + if !hostMatches { + return false, "" + } + + path := originalURL.Path + + if !feature.DomainRedirects.Enabled() && isDomainURL(rule.To) { + return false, "" + } + // If the requested URL exactly matches this rule's "from" path, // exit early and return the rule's "to" path to avoid building // and compiling the regex below. // However, only do this if there's nothing to template in the "to" path, - // to avoid redirect/rewriting to a url with a literal `:placeholder` in it. - if normalizePath(rule.From) == normalizePath(path) && !regexPlaceholderOrSplats.MatchString(rule.To) { + // to avoid redirect/rewriting to a originalURL with a literal `:placeholder` in it. + if normalizePath(fromPath) == normalizePath(path) && !regexPlaceholderOrSplats.MatchString(rule.To) { return true, rule.To } + return matchesRuleWithPlaceholderOrSplats(path, fromPath, rule.To, rule.Status) +} + +// matchesRuleWithPlaceholderOrSplats returns `true` if the rule's "from" pattern matches the requested URL. +// This is specifically for Placeholders and Splats matching +// +// For example, given a "from" URL like this: +// +// /a/*/originalURL/with/:placeholders +// +// this function would match URLs like this: +// +// /a/nice/originalURL/with/text +// /a/super/extra/nice/originalURL/with/matches +// +// If the first return value is `true`, the second return value is the path that this +// rule should redirect/rewrite to. This path is effectively the rule's "to" path that +// has been templated with all the placeholders (if any) from the originally requested URL. +func matchesRuleWithPlaceholderOrSplats(requestPath string, fromPath string, toPath string, status int) (bool, string) { // Any logic beyond this point handles placeholders and splats. // If the FF_ENABLE_PLACEHOLDERS feature flag isn't enabled, exit now. if !feature.RedirectsPlaceholders.Enabled() { return false, "" } - var regexSegments []string - for _, segment := range strings.Split(rule.From, "/") { - if segment == "" { - continue - } else if regexSplat.MatchString(segment) { - regexSegments = append(regexSegments, `/*(?P<splat>.*)/*`) - } else if regexPlaceholder.MatchString(segment) { - segmentName := strings.Replace(segment, ":", "", 1) - regexSegments = append(regexSegments, fmt.Sprintf(`/+(?P<%s>[^/]+)`, segmentName)) - } else { - regexSegments = append(regexSegments, "/+"+regexp.QuoteMeta(segment)) - } + regexSegments := convertToRegexSegments(fromPath) + if len(regexSegments) == 0 { + return false, "" } fromRegexString := `(?i)^` + strings.Join(regexSegments, "") + `/*$` @@ -68,40 +89,64 @@ func matchesRule(rule *netlifyRedirects.Rule, path string) (bool, string) { if err != nil { log.WithFields(log.Fields{ "fromRegexString": fromRegexString, - "rule.From": rule.From, - "rule.To": rule.To, - "rule.Status": rule.Status, - "path": path, + "rule.From": fromPath, + "rule.To": toPath, + "rule.Status": status, + "path": requestPath, }).WithError(err).Warnf("matchesRule generated an invalid regex: %q", fromRegexString) return false, "" } - template := regexPlaceholderReplacement.ReplaceAllString(rule.To, `${$placeholder}`) - submatchIndex := fromRegex.FindStringSubmatchIndex(path) + template := regexPlaceholderReplacement.ReplaceAllString(toPath, `${$placeholder}`) + subMatchIndex := fromRegex.FindStringSubmatchIndex(requestPath) - if submatchIndex == nil { + if subMatchIndex == nil { return false, "" } - templatedToPath := []byte{} - templatedToPath = fromRegex.ExpandString(templatedToPath, template, path, submatchIndex) + var templatedToPath []byte + templatedToPath = fromRegex.ExpandString(templatedToPath, template, requestPath, subMatchIndex) // Some replacements result in subsequent slashes. For example, a rule with a "to" // like `foo/:splat/bar` will result in a path like `foo//bar` if the splat // character matches nothing. To avoid this, replace all instances // of multiple subsequent forward slashes with a single forward slash. - templatedToPath = regexMultipleSlashes.ReplaceAll(templatedToPath, []byte("/")) + // The regex captures any character except a colon ([^:]) before the double slashes + // and includes it in the replacement. + templatedToPath = regexMultipleSlashes.ReplaceAll(templatedToPath, []byte("$1/")) return true, string(templatedToPath) } +// convertToRegexSegments converts the path string to an array of regex segments +// It replaces placeholders with named capture groups and splat characters with a wildcard regex +// This allows matching the path segments to the request path and extracting matched placeholder values +func convertToRegexSegments(path string) []string { + var regexSegments []string + + for _, segment := range strings.Split(path, "/") { + if segment == "" { + continue + } else if regexSplat.MatchString(segment) { + regexSegments = append(regexSegments, `/*(?P<splat>.*)/*`) + } else if regexPlaceholder.MatchString(segment) { + segmentName := strings.Replace(segment, ":", "", 1) + regexSegments = append(regexSegments, fmt.Sprintf(`/+(?P<%s>[^/]+)`, segmentName)) + } else { + regexSegments = append(regexSegments, "/+"+regexp.QuoteMeta(segment)) + } + } + + return regexSegments +} + // `match` returns: // 1. The first valid redirect or rewrite rule that matches the requested URL // 2. The URL to redirect/rewrite to // // If no rule matches, this function returns `nil` and an empty string -func (r *Redirects) match(path string) (*netlifyRedirects.Rule, string) { +func (r *Redirects) match(originalURL *url.URL) (*netlifyRedirects.Rule, string) { for i := range r.rules { if i >= cfg.MaxRuleCount { // do not process any more rules @@ -116,7 +161,7 @@ func (r *Redirects) match(path string) (*netlifyRedirects.Rule, string) { continue } - if isMatch, path := matchesRule(&rule, path); isMatch { + if isMatch, path := matchesRule(&rule, originalURL); isMatch { return &rule, path } } diff --git a/internal/redirects/matching_test.go b/internal/redirects/matching_test.go index 23815b97..3938b680 100644 --- a/internal/redirects/matching_test.go +++ b/internal/redirects/matching_test.go @@ -1,6 +1,7 @@ package redirects import ( + "net/url" "testing" "github.com/stretchr/testify/require" @@ -56,7 +57,7 @@ var testsWithoutPlaceholders = map[string]testCaseData{ }, } -func Test_matchesRule(t *testing.T) { +func testMatchesRule(t *testing.T) { t.Setenv(feature.RedirectsPlaceholders.EnvVariable, "true") tests := mergeTestSuites(testsWithoutPlaceholders, map[string]testCaseData{ @@ -68,11 +69,12 @@ func Test_matchesRule(t *testing.T) { expectMatch: true, expectedPath: "/bar/", }, + // Since we are treating path as an entire path, so // is considered domain name "multiple_leading_slashes": { rule: "/foo/ /bar/", path: "//foo", - expectMatch: true, - expectedPath: "/bar/", + expectMatch: false, + expectedPath: "", }, "multiple_slashes_in_middle": { rule: "/foo/bar /baz/", @@ -81,6 +83,13 @@ func Test_matchesRule(t *testing.T) { expectedPath: "/baz/", }, + "schema_host_from_url": { + rule: "http://pages.example.io/foo /baz/", + path: "http://pages.example.io/foo", + expectMatch: true, + expectedPath: "/baz/", + }, + "splat_match": { rule: "/foo/*/bar /foo/:splat/qux", path: "/foo/baz/bar", @@ -153,12 +162,6 @@ func Test_matchesRule(t *testing.T) { expectMatch: true, expectedPath: "/qux/foo/bar", }, - "multiple_splats": { - rule: "/foo/*/bar/*/baz /qux/:splat", - path: "/foo/hello/bar/world/baz", - expectMatch: true, - expectedPath: "/qux/hello", - }, "duplicate_splat_replacements": { rule: "/foo/* /qux/:splat/:splat", path: "/foo/hello", @@ -301,13 +304,25 @@ func Test_matchesRule(t *testing.T) { rules, err := netlifyRedirects.ParseString(tt.rule) require.NoError(t, err) - isMatch, path := matchesRule(&rules[0], tt.path) + parsedURL, err := url.Parse(tt.path) + require.NoError(t, err) + + isMatch, path := matchesRule(&rules[0], parsedURL) require.Equal(t, tt.expectMatch, isMatch) require.Equal(t, tt.expectedPath, path) }) } } +func Test_matchesRule(t *testing.T) { + testMatchesRule(t) +} + +func Test_matchesNonDomainRule_DomainRedirects_enabled(t *testing.T) { + t.Setenv(feature.DomainRedirects.EnvVariable, "true") + testMatchesRule(t) +} + // Tests matching behavior when the `FF_ENABLE_PLACEHOLDERS` // feature flag is not enabled. These tests can be removed when the // `FF_ENABLE_PLACEHOLDERS` flag is removed. @@ -343,7 +358,325 @@ func Test_matchesRule_NoPlaceholders(t *testing.T) { rules, err := netlifyRedirects.ParseString(tt.rule) require.NoError(t, err) - isMatch, path := matchesRule(&rules[0], tt.path) + parsedURL, err := url.Parse(tt.path) + require.NoError(t, err) + + isMatch, path := matchesRule(&rules[0], parsedURL) + require.Equal(t, tt.expectMatch, isMatch) + require.Equal(t, tt.expectedPath, path) + }) + } +} + +// Tests matching behavior when the `FF_ENABLE_DOMAIN_REDIRECT` +// feature flag is enabled. +func Test_matchesDomainRule(t *testing.T) { + t.Setenv(feature.RedirectsPlaceholders.EnvVariable, "true") + t.Setenv(feature.DomainRedirects.EnvVariable, "true") + + tests := map[string]testCaseData{ + "exact_path_match": { + rule: "/foo/ http://test.example.io/bar/", + path: "http://pages.example.io/foo/", + expectMatch: true, + expectedPath: "http://test.example.io/bar/", + }, + "exact_url_match": { + rule: "http://pages.example.io/foo/ http://test.example.io/bar/", + path: "http://pages.example.io/foo/", + expectMatch: true, + expectedPath: "http://test.example.io/bar/", + }, + "single_trailing_slash": { + rule: "http://pages.example.io/foo/ http://test.example.io/bar/", + path: "http://pages.example.io/foo", + expectMatch: true, + expectedPath: "http://test.example.io/bar/", + }, + "ignore_missing_slash": { + rule: "http://pages.example.io/foo http://test.example.io/bar/", + path: "http://pages.example.io/foo/", + expectMatch: true, + expectedPath: "http://test.example.io/bar/", + }, + "no_match": { + rule: "http://pages.example.io/foo http://test.example.io/bar/", + path: "http://pages.example.io/foo/bar", + expectMatch: false, + expectedPath: "", + }, + + // Note: the following 3 cases behave differently when + // placeholders are disabled. See the similar test cases below. + "multiple_trailing_slashes": { + rule: "http://pages.example.io/foo/ http://test.example.io/bar/", + path: "http://pages.example.io/foo//", + expectMatch: true, + expectedPath: "http://test.example.io/bar/", + }, + "multiple_slashes_in_middle": { + rule: "http://pages.example.io/foo/bar http://test.example.io/baz/", + path: "http://pages.example.io/foo//bar", + expectMatch: true, + expectedPath: "http://test.example.io/baz/", + }, + + "domain_redirect_different_protocol": { + rule: "http://pages.example.io/foo/bar https://test.example.io/baz/", + path: "http://pages.example.io/foo//bar", + expectMatch: true, + expectedPath: "https://test.example.io/baz/", + }, + + "splat_match": { + rule: "http://pages.example.io/foo/*/bar http://test.example.io/foo/:splat/qux", + path: "http://pages.example.io/foo/baz/bar", + expectMatch: true, + expectedPath: "http://test.example.io/foo/baz/qux", + }, + "splat_match_multiple_segments": { + rule: "http://pages.example.io/foo/*/bar http://test.example.io/foo/:splat/qux", + path: "http://pages.example.io/foo/hello/world/bar", + expectMatch: true, + expectedPath: "http://test.example.io/foo/hello/world/qux", + }, + "splat_match_ignore_trailing_slash": { + rule: "http://pages.example.io/foo/*/bar http://test.example.io/foo/:splat/qux", + path: "http://pages.example.io/foo/baz/bar/", + expectMatch: true, + expectedPath: "http://test.example.io/foo/baz/qux", + }, + "splat_match_end": { + rule: "http://pages.example.io/foo/* http://test.example.io/qux/:splat", + path: "http://pages.example.io/foo/baz/bar", + expectMatch: true, + expectedPath: "http://test.example.io/qux/baz/bar", + }, + "splat_match_end_with_slash": { + rule: "http://pages.example.io/foo/* http://test.example.io/qux/:splat", + path: "http://pages.example.io/foo/baz/bar/", + expectMatch: true, + expectedPath: "http://test.example.io/qux/baz/bar/", + }, + "splat_match_beginning": { + rule: "http://pages.example.io/*/baz/bar http://test.example.io/qux/:splat", + path: "http://pages.example.io/foo/baz/bar", + expectMatch: true, + expectedPath: "http://test.example.io/qux/foo", + }, + "splat_match_empty_suffix": { + rule: "http://pages.example.io/foo/* http://test.example.io/qux/:splat", + path: "http://pages.example.io/foo/", + expectMatch: true, + expectedPath: "http://test.example.io/qux/", + }, + "splat_consumes_trailing_slash": { + rule: "http://pages.example.io/foo/* http://test.example.io/qux/:splat", + path: "http://pages.example.io/foo", + expectMatch: true, + expectedPath: "http://test.example.io/qux/", + }, + "splat_match_empty_prefix": { + rule: "http://pages.example.io/*/foo http://test.example.io/qux/:splat", + path: "http://pages.example.io/foo", + expectMatch: true, + expectedPath: "http://test.example.io/qux/", + }, + "splat_mid_segment": { + rule: "http://pages.example.io/foo*bar http://test.example.io/qux/:splat", + path: "http://pages.example.io/foobazbar", + expectMatch: false, + expectedPath: "", + }, + "splat_mid_segment_no_content": { + rule: "http://pages.example.io/foo*bar http://test.example.io/qux/:splat", + path: "http://pages.example.io/foobar", + expectMatch: false, + expectedPath: "", + }, + "lone_splat": { + rule: "http://pages.example.io/* http://test.example.io/qux/:splat", + path: "http://pages.example.io/foo/bar", + expectMatch: true, + expectedPath: "http://test.example.io/qux/foo/bar", + }, + "duplicate_splat_replacements": { + rule: "http://pages.example.io/foo/* http://test.example.io/qux/:splat/:splat", + path: "http://pages.example.io/foo/hello", + expectMatch: true, + expectedPath: "http://test.example.io/qux/hello/hello", + }, + "splat_missing_path_segment_behavior": { + rule: "http://pages.example.io/foo/*/bar http://test.example.io/foo/:splat/qux", + path: "http://pages.example.io/foo/bar", + expectMatch: true, + expectedPath: "http://test.example.io/foo/qux", + }, + "missing_splat_placeholder": { + rule: "http://pages.example.io/foo/ http://test.example.io/qux/:splat", + path: "http://pages.example.io/foo/", + expectMatch: true, + expectedPath: "http://test.example.io/qux/", + }, + "placeholder_match": { + rule: "http://pages.example.io/foo/:year/:month/:day/bar http://test.example.io/qux/:year-:month-:day", + path: "http://pages.example.io/foo/2021/08/16/bar", + expectMatch: true, + expectedPath: "http://test.example.io/qux/2021-08-16", + }, + "placeholder_match_end": { + rule: "http://pages.example.io/foo/:placeholder http://test.example.io/qux/:placeholder", + path: "http://pages.example.io/foo/bar", + expectMatch: true, + expectedPath: "http://test.example.io/qux/bar", + }, + "placeholder_match_beginning": { + rule: "http://pages.example.io/:placeholder/foo http://test.example.io/qux/:placeholder", + path: "http://pages.example.io/baz/foo", + expectMatch: true, + expectedPath: "http://test.example.io/qux/baz", + }, + "placeholder_no_multiple_segments": { + rule: "http://pages.example.io/foo/:placeholder/bar http://test.example.io/foo/:placeholder/qux", + path: "http://pages.example.io/foo/hello/world/bar", + expectMatch: false, + expectedPath: "", + }, + "placeholder_at_beginning_no_content": { + rule: "http://pages.example.io/:placeholder/foo http://test.example.io/qux/:placeholder", + path: "http://pages.example.io/foo", + expectMatch: false, + expectedPath: "", + }, + "placeholder_at_end_no_content": { + rule: "http://pages.example.io/foo/:placeholder http://test.example.io/qux/:placeholder", + path: "http://pages.example.io/foo/", + expectMatch: false, + expectedPath: "", + }, + "placeholder_mid_segment_in_from": { + rule: "http://pages.example.io/foo:placeholder http://test.example.io/qux/:placeholder", + path: "http://pages.example.io/foorbar", + expectMatch: false, + expectedPath: "", + }, + "placeholder_mid_segment_in_to": { + rule: "http://pages.example.io/foo/:placeholder http://test.example.io/qux/bar:placeholder", + path: "http://pages.example.io/foo/baz", + expectMatch: true, + expectedPath: "http://test.example.io/qux/barbaz", + }, + "placeholder_missing_replacement_with_substring": { + rule: "http://pages.example.io/:foo http://test.example.io/:foobar", + path: "http://pages.example.io/baz", + expectMatch: true, + expectedPath: "http://test.example.io/", + }, + "placeholder_mid_segment_no_content": { + rule: "http://pages.example.io/foo:placeholder http://test.example.io/qux/:splat", + path: "http://pages.example.io/foo", + expectMatch: false, + expectedPath: "", + }, + "placeholder_name_substring": { + rule: "http://pages.example.io/foo/:foo/:foobar http://test.example.io/qux/:foo/:foobar", + path: "http://pages.example.io/foo/baz/quux", + expectMatch: true, + expectedPath: "http://test.example.io/qux/baz/quux", + }, + "lone_placeholder": { + rule: "http://pages.example.io/:placeholder http://test.example.io/qux/:placeholder", + path: "http://pages.example.io/foo", + expectMatch: true, + expectedPath: "http://test.example.io/qux/foo", + }, + "duplicate_placeholders": { + rule: "http://pages.example.io/foo/:placeholder/bar/:placeholder/baz http://test.example.io/qux/:placeholder", + path: "http://pages.example.io/foo/hello/bar/world/baz", + expectMatch: true, + expectedPath: "http://test.example.io/qux/hello", + }, + "duplicate_placeholder_replacements": { + rule: "http://pages.example.io/foo/:placeholder http://test.example.io/qux/:placeholder/:placeholder", + path: "http://pages.example.io/foo/hello", + expectMatch: true, + expectedPath: "http://test.example.io/qux/hello/hello", + }, + "splat_and_placeholder_named_splat": { + rule: "http://pages.example.io/foo/*/bar/:splat http://test.example.io/qux/:splat", + path: "http://pages.example.io/foo/hello/bar/world", + expectMatch: true, + expectedPath: "http://test.example.io/qux/hello", + }, + "placeholder_named_splat_and_splat": { + rule: "http://pages.example.io/foo/:splat/bar/* http://test.example.io/qux/:splat", + path: "http://pages.example.io/foo/hello/bar/world", + expectMatch: true, + expectedPath: "http://test.example.io/qux/hello", + }, + + // Note: These differ slightly from Netlify's matching behavior. + // GitLab replaces _all_ placeholders in the "to" path, even if + // the placeholder doesn't have corresponding match in the "from". + // Netlify only replaces placeholders that appear in the "from". + "missing_placeholder_exact_match": { + rule: "http://pages.example.io/foo/ http://test.example.io/qux/:placeholder", + path: "http://pages.example.io/foo/", + expectMatch: true, + + // Netlify would instead redirect to "/qux/:placeholder" + expectedPath: "http://test.example.io/qux/", + }, + "missing_placeholder_nonexact_match": { + rule: "http://pages.example.io/foo/:placeholderA http://test.example.io/qux/:placeholderB", + path: "http://pages.example.io/foo/bar", + expectMatch: true, + + // Netlify would instead redirect to "/qux/:placeholderB" + expectedPath: "http://test.example.io/qux/", + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + rules, err := netlifyRedirects.ParseString(tt.rule) + require.NoError(t, err) + + parsedURL, err := url.Parse(tt.path) + require.NoError(t, err) + + isMatch, path := matchesRule(&rules[0], parsedURL) + require.Equal(t, tt.expectMatch, isMatch) + require.Equal(t, tt.expectedPath, path) + }) + } +} + +// Tests matching behavior when the `FF_ENABLE_DOMAIN_REDIRECT` +// feature flag is not enabled. These tests can be removed when the +// `FF_ENABLE_DOMAIN_REDIRECT` flag is removed. +func Test_matchesDomainRule_DomainRedirect_Disabled(t *testing.T) { + t.Setenv(feature.RedirectsPlaceholders.EnvVariable, "true") + t.Setenv(feature.DomainRedirects.EnvVariable, "false") + + tests := map[string]testCaseData{ + "exact_match": { + rule: "http://pages.example.io/foo/ http://test.example.io/bar/", + path: "http://pages.example.io/foo/", + expectMatch: false, + expectedPath: "", + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + rules, err := netlifyRedirects.ParseString(tt.rule) + require.NoError(t, err) + + parsedURL, err := url.Parse(tt.path) + require.NoError(t, err) + + isMatch, path := matchesRule(&rules[0], parsedURL) require.Equal(t, tt.expectMatch, isMatch) require.Equal(t, tt.expectedPath, path) }) diff --git a/internal/redirects/redirects.go b/internal/redirects/redirects.go index 4903518a..2c01f396 100644 --- a/internal/redirects/redirects.go +++ b/internal/redirects/redirects.go @@ -52,8 +52,11 @@ var ( errFailedToParseConfig = errors.New("failed to parse _redirects file") errFailedToParseURL = errors.New("unable to parse URL") errNoDomainLevelRedirects = errors.New("no domain-level redirects to outside sites") + errNoDomainLevelRewrite = errors.New("no domain-level rewrite to outside sites") errNoStartingForwardSlashInURLPath = errors.New("url path must start with forward slash /") + errNoValidStartingInURLPath = errors.New("url path must start with forward slash / or http:// or https://") errNoSplats = errors.New("splats are not enabled. See https://docs.gitlab.com/ee/user/project/pages/redirects.html#feature-flag-for-rewrites") + errMoreThanOneSplats = errors.New("rule cannot contain more than 1 asterisk (*) in its from path") errNoPlaceholders = errors.New("placeholders are not enabled. See https://docs.gitlab.com/ee/user/project/pages/redirects.html#feature-flag-for-rewrites") errNoParams = errors.New("params not supported") errUnsupportedStatus = errors.New("status not supported") @@ -111,7 +114,7 @@ func (r *Redirects) Rewrite(originalURL *url.URL) (*url.URL, int, error) { return nil, 0, ErrNoRedirect } - rule, newPath := r.match(originalURL.Path) + rule, newPath := r.match(originalURL) if rule == nil { return nil, 0, ErrNoRedirect } diff --git a/internal/redirects/utils.go b/internal/redirects/utils.go new file mode 100644 index 00000000..b9aa819f --- /dev/null +++ b/internal/redirects/utils.go @@ -0,0 +1,36 @@ +package redirects + +import "net/url" + +// isDomainURL checks if the given urlString is a valid domain URL with scheme and host parts. +// Returns true if urlString is a valid domain URL, false otherwise. +func isDomainURL(urlString string) bool { + parsedURL, err := url.Parse(urlString) + if err != nil { + return false + } + if len(parsedURL.Scheme) > 0 && len(parsedURL.Host) > 0 { + return true + } + return false +} + +// Write documentation for below method +// matchHost checks if the originalURL matches the domain and path provided in path argument. +// It returns a bool indicating if there is a host match and the matched path. +// It returns true and path when path does not contain scheme and host +func matchHost(originalURL *url.URL, path string) (bool, string) { + if !isDomainURL(path) { + return true, path + } + + parsedURL, err := url.Parse(path) + if err != nil { + return false, "" + } + + if originalURL.Scheme == parsedURL.Scheme && originalURL.Host == parsedURL.Host { + return true, parsedURL.Path + } + return false, "" +} diff --git a/internal/redirects/utils_test.go b/internal/redirects/utils_test.go new file mode 100644 index 00000000..cda24e30 --- /dev/null +++ b/internal/redirects/utils_test.go @@ -0,0 +1,83 @@ +package redirects + +import ( + "net/url" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestIsDomainURL(t *testing.T) { + tests := map[string]struct { + url string + expectedBool bool + }{ + "only_path": { + url: "/goto.html", + expectedBool: false, + }, + "valid_domain_url": { + url: "https://GitLab.com", + expectedBool: true, + }, + "schemaless_domain_url_with_special_char": { + url: "/\\GitLab.com", + expectedBool: false, + }, + "schemaless_domain_url": { + url: "//GitLab.com/pages.html", + expectedBool: false, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + require.EqualValues(t, tt.expectedBool, isDomainURL(tt.url)) + }) + } +} + +func TestMatchHost(t *testing.T) { + tests := map[string]struct { + originalURL string + path string + expectedBool bool + expectedPath string + }{ + "path_without_domain": { + originalURL: "https://GitLab.com/goto.html", + path: "/goto.html", + expectedBool: true, + expectedPath: "/goto.html", + }, + "valid_matching_host": { + originalURL: "https://GitLab.com/goto.html", + path: "https://GitLab.com/goto.html", + expectedBool: true, + expectedPath: "/goto.html", + }, + "different_schema_path": { + originalURL: "http://GitLab.com/goto.html", + path: "https://GitLab.com/goto.html", + expectedBool: false, + expectedPath: "", + }, + "different_host_path": { + originalURL: "https://GitLab.com/goto.html", + path: "https://GitLab-test.com/goto.html", + expectedBool: false, + expectedPath: "", + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + parsedURL, err := url.Parse(tt.originalURL) + require.NoError(t, err) + + hostMatches, path := matchHost(parsedURL, tt.path) + require.EqualValues(t, tt.expectedBool, hostMatches) + require.EqualValues(t, tt.expectedPath, path) + }) + } +} diff --git a/internal/redirects/validations.go b/internal/redirects/validations.go index 86fb0212..c469ecfc 100644 --- a/internal/redirects/validations.go +++ b/internal/redirects/validations.go @@ -18,43 +18,110 @@ var ( regexPlaceholderReplacement = regexp.MustCompile(`(?i):(?P<placeholder>[a-z]+)`) ) -// validateURL runs validations against a rule URL. +// validateFromURL validates the from URL in a redirect rule. +// It checks for various invalid cases like unsupported schemes, +// relative URLs, domain redirects without scheme, etc. // Returns `nil` if the URL is valid. -func validateURL(urlText string) error { - url, err := url.Parse(urlText) +// nolint: gocyclo +func validateFromURL(urlText string) error { + fromURL, err := url.Parse(urlText) if err != nil { return errFailedToParseURL } - // No support for domain-level redirects to outside sites: - // - `https://google.com` + // No support for domain level redirects starting with special characters without scheme: // - `//google.com` // - `/\google.com` - if url.Host != "" || url.Scheme != "" || strings.HasPrefix(url.Path, "/\\") { - return errNoDomainLevelRedirects + if (fromURL.Host == "") != (fromURL.Scheme == "") || strings.HasPrefix(fromURL.Path, "/\\") { + return errNoValidStartingInURLPath + } + + if fromURL.Scheme != "" && fromURL.Scheme != "http" && fromURL.Scheme != "https" { + return errNoValidStartingInURLPath + } + + if fromURL.Scheme == "" && fromURL.Host == "" { + // No parent traversing relative URL's with `./` or `../` + // No ambiguous URLs like bare domains `GitLab.com` + if !strings.HasPrefix(urlText, "/") { + return errNoValidStartingInURLPath + } + } + + if feature.RedirectsPlaceholders.Enabled() && strings.Count(fromURL.Path, "/*") > 1 { + return errMoreThanOneSplats } - // No parent traversing relative URL's with `./` or `../` - // No ambiguous URLs like bare domains `GitLab.com` - if !strings.HasPrefix(url.Path, "/") { - return errNoStartingForwardSlashInURLPath + return validateSplatAndPlaceholders(fromURL.Path) +} + +// validateURL runs validations against a rule URL. +// Returns `nil` if the URL is valid. +// nolint: gocyclo +func validateToURL(urlText string, status int) error { + toURL, err := url.Parse(urlText) + if err != nil { + return errFailedToParseURL } + allowedPrefix := []string{"/"} + if feature.DomainRedirects.Enabled() { + // No support for domain level redirects starting with // or special characters: + // - `//google.com` + // - `/\google.com` + if (toURL.Host == "") != (toURL.Scheme == "") || strings.HasPrefix(toURL.Path, "/\\") { + return errNoValidStartingInURLPath + } + + // No support for domain level rewrite + if isDomainURL(urlText) { + if status == http.StatusOK { + return errNoDomainLevelRewrite + } + allowedPrefix = append(allowedPrefix, "http://", "https://") + } + + // No parent traversing relative URL's with `./` or `../` + // No ambiguous URLs like bare domains `GitLab.com` + if !startsWithAnyPrefix(urlText, allowedPrefix...) { + return errNoValidStartingInURLPath + } + } else { + // No support for domain-level redirects to outside sites: + // - `https://google.com` + // - `//google.com` + // - `/\google.com` + if toURL.Host != "" || toURL.Scheme != "" || strings.HasPrefix(toURL.Path, "/\\") { + return errNoDomainLevelRedirects + } + + // No parent traversing relative URL's with `./` or `../` + // No ambiguous URLs like bare domains `GitLab.com` + if !startsWithAnyPrefix(urlText, allowedPrefix...) { + return errNoStartingForwardSlashInURLPath + } + } + + return validateSplatAndPlaceholders(toURL.Path) +} + +func validateSplatAndPlaceholders(path string) error { if feature.RedirectsPlaceholders.Enabled() { + maxPathSegments := cfg.MaxPathSegments // Limit the number of path segments a rule can contain. // This prevents the matching logic from generating regular // expressions that are too large/complex. - if strings.Count(url.Path, "/") > cfg.MaxPathSegments { + if strings.Count(path, "/") > maxPathSegments { return fmt.Errorf("url path cannot contain more than %d forward slashes", cfg.MaxPathSegments) } } else { // No support for splats, https://docs.netlify.com/routing/redirects/redirect-options/#splats - if strings.Contains(url.Path, "*") { + if strings.Contains(path, "*") { return errNoSplats } // No support for placeholders, https://docs.netlify.com/routing/redirects/redirect-options/#placeholders - if regexpPlaceholder.MatchString(url.Path) { + if regexpPlaceholder.MatchString(path) { return errNoPlaceholders } } @@ -65,11 +132,11 @@ func validateURL(urlText string) error { // validateRule runs all validation rules on the provided rule. // Returns `nil` if the rule is valid func validateRule(r netlifyRedirects.Rule) error { - if err := validateURL(r.From); err != nil { + if err := validateFromURL(r.From); err != nil { return err } - if err := validateURL(r.To); err != nil { + if err := validateToURL(r.To, r.Status); err != nil { return err } @@ -93,3 +160,12 @@ func validateRule(r netlifyRedirects.Rule) error { return nil } + +func startsWithAnyPrefix(s string, prefixes ...string) bool { + for _, prefix := range prefixes { + if strings.HasPrefix(s, prefix) { + return true + } + } + return false +} diff --git a/internal/redirects/validations_test.go b/internal/redirects/validations_test.go index 2b8ac1b1..13f115cd 100644 --- a/internal/redirects/validations_test.go +++ b/internal/redirects/validations_test.go @@ -2,6 +2,7 @@ package redirects import ( "fmt" + "net/http" "strings" "testing" @@ -11,7 +12,68 @@ import ( "gitlab.com/gitlab-org/gitlab-pages/internal/feature" ) -func TestRedirectsValidateUrl(t *testing.T) { +func TestRedirectsValidateFromUrl(t *testing.T) { + t.Setenv(feature.RedirectsPlaceholders.EnvVariable, "true") + + tests := map[string]struct { + url string + expectedErr error + }{ + "valid_url": { + url: "/goto.html", + }, + "no_domain_level_redirects": { + url: "https://GitLab.com", + }, + "no_special_characters_escape_domain_level_redirects": { + url: "/\\GitLab.com", + expectedErr: errNoValidStartingInURLPath, + }, + "no_schemaless_url_domain_level_redirects": { + url: "//GitLab.com/pages.html", + expectedErr: errNoValidStartingInURLPath, + }, + "no_bare_domain_level_redirects": { + url: "GitLab.com", + expectedErr: errNoValidStartingInURLPath, + }, + "no_parent_traversing_relative_url": { + url: "../target.html", + expectedErr: errNoValidStartingInURLPath, + }, + "too_many_slashes": { + url: strings.Repeat("/a", 26), + expectedErr: fmt.Errorf("url path cannot contain more than %d forward slashes", defaultMaxPathSegments), + }, + "placeholders": { + url: "/news/:year/:month/:date/:slug", + }, + "splats": { + url: "/blog/*", + }, + "no_multiple_splats_redirects": { + url: "/foo/*/bar/*/baz", + expectedErr: errMoreThanOneSplats, + }, + "splat_placeholders": { + url: "/new/path/:splat", + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + err := validateFromURL(tt.url) + if tt.expectedErr != nil { + require.EqualError(t, err, tt.expectedErr.Error()) + return + } + + require.NoError(t, err) + }) + } +} + +func TestRedirectsValidateToUrl(t *testing.T) { t.Setenv(feature.RedirectsPlaceholders.EnvVariable, "true") tests := map[string]struct { @@ -58,7 +120,7 @@ func TestRedirectsValidateUrl(t *testing.T) { for name, tt := range tests { t.Run(name, func(t *testing.T) { - err := validateURL(tt.url) + err := validateToURL(tt.url, http.StatusMovedPermanently) if tt.expectedErr != nil { require.EqualError(t, err, tt.expectedErr.Error()) return @@ -92,7 +154,7 @@ func TestRedirectsValidateUrlNoPlaceholders(t *testing.T) { for name, tt := range tests { t.Run(name, func(t *testing.T) { - err := validateURL(tt.url) + err := validateToURL(tt.url, http.StatusMovedPermanently) require.ErrorIs(t, err, tt.expectedErr) }) } @@ -108,9 +170,12 @@ func TestRedirectsValidateRule(t *testing.T) { "valid_rule": { rule: "/goto.html /target.html 301", }, + "valid_from_host_url": { + rule: "http://valid.com/ /teapot.html 302", + }, "invalid_from_url": { rule: "invalid.com /teapot.html 302", - expectedErr: errNoStartingForwardSlashInURLPath, + expectedErr: errNoValidStartingInURLPath, }, "invalid_to_url": { rule: "/goto.html invalid.com", @@ -140,3 +205,255 @@ func TestRedirectsValidateRule(t *testing.T) { }) } } + +func TestRedirectsValidateFromUrl_DomainRedirect_Enabled(t *testing.T) { + t.Setenv(feature.RedirectsPlaceholders.EnvVariable, "true") + + tests := map[string]struct { + url string + expectedErr error + }{ + "valid_url": { + url: "/goto.html", + }, + "domain_level": { + url: "https://GitLab.com", + }, + "no_special_characters_escape_domain_level": { + url: "/\\GitLab.com", + expectedErr: errNoValidStartingInURLPath, + }, + "no_schemaless_url_domain_level": { + url: "//GitLab.com/pages.html", + expectedErr: errNoValidStartingInURLPath, + }, + "no_bare_domain_level": { + url: "GitLab.com", + expectedErr: errNoValidStartingInURLPath, + }, + "no_parent_traversing_relative_url": { + url: "../target.html", + expectedErr: errNoValidStartingInURLPath, + }, + "too_many_slashes": { + url: strings.Repeat("/a", 26), + expectedErr: fmt.Errorf("url path cannot contain more than %d forward slashes", defaultMaxPathSegments), + }, + "placeholders": { + url: "/news/:year/:month/:date/:slug", + }, + "domain_redirect_placeholders": { + url: "https://GitLab.com/news/:year/:month/:date/:slug", + }, + "splats": { + url: "/blog/*", + }, + "lone_splats": { + url: "https://GitLab.com/*", + }, + "domain_redirect_splats": { + url: "https://GitLab.com/blog/*", + }, + "splat_placeholders": { + url: "/new/path/:splat", + }, + "domain_redirect_splat_placeholders": { + url: "https://GitLab.com/new/path/:splat", + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + err := validateFromURL(tt.url) + if tt.expectedErr != nil { + require.EqualError(t, err, tt.expectedErr.Error()) + return + } + + require.NoError(t, err) + }) + } +} + +func TestRedirectsValidateToUrl_DomainRedirect_Enabled(t *testing.T) { + t.Setenv(feature.RedirectsPlaceholders.EnvVariable, "true") + t.Setenv(feature.DomainRedirects.EnvVariable, "true") + + tests := map[string]struct { + url string + status int + expectedErr error + }{ + "valid_url": { + url: "/goto.html", + status: http.StatusOK, + }, + "domain_level_redirects": { + url: "https://GitLab.com", + status: http.StatusMovedPermanently, + }, + "domain_level_rewrite": { + url: "https://GitLab.com", + status: http.StatusOK, + expectedErr: errNoDomainLevelRewrite, + }, + "no_special_characters_escape_domain_level_redirects": { + url: "/\\GitLab.com", + status: http.StatusMovedPermanently, + expectedErr: errNoValidStartingInURLPath, + }, + "no_schemaless_url_domain_level_redirects": { + url: "//GitLab.com/pages.html", + status: http.StatusMovedPermanently, + expectedErr: errNoValidStartingInURLPath, + }, + "no_bare_domain_level_redirects": { + url: "GitLab.com", + status: http.StatusMovedPermanently, + expectedErr: errNoValidStartingInURLPath, + }, + "no_parent_traversing_relative_url": { + url: "../target.html", + status: http.StatusMovedPermanently, + expectedErr: errNoValidStartingInURLPath, + }, + "too_many_slashes": { + url: strings.Repeat("/a", 26), + status: http.StatusMovedPermanently, + expectedErr: fmt.Errorf("url path cannot contain more than %d forward slashes", defaultMaxPathSegments), + }, + "placeholders": { + url: "/news/:year/:month/:date/:slug", + status: http.StatusMovedPermanently, + }, + "domain_redirect_placeholders": { + url: "https://GitLab.com/news/:year/:month/:date/:slug", + status: http.StatusMovedPermanently, + }, + "splats": { + url: "/blog/*", + status: http.StatusMovedPermanently, + }, + "lone_splats": { + url: "https://GitLab.com/*", + status: http.StatusMovedPermanently, + }, + "domain_redirect_splats": { + url: "https://GitLab.com/blog/*", + status: http.StatusMovedPermanently, + }, + "splat_placeholders": { + url: "/new/path/:splat", + status: http.StatusMovedPermanently, + }, + "domain_redirect_splat_placeholders": { + url: "https://GitLab.com/new/path/:splat", + status: http.StatusMovedPermanently, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + err := validateToURL(tt.url, tt.status) + if tt.expectedErr != nil { + require.EqualError(t, err, tt.expectedErr.Error()) + return + } + + require.NoError(t, err) + }) + } +} + +// Tests validation rules that only apply when the `FF_ENABLE_DOMAIN_REDIRECT` +// feature flag is not enabled. These tests can be removed when the +// `FF_ENABLE_DOMAIN_REDIRECT` flag is removed. +func TestRedirectsValidateToUrl_DomainRedirect_Disabled(t *testing.T) { + t.Setenv(feature.RedirectsPlaceholders.EnvVariable, "true") + t.Setenv(feature.DomainRedirects.EnvVariable, "true") + + tests := map[string]struct { + url string + status int + expectedErr error + }{ + "valid_url": { + url: "/goto.html", + status: http.StatusOK, + }, + "domain_level_redirects": { + url: "https://GitLab.com", + status: http.StatusMovedPermanently, + }, + "domain_level_rewrite": { + url: "https://GitLab.com", + status: http.StatusOK, + expectedErr: errNoDomainLevelRewrite, + }, + "no_special_characters_escape_domain_level_redirects": { + url: "/\\GitLab.com", + status: http.StatusMovedPermanently, + expectedErr: errNoValidStartingInURLPath, + }, + "no_schemaless_url_domain_level_redirects": { + url: "//GitLab.com/pages.html", + status: http.StatusMovedPermanently, + expectedErr: errNoValidStartingInURLPath, + }, + "no_bare_domain_level_redirects": { + url: "GitLab.com", + status: http.StatusMovedPermanently, + expectedErr: errNoValidStartingInURLPath, + }, + "no_parent_traversing_relative_url": { + url: "../target.html", + status: http.StatusMovedPermanently, + expectedErr: errNoValidStartingInURLPath, + }, + "too_many_slashes": { + url: strings.Repeat("/a", 26), + status: http.StatusMovedPermanently, + expectedErr: fmt.Errorf("url path cannot contain more than %d forward slashes", defaultMaxPathSegments), + }, + "placeholders": { + url: "/news/:year/:month/:date/:slug", + status: http.StatusMovedPermanently, + }, + "domain_redirect_placeholders": { + url: "https://GitLab.com/news/:year/:month/:date/:slug", + status: http.StatusMovedPermanently, + }, + "splats": { + url: "/blog/*", + status: http.StatusMovedPermanently, + }, + "lone_splats": { + url: "https://GitLab.com/*", + status: http.StatusMovedPermanently, + }, + "domain_redirect_splats": { + url: "https://GitLab.com/blog/*", + status: http.StatusMovedPermanently, + }, + "splat_placeholders": { + url: "/new/path/:splat", + status: http.StatusMovedPermanently, + }, + "domain_redirect_splat_placeholders": { + url: "https://GitLab.com/new/path/:splat", + status: http.StatusMovedPermanently, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + err := validateToURL(tt.url, tt.status) + if tt.expectedErr != nil { + require.EqualError(t, err, tt.expectedErr.Error()) + return + } + + require.NoError(t, err) + }) + } +} diff --git a/internal/serving/disk/helpers.go b/internal/serving/disk/helpers.go index 96360d49..c919f188 100644 --- a/internal/serving/disk/helpers.go +++ b/internal/serving/disk/helpers.go @@ -5,6 +5,7 @@ import ( "io" "mime" "net/http" + "net/url" "path/filepath" "strconv" "strings" @@ -36,6 +37,15 @@ func endsWithoutHTMLExtension(path string) bool { return !strings.HasSuffix(path, ".html") } +func cloneURL(originalURL *url.URL) *url.URL { + newURL := new(url.URL) + + // Copy relevant fields + *newURL = *originalURL + + return newURL +} + // Detect file's content-type either by extension or mime-sniffing. // Implementation is adapted from Golang's `http.serveContent()` // See https://github.com/golang/go/blob/902fc114272978a40d2e65c2510a18e870077559/src/net/http/fs.go#L194 diff --git a/internal/serving/disk/reader.go b/internal/serving/disk/reader.go index a483a8fe..a7d1bb29 100644 --- a/internal/serving/disk/reader.go +++ b/internal/serving/disk/reader.go @@ -49,7 +49,10 @@ func (reader *Reader) tryRedirects(h serving.Handler) bool { r := redirects.ParseRedirects(ctx, root) - rewrittenURL, status, err := r.Rewrite(h.Request.URL) + requestURL := cloneURL(h.Request.URL) + // Taking value from h.Request.Host as h.Request.URL.Host is not populated + requestURL.Host = h.Request.Host + rewrittenURL, status, err := r.Rewrite(requestURL) if err != nil { if !errors.Is(err, redirects.ErrNoRedirect) { // We assume that rewrite failure is not fatal @@ -65,7 +68,7 @@ func (reader *Reader) tryRedirects(h serving.Handler) bool { return reader.tryFile(h) } - http.Redirect(h.Writer, h.Request, rewrittenURL.Path, status) + http.Redirect(h.Writer, h.Request, rewrittenURL.String(), status) return true } diff --git a/shared/pages/group.redirects/project-redirects/public.zip b/shared/pages/group.redirects/project-redirects/public.zip Binary files differindex 88c2cbb0..70d9499b 100644 --- a/shared/pages/group.redirects/project-redirects/public.zip +++ b/shared/pages/group.redirects/project-redirects/public.zip diff --git a/shared/pages/group.redirects/project-redirects/public/_redirects b/shared/pages/group.redirects/project-redirects/public/_redirects index 0adee383..63996c07 100644 --- a/shared/pages/group.redirects/project-redirects/public/_redirects +++ b/shared/pages/group.redirects/project-redirects/public/_redirects @@ -6,6 +6,7 @@ /project-redirects/news/:year/:month/:date/:slug /project-redirects/blog/:year/:month/:date/:slug /project-redirects/pit.html /project-redirects/spikes.html 302 /project-redirects/goto-domain.html https://GitLab.com/pages.html 302 +/project-redirects/goto-no-schema-domain.html GitLab.com/pages.html 302 /project-redirects/goto-bare-domain.html GitLab.com/pages.html 302 /project-redirects/goto-schemaless.html //GitLab.com/pages.html 302 /project-redirects/cake-portal/ /project-redirects/still-alive/ 302 diff --git a/test/acceptance/redirects_test.go b/test/acceptance/redirects_test.go index a2bdde53..5c7f765a 100644 --- a/test/acceptance/redirects_test.go +++ b/test/acceptance/redirects_test.go @@ -14,6 +14,7 @@ import ( func TestRedirectStatusPage(t *testing.T) { t.Setenv(feature.RedirectsPlaceholders.EnvVariable, "true") + t.Setenv(feature.DomainRedirects.EnvVariable, "true") RunPagesProcess(t, withListeners([]ListenSpec{httpListener}), @@ -26,11 +27,18 @@ func TestRedirectStatusPage(t *testing.T) { require.NoError(t, err) testhelpers.Close(t, rsp.Body) - require.Contains(t, string(body), "14 rules") + require.Contains(t, string(body), "15 rules") require.Equal(t, http.StatusOK, rsp.StatusCode) } -func TestRedirect(t *testing.T) { +type rewrite struct { + host string + path string + expectedStatus int + expectedLocation string +} + +func testRedirect(t *testing.T, tests []rewrite) { t.Setenv(feature.RedirectsPlaceholders.EnvVariable, "true") RunPagesProcess(t, @@ -44,12 +52,20 @@ func TestRedirect(t *testing.T) { require.Equal(t, http.StatusOK, rsp.StatusCode) - tests := []struct { - host string - path string - expectedStatus int - expectedLocation string - }{ + for _, tt := range tests { + t.Run(fmt.Sprintf("%s%s -> %s (%d)", tt.host, tt.path, tt.expectedLocation, tt.expectedStatus), func(t *testing.T) { + rsp, err := GetRedirectPage(t, httpListener, tt.host, tt.path) + require.NoError(t, err) + testhelpers.Close(t, rsp.Body) + + require.Equal(t, tt.expectedLocation, rsp.Header.Get("Location")) + require.Equal(t, tt.expectedStatus, rsp.StatusCode) + }) + } +} + +func TestRedirect(t *testing.T) { + tests := []rewrite{ // Project domain { host: "group.redirects.gitlab-example.com", @@ -60,7 +76,7 @@ func TestRedirect(t *testing.T) { // Make sure invalid rule does not redirect { host: "group.redirects.gitlab-example.com", - path: "/project-redirects/goto-domain.html", + path: "/project-redirects/goto-no-schema-domain.html", expectedStatus: http.StatusNotFound, expectedLocation: "", }, @@ -92,16 +108,70 @@ func TestRedirect(t *testing.T) { expectedStatus: http.StatusMovedPermanently, expectedLocation: "/project-redirects/careers/assistant-to-the-regional-manager.html", }, + // Domain redirect + { + host: "group.redirects.gitlab-example.com", + path: "/project-redirects/goto-domain.html", + expectedStatus: http.StatusNotFound, + expectedLocation: "", + }, } + testRedirect(t, tests) +} - for _, tt := range tests { - t.Run(fmt.Sprintf("%s%s -> %s (%d)", tt.host, tt.path, tt.expectedLocation, tt.expectedStatus), func(t *testing.T) { - rsp, err := GetRedirectPage(t, httpListener, tt.host, tt.path) - require.NoError(t, err) - testhelpers.Close(t, rsp.Body) +func TestRedirect_DomainRedirects_Enabled(t *testing.T) { + t.Setenv(feature.DomainRedirects.EnvVariable, "true") - require.Equal(t, tt.expectedLocation, rsp.Header.Get("Location")) - require.Equal(t, tt.expectedStatus, rsp.StatusCode) - }) + tests := []rewrite{ + // Project domain + { + host: "group.redirects.gitlab-example.com", + path: "/project-redirects/redirect-portal.html", + expectedStatus: http.StatusFound, + expectedLocation: "/project-redirects/magic-land.html", + }, + // Make sure invalid rule does not redirect + { + host: "group.redirects.gitlab-example.com", + path: "/project-redirects/goto-no-schema-domain.html", + expectedStatus: http.StatusNotFound, + expectedLocation: "", + }, + // Actual file on disk should override any redirects that match + { + host: "group.redirects.gitlab-example.com", + path: "/project-redirects/file-override.html", + expectedStatus: http.StatusOK, + expectedLocation: "", + }, + // Group-level domain + { + host: "group.redirects.gitlab-example.com", + path: "/redirect-portal.html", + expectedStatus: http.StatusFound, + expectedLocation: "/magic-land.html", + }, + // Custom domain + { + host: "redirects.custom-domain.com", + path: "/redirect-portal.html", + expectedStatus: http.StatusFound, + expectedLocation: "/magic-land.html", + }, + // Permanent redirect for splat (*) with replacement (:splat) + { + host: "group.redirects.gitlab-example.com", + path: "/project-redirects/jobs/assistant-to-the-regional-manager.html", + expectedStatus: http.StatusMovedPermanently, + expectedLocation: "/project-redirects/careers/assistant-to-the-regional-manager.html", + }, + // Domain redirect + { + host: "group.redirects.gitlab-example.com", + path: "/project-redirects/goto-domain.html", + expectedStatus: http.StatusFound, + expectedLocation: "https://GitLab.com/pages.html", + }, } + testRedirect(t, tests) } |