chore: process the first 1000 rules only

7 files changed, 69 insertions, 46 deletions

diff --git a/internal/redirects/matching.go b/internal/redirects/matching.go
index fb879e4f..b5bceb71 100644
--- a/internal/redirects/matching.go
+++ b/internal/redirects/matching.go
@@ -100,11 +100,12 @@ 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) {
-	if validateRedirectsFile(r) != nil {
-		return nil, ""
-	}
-
 	for i := range r.rules {
+		if i >= maxRuleCount {
+			// do not process any more rules
+			return nil, ""
+		}
+
 		// assign rule to a new var to prevent the following gosec error
 		// G601: Implicit memory aliasing in for loop
 		rule := r.rules[i]
diff --git a/internal/redirects/redirects.go b/internal/redirects/redirects.go
index fafeaf4b..24ce8692 100644
--- a/internal/redirects/redirects.go
+++ b/internal/redirects/redirects.go
@@ -55,7 +55,6 @@ var (
 	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)
-	errTooManyRules                    = fmt.Errorf("_redirects file may not contain more than %d rules", maxRuleCount)
 	regexpPlaceholder                  = regexp.MustCompile(`(?i)/:[a-z]+`)
 )
 
@@ -73,11 +72,21 @@ func (r *Redirects) Status() string {
 	messages := make([]string, 0, len(r.rules)+1)
 	messages = append(messages, fmt.Sprintf("%d rules", len(r.rules)))
 
-	if err := validateRedirectsFile(r); err != nil {
-		messages = append(messages, fmt.Sprintf("error: %s", err.Error()))
-	}
-
 	for i, rule := range r.rules {
+		if i >= 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,
+				)},
+				messages...,
+			)
+
+			break
+		}
+
 		if err := validateRule(rule); err != nil {
 			messages = append(messages, fmt.Sprintf("rule %d: error: %s", i+1, err.Error()))
 		} else {
diff --git a/internal/redirects/redirects_benchmark_test.go b/internal/redirects/redirects_benchmark_test.go
index 38adcb68..87452172 100644
--- a/internal/redirects/redirects_benchmark_test.go
+++ b/internal/redirects/redirects_benchmark_test.go
@@ -30,8 +30,7 @@ func generateRedirectsFile(dirPath string, count int) error {
 func benchmarkRedirectsRewrite(b *testing.B, redirectsCount int) {
 	ctx := context.Background()
 
-	root, tmpDir, cleanup := testhelpers.TmpDir(nil, "ParseRedirects_benchmarks")
-	defer cleanup()
+	root, tmpDir := testhelpers.TmpDir(nil, "ParseRedirects_benchmarks")
 
 	err := generateRedirectsFile(tmpDir, redirectsCount)
 	require.NoError(b, err)
@@ -65,8 +64,7 @@ func BenchmarkRedirectsRewrite_PlaceholdersEnabled(b *testing.B) {
 func benchmarkRedirectsParseRedirects(b *testing.B, redirectsCount int) {
 	ctx := context.Background()
 
-	root, tmpDir, cleanup := testhelpers.TmpDir(nil, "ParseRedirects_benchmarks")
-	defer cleanup()
+	root, tmpDir := testhelpers.TmpDir(nil, "ParseRedirects_benchmarks")
 
 	err := generateRedirectsFile(tmpDir, redirectsCount)
 	require.NoError(b, err)
diff --git a/internal/redirects/redirects_test.go b/internal/redirects/redirects_test.go
index 6ae141f5..2fe0034a 100644
--- a/internal/redirects/redirects_test.go
+++ b/internal/redirects/redirects_test.go
@@ -160,8 +160,7 @@ func TestRedirectsRewrite(t *testing.T) {
 func TestRedirectsParseRedirects(t *testing.T) {
 	ctx := context.Background()
 
-	root, tmpDir, cleanup := testhelpers.TmpDir(t, "ParseRedirects_tests")
-	defer cleanup()
+	root, tmpDir := testhelpers.TmpDir(t, "ParseRedirects_tests")
 
 	tests := []struct {
 		name          string
@@ -222,3 +221,36 @@ func TestRedirectsParseRedirects(t *testing.T) {
 		})
 	}
 }
+
+func TestMaxRuleCount(t *testing.T) {
+	root, tmpDir := testhelpers.TmpDir(t, "TooManyRules_tests")
+
+	err := ioutil.WriteFile(path.Join(tmpDir, ConfigFile), []byte(strings.Repeat("/goto.html /target.html 301\n", maxRuleCount-1)+
+		"/1000.html /target1000 301\n"+
+		"/1001.html /target1001 301\n",
+	), 0600)
+	require.NoError(t, err)
+
+	redirects := ParseRedirects(context.Background(), root)
+
+	testFn := func(path, expectedToURL string, expectedStatus int, expectedErr string) func(t *testing.T) {
+		return func(t *testing.T) {
+			originalURL, err := url.Parse(path)
+			require.NoError(t, err)
+
+			toURL, status, err := redirects.Rewrite(originalURL)
+			if expectedErr != "" {
+				require.EqualError(t, err, expectedErr)
+				return
+			}
+
+			require.NoError(t, err)
+
+			require.Equal(t, expectedToURL, toURL.String())
+			require.Equal(t, expectedStatus, status)
+		}
+	}
+
+	t.Run("maxRuleCount matches", testFn("/1000.html", "/target1000", 301, ""))
+	t.Run("maxRuleCount+1 does not match", testFn("/1001.html", "", 0, ErrNoRedirect.Error()))
+}
diff --git a/internal/redirects/validations.go b/internal/redirects/validations.go
index 7c6d8089..1af5c5c5 100644
--- a/internal/redirects/validations.go
+++ b/internal/redirects/validations.go
@@ -89,15 +89,3 @@ func validateRule(r netlifyRedirects.Rule) error {
 
 	return nil
 }
-
-// validateRedirectsFile runs rules on the _redirects file as a whole.
-// Returns `nil` if no errors are found.
-// Does not run rule-specific validations - this is
-// handled by `validateRule` instead.
-func validateRedirectsFile(r *Redirects) error {
-	if len(r.rules) > maxRuleCount {
-		return errTooManyRules
-	}
-
-	return nil
-}
diff --git a/internal/serving/disk/symlink/path_test.go b/internal/serving/disk/symlink/path_test.go
index 66b4fceb..c9da51c3 100644
--- a/internal/serving/disk/symlink/path_test.go
+++ b/internal/serving/disk/symlink/path_test.go
@@ -85,8 +85,7 @@ func testEvalSymlinks(t *testing.T, wd, path, want string) {
 }
 
 func TestEvalSymlinks(t *testing.T) {
-	_, tmpDir, cleanup := testhelpers.TmpDir(t, "symlink_tests")
-	defer cleanup()
+	_, tmpDir := testhelpers.TmpDir(t, "symlink_tests")
 
 	// Create the symlink farm using relative paths.
 	for _, d := range EvalSymlinksTestDirs {
@@ -124,8 +123,7 @@ func TestEvalSymlinks(t *testing.T) {
 }
 
 func TestEvalSymlinksIsNotExist(t *testing.T) {
-	root, _, cleanup := testhelpers.TmpDir(t, "symlink_tests")
-	defer cleanup()
+	root, _ := testhelpers.TmpDir(t, "symlink_tests")
 
 	_, err := symlink.EvalSymlinks(context.Background(), root, "notexist")
 	if !os.IsNotExist(err) {
@@ -145,8 +143,7 @@ func TestEvalSymlinksIsNotExist(t *testing.T) {
 }
 
 func TestIssue13582(t *testing.T) {
-	root, tmpDir, cleanup := testhelpers.TmpDir(t, "symlink_tests")
-	defer cleanup()
+	root, tmpDir := testhelpers.TmpDir(t, "symlink_tests")
 
 	dir := filepath.Join(tmpDir, "dir")
 	err := os.Mkdir(dir, 0755)
diff --git a/internal/testhelpers/tmpdir.go b/internal/testhelpers/tmpdir.go
index 81f0781b..5765d308 100644
--- a/internal/testhelpers/tmpdir.go
+++ b/internal/testhelpers/tmpdir.go
@@ -15,24 +15,22 @@ import (
 
 var fs = vfs.Instrumented(&local.VFS{})
 
-func TmpDir(t *testing.T, pattern string) (vfs.Root, string, func()) {
+func TmpDir(tb testing.TB, pattern string) (vfs.Root, string) {
+	tb.Helper()
+
 	tmpDir, err := ioutil.TempDir("", pattern)
-	if t != nil {
-		require.NoError(t, err)
-	}
+	require.NoError(tb, err)
 
 	// On some systems `/tmp` can be a symlink
 	tmpDir, err = filepath.EvalSymlinks(tmpDir)
-	if t != nil {
-		require.NoError(t, err)
-	}
+	require.NoError(tb, err)
 
 	root, err := fs.Root(context.Background(), tmpDir)
-	if t != nil {
-		require.NoError(t, err)
-	}
+	require.NoError(tb, err)
+
+	tb.Cleanup(func() {
+		require.NoError(tb, os.RemoveAll(tmpDir))
+	})
 
-	return root, tmpDir, func() {
-		os.RemoveAll(tmpDir)
-	}
+	return root, tmpDir
 }