diff --git a/app.go b/app.go index c08dfb2974aa444a83e1bb95935ebeaf6e3423b6..7a22a3fd5e75f20adc4c1f1f5e06aa71b769f513 100644 --- a/app.go +++ b/app.go @@ -32,6 +32,7 @@ import ( "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" "gitlab.com/gitlab-org/gitlab-pages/internal/logging" "gitlab.com/gitlab-org/gitlab-pages/internal/netutil" + "gitlab.com/gitlab-org/gitlab-pages/internal/redirects" "gitlab.com/gitlab-org/gitlab-pages/internal/rejectmethods" "gitlab.com/gitlab-org/gitlab-pages/internal/request" "gitlab.com/gitlab-org/gitlab-pages/internal/routing" @@ -349,6 +350,8 @@ func (a *theApp) listenMetrics(eg *errgroup.Group, config cfg.Metrics) *http.Ser } func runApp(config *cfg.Config) error { + redirects.SetConfig(config.Redirects) + source, err := gitlab.New(&config.GitLab) if err != nil { return fmt.Errorf("could not create domains config source: %w", err) diff --git a/internal/config/config.go b/internal/config/config.go index 18fe33ebfd5e6ac9ffabcc4b52f09ddc4b7c0d6a..fcaa1d5e79523ce6bd9fd3540dce58421e53206a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -21,6 +21,7 @@ type Config struct { Authentication Auth GitLab GitLab Log Log + Redirects Redirects Sentry Sentry Server Server TLS TLS @@ -119,6 +120,13 @@ type Log struct { Verbose bool } +// Redirects groups settings related to configuring _redirects limits +type Redirects struct { + MaxConfigSize int + MaxPathSegments int + MaxRuleCount int +} + // Sentry groups settings related to configuring Sentry type Sentry struct { DSN string @@ -287,6 +295,11 @@ func loadConfig() (*Config, error) { Format: *logFormat, Verbose: *logVerbose, }, + Redirects: Redirects{ + MaxConfigSize: *redirectsMaxConfigSize, + MaxPathSegments: *redirectsMaxPathSegments, + MaxRuleCount: *redirectsMaxRuleCount, + }, Sentry: Sentry{ DSN: *sentryDSN, Environment: *sentryEnvironment, @@ -401,6 +414,9 @@ func LogConfig(config *Config) { "rate-limit-tls-source-ip-burst": config.RateLimit.TLSSourceIPBurst, "rate-limit-tls-domain": config.RateLimit.TLSDomainLimitPerSecond, "rate-limit-tls-domain-burst": config.RateLimit.TLSDomainBurst, + "redirects-max-config-size": config.Redirects.MaxConfigSize, + "redirects-max-path-segments": config.Redirects.MaxPathSegments, + "redirects-max-rule-count": config.Redirects.MaxRuleCount, "server-read-timeout": config.Server.ReadTimeout, "server-read-header-timeout": config.Server.ReadHeaderTimeout, "server-write-timeout": config.Server.WriteTimeout, diff --git a/internal/config/flags.go b/internal/config/flags.go index 88ebbb155619aa9d070fb5c8f36a83524374fc46..9d79f0bcd5a50e6734c557d42835f689ef12bc68 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -61,6 +61,11 @@ var ( gitlabRetrievalInterval = flag.Duration("gitlab-retrieval-interval", time.Second, "The interval to wait before retrying to resolve a domain's configuration via the GitLab API") gitlabRetrievalRetries = flag.Int("gitlab-retrieval-retries", 3, "The maximum number of times to retry to resolve a domain's configuration via the API") + // Check https://gitlab.com/gitlab-org/gitlab-pages/-/issues/472 before increasing default redirectsMaxConfigSize value + redirectsMaxConfigSize = flag.Int("redirects-max-config-size", 64*1024, "The maximum size of the _redirects file, in bytes") + redirectsMaxPathSegments = flag.Int("redirects-max-path-segments", 25, "The maximum number of path segments allowed in _redirects rules URLs") + redirectsMaxRuleCount = flag.Int("redirects-max-rule-count", 1000, "The maximum number of rules allowed in _redirects") + enableDisk = flag.Bool("enable-disk", true, "Enable disk access, shall be disabled in environments where shared disk storage isn't available") clientID = flag.String("auth-client-id", "", "GitLab application Client ID") diff --git a/internal/redirects/matching.go b/internal/redirects/matching.go index a75255bbef104c047697b7e236dd35689febea10..25e2ce2e33ebc78b5f1506eda34b616c581ac369 100644 --- a/internal/redirects/matching.go +++ b/internal/redirects/matching.go @@ -103,7 +103,7 @@ func matchesRule(rule *netlifyRedirects.Rule, path string) (bool, string) { // If no rule matches, this function returns `nil` and an empty string func (r *Redirects) match(path string) (*netlifyRedirects.Rule, string) { for i := range r.rules { - if i >= maxRuleCount { + if i >= cfg.MaxRuleCount { // do not process any more rules return nil, "" } diff --git a/internal/redirects/redirects.go b/internal/redirects/redirects.go index f3c4e491a7d67a4ae482452c67776cdfd5414280..4add6537ddd4b9e7258d518ccd3e75bf9ca4dea7 100644 --- a/internal/redirects/redirects.go +++ b/internal/redirects/redirects.go @@ -13,6 +13,7 @@ import ( netlifyRedirects "github.com/tj/go-redirects" "gitlab.com/gitlab-org/labkit/log" + "gitlab.com/gitlab-org/gitlab-pages/internal/config" "gitlab.com/gitlab-org/gitlab-pages/internal/vfs" ) @@ -24,16 +25,22 @@ const ( ConfigFile = "_redirects" // Check https://gitlab.com/gitlab-org/gitlab-pages/-/issues/472 before increasing this value - maxConfigSize = 64 * 1024 + defaultMaxConfigSize = 64 * 1024 // maxPathSegments is used to limit the number of path segments allowed in rules URLs - maxPathSegments = 25 + defaultMaxPathSegments = 25 // maxRuleCount is used to limit the total number of rules allowed in _redirects - maxRuleCount = 1000 + defaultMaxRuleCount = 1000 ) var ( + cfg = config.Redirects{ + MaxConfigSize: defaultMaxConfigSize, + MaxPathSegments: defaultMaxPathSegments, + MaxRuleCount: defaultMaxRuleCount, + } + // ErrNoRedirect is the error thrown when a no redirect rule matches while trying to Rewrite URL. // This means that no redirect applies to the URL and you can fallback to serving actual content instead. ErrNoRedirect = errors.New("no redirect found") @@ -50,10 +57,13 @@ var ( errNoParams = errors.New("params not supported") errUnsupportedStatus = errors.New("status not supported") errNoForce = errors.New("force! not supported") - errTooManyPathSegments = fmt.Errorf("url path cannot contain more than %d forward slashes", maxPathSegments) regexpPlaceholder = regexp.MustCompile(`(?i)/:[a-z]+`) ) +func SetConfig(redirectsConfig config.Redirects) { + cfg = redirectsConfig +} + type Redirects struct { rules []netlifyRedirects.Rule error error @@ -69,13 +79,13 @@ func (r *Redirects) Status() string { messages = append(messages, fmt.Sprintf("%d rules", len(r.rules))) for i, rule := range r.rules { - if i >= maxRuleCount { + 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.", len(r.rules), - maxRuleCount, - maxRuleCount, + cfg.MaxRuleCount, + cfg.MaxRuleCount, )}, messages..., ) @@ -126,7 +136,7 @@ func ParseRedirects(ctx context.Context, root vfs.Root) *Redirects { return &Redirects{error: errNeedRegularFile} } - if fi.Size() > maxConfigSize { + if fi.Size() > int64(cfg.MaxConfigSize) { return &Redirects{error: errFileTooLarge} } diff --git a/internal/redirects/redirects_test.go b/internal/redirects/redirects_test.go index 66c5d73391d02ed75725ec97c3ba14a683600050..d40b065b8c0d0fb2fd131d78c0eac6d7d818cb80 100644 --- a/internal/redirects/redirects_test.go +++ b/internal/redirects/redirects_test.go @@ -165,7 +165,7 @@ func TestRedirectsParseRedirects(t *testing.T) { }, { name: "Config file too big", - redirectsFile: strings.Repeat("a", 2*maxConfigSize), + redirectsFile: strings.Repeat("a", 2*cfg.MaxConfigSize), expectedRules: 0, expectedErr: errFileTooLarge, }, @@ -197,7 +197,7 @@ func TestRedirectsParseRedirects(t *testing.T) { func TestMaxRuleCount(t *testing.T) { root, tmpDir := testhelpers.TmpDir(t) - err := os.WriteFile(path.Join(tmpDir, ConfigFile), []byte(strings.Repeat("/goto.html /target.html 301\n", maxRuleCount-1)+ + err := os.WriteFile(path.Join(tmpDir, ConfigFile), []byte(strings.Repeat("/goto.html /target.html 301\n", cfg.MaxRuleCount-1)+ "/1000.html /target1000 301\n"+ "/1001.html /target1001 301\n", ), 0600) diff --git a/internal/redirects/validations.go b/internal/redirects/validations.go index ed022f527bfee9357444aa6951bade1209fd2f0e..86fb02128b7560c07770d26cff3a3e373a134c36 100644 --- a/internal/redirects/validations.go +++ b/internal/redirects/validations.go @@ -1,6 +1,7 @@ package redirects import ( + "fmt" "net/http" "net/url" "regexp" @@ -43,8 +44,8 @@ func validateURL(urlText string) error { // 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, "/") > maxPathSegments { - return errTooManyPathSegments + if strings.Count(url.Path, "/") > cfg.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 diff --git a/internal/redirects/validations_test.go b/internal/redirects/validations_test.go index 296be5118c55a17c6731b331de1eeb1a0b1d0d70..0d6df3ecf97fdce723d00044b930847384477f1c 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,12 @@ 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.EqualError(t, err, tt.expectedErr.Error()) + return + } + + require.NoError(t, err) }) } }