diff options
author | Jaime Martinez <jmartinez@gitlab.com> | 2022-06-28 03:25:42 +0300 |
---|---|---|
committer | Jaime Martinez <jmartinez@gitlab.com> | 2022-06-28 03:25:42 +0300 |
commit | 1737c75532848a74018fe62f817ceab074271bdd (patch) | |
tree | 93c266b0c7f33a14e6783e39201ac02e51befcdc /internal | |
parent | cfa77848a58424c0048c3e52f48490cfd783176f (diff) | |
parent | 520fa6df5119d8fb2ff7ec5ac7da521a3e102820 (diff) |
Merge branch 'feat/configurable-redirects' into 'master'
feat: make _redirects limits configurable
See merge request gitlab-org/gitlab-pages!778
Diffstat (limited to 'internal')
-rw-r--r-- | internal/config/config.go | 16 | ||||
-rw-r--r-- | internal/config/flags.go | 5 | ||||
-rw-r--r-- | internal/redirects/matching.go | 2 | ||||
-rw-r--r-- | internal/redirects/redirects.go | 26 | ||||
-rw-r--r-- | internal/redirects/redirects_test.go | 4 | ||||
-rw-r--r-- | internal/redirects/validations.go | 5 | ||||
-rw-r--r-- | internal/redirects/validations_test.go | 10 |
7 files changed, 53 insertions, 15 deletions
diff --git a/internal/config/config.go b/internal/config/config.go index 18fe33eb..fcaa1d5e 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 88ebbb15..9d79f0bc 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 a75255bb..25e2ce2e 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 f3c4e491..4add6537 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 66c5d733..d40b065b 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 ed022f52..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" @@ -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 296be511..0d6df3ec 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) }) } } |