diff options
author | Nejc Habjan <nejc.habjan@siemens.com> | 2022-06-22 20:27:04 +0300 |
---|---|---|
committer | Nejc Habjan <nejc.habjan@siemens.com> | 2022-06-27 11:36:53 +0300 |
commit | 89dfffbca7d5a21f002354909edb394b5001604a (patch) | |
tree | 22936f55ede3e652b884403656acc8417d2b31af /internal/redirects | |
parent | 770aba26fddb728107d278e906a5c1669671db0c (diff) |
Apply maintainer review suggestions
Diffstat (limited to 'internal/redirects')
-rw-r--r-- | internal/redirects/redirects.go | 12 | ||||
-rw-r--r-- | internal/redirects/redirects_test.go | 6 | ||||
-rw-r--r-- | internal/redirects/validations.go | 3 | ||||
-rw-r--r-- | internal/redirects/validations_test.go | 11 |
4 files changed, 21 insertions, 11 deletions
diff --git a/internal/redirects/redirects.go b/internal/redirects/redirects.go index f92a5806..ed3bc4b1 100644 --- a/internal/redirects/redirects.go +++ b/internal/redirects/redirects.go @@ -23,6 +23,15 @@ const ( // - https://docs.netlify.com/routing/redirects/ // - https://docs.netlify.com/routing/redirects/redirect-options/ ConfigFile = "_redirects" + + // Check https://gitlab.com/gitlab-org/gitlab-pages/-/issues/472 before increasing this value + defaultMaxConfigSize = 64 * 1024 + + // maxPathSegments is used to limit the number of path segments allowed in rules URLs + defaultMaxPathSegments = 25 + + // maxRuleCount is used to limit the total number of rules allowed in _redirects + defaultMaxRuleCount = 1000 ) var ( @@ -43,7 +52,6 @@ var ( errNoParams = errors.New("params not supported") errUnsupportedStatus = errors.New("status not supported") errNoForce = errors.New("force! not supported") - errTooManyPathSegments = errors.New("url path contains more forward slashes than the configured maximum") regexpPlaceholder = regexp.MustCompile(`(?i)/:[a-z]+`) ) @@ -66,7 +74,7 @@ func (r *Redirects) Status() string { messages = append(messages, fmt.Sprintf("%d rules", len(r.rules))) for i, rule := range r.rules { - if i >= cfg.MaxConfigSize { + if i >= cfg.MaxRuleCount { messages = append([]string{ fmt.Sprintf( "The _redirects file contains (%d) rules, more than the maximum of %d rules. Only the first %d rules will be processed.", diff --git a/internal/redirects/redirects_test.go b/internal/redirects/redirects_test.go index 210e8fe0..a9891b90 100644 --- a/internal/redirects/redirects_test.go +++ b/internal/redirects/redirects_test.go @@ -17,12 +17,6 @@ import ( "gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers" ) -const ( - defaultMaxConfigSize = 64 * 1024 - defaultMaxPathSegments = 25 - defaultMaxRuleCount = 1000 -) - func TestRedirectsRewrite(t *testing.T) { t.Setenv(feature.RedirectsPlaceholders.EnvVariable, "true") setupRedirectsConfig() diff --git a/internal/redirects/validations.go b/internal/redirects/validations.go index c37b79ec..86fb0212 100644 --- a/internal/redirects/validations.go +++ b/internal/redirects/validations.go @@ -1,6 +1,7 @@ package redirects import ( + "fmt" "net/http" "net/url" "regexp" @@ -44,7 +45,7 @@ func validateURL(urlText string) error { // This prevents the matching logic from generating regular // expressions that are too large/complex. if strings.Count(url.Path, "/") > cfg.MaxPathSegments { - return errTooManyPathSegments + 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 diff --git a/internal/redirects/validations_test.go b/internal/redirects/validations_test.go index 296be511..906845a1 100644 --- a/internal/redirects/validations_test.go +++ b/internal/redirects/validations_test.go @@ -1,6 +1,7 @@ package redirects import ( + "fmt" "strings" "testing" @@ -42,7 +43,7 @@ func TestRedirectsValidateUrl(t *testing.T) { }, "too_many_slashes": { url: strings.Repeat("/a", 26), - expectedErr: errTooManyPathSegments, + expectedErr: fmt.Errorf("url path cannot contain more than %d forward slashes", defaultMaxPathSegments), }, "placeholders": { url: "/news/:year/:month/:date/:slug", @@ -58,7 +59,13 @@ func TestRedirectsValidateUrl(t *testing.T) { for name, tt := range tests { t.Run(name, func(t *testing.T) { err := validateURL(tt.url) - require.ErrorIs(t, err, tt.expectedErr) + if tt.expectedErr != nil { + require.Error(t, err) + require.Equal(t, err, tt.expectedErr) + return + } + + require.NoError(t, err) }) } } |