Apply maintainer review suggestions

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)
 		})
 	}
 }