diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 3bb3f72..6c02094 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -7,7 +7,7 @@ jobs: fail-fast: false matrix: os: [ "ubuntu" ] - go: [ "1.17.x", "1.18.x" ] + go: [ "1.18.x", "1.19.x" ] env: COVERAGES: "" runs-on: ${{ matrix.os }}-latest @@ -25,3 +25,5 @@ jobs: run: go test -v ./... - name: Check formatted run: gofmt -l . + - name: Fuzz + run: go test ./... -fuzz=Fuzz -fuzztime=1m diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..ace1063 --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +/testdata diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..a755b9e --- /dev/null +++ b/Makefile @@ -0,0 +1,19 @@ +FUZZTIME ?= 1m + +.PHONY: test + +default: all + +all: build test + +clean: + go clean ./... + +build: + go build ./... + +test: + go test ./... + +fuzz: + go test ./... -fuzz=Fuzz -fuzztime $(FUZZTIME) \ No newline at end of file diff --git a/README.md b/README.md index 1105904..b63aa79 100644 --- a/README.md +++ b/README.md @@ -28,19 +28,24 @@ from to [status] # Redirect with a 302 /my-redirect / 302 -# Rewrite a path -/pass-through /index.html 200 +# Redirect with wildcard (splat placeholder) +/splat/* /redirected-splat/:splat 301 + +# Redirect with multiple named placeholder +/posts/:year/:month/:day/:title /articles/:year/:month/:day/:title 301 # Show a custom 404 for everything under this path -/ecommerce/* /store-closed 404 +/ecommerce/* /store-closed.html 404 -# Single page app rewrite +# Single page app rewrite (SPA, PWA) /* /index.html 200 - -# Proxying -/api/* https://api.example.com/:splat 200 ``` +## Notes for contributors + +- `make all` builds and runs tests +- `FUZZTIME=1m make fuzz` runs fuzzing for specified amount of time + --- ## Credit diff --git a/redirects.go b/redirects.go index e152375..e4b9f01 100644 --- a/redirects.go +++ b/redirects.go @@ -126,11 +126,11 @@ func Parse(r io.Reader) (rules []Rule, err error) { // missing dst if len(fields) <= 1 { - return nil, fmt.Errorf("missing destination path: %q", line) + return nil, fmt.Errorf("missing 'to' path: %q", line) } if len(fields) > 3 { - return nil, fmt.Errorf("must match format `from to [status][!]`") + return nil, fmt.Errorf("must match format 'from to [status]'") } // src and dst @@ -140,6 +140,23 @@ func Parse(r io.Reader) (rules []Rule, err error) { Status: 301, } + // from + if !strings.HasPrefix(rule.From, "/") { + return nil, fmt.Errorf("'from' path must begin with '/'") + } + + if strings.Contains(rule.From, "*") && !strings.HasSuffix(rule.From, "*") { + return nil, fmt.Errorf("'from' path can only end with splat") + } + + // to + if !strings.HasPrefix(rule.To, "/") { + _, err := url.Parse(rule.To) + if err != nil { + return nil, errors.Wrapf(err, "invalid 'to' path") + } + } + // status if len(fields) > 2 { code, err := parseStatus(fields[2]) diff --git a/redirects_test.go b/redirects_test.go index aa549fe..d91700f 100644 --- a/redirects_test.go +++ b/redirects_test.go @@ -1,6 +1,7 @@ package redirects import ( + "bufio" "bytes" "strings" "testing" @@ -8,7 +9,7 @@ import ( "github.com/tj/assert" ) -func TestRule_IsProxy(t *testing.T) { +func TestRuleIsProxy(t *testing.T) { t.Run("without host", func(t *testing.T) { r := Rule{ From: "/blog", @@ -28,7 +29,7 @@ func TestRule_IsProxy(t *testing.T) { }) } -func TestRule_IsRewrite(t *testing.T) { +func TestRuleIsRewrite(t *testing.T) { t.Run("with 3xx", func(t *testing.T) { r := Rule{ From: "/blog", @@ -59,7 +60,7 @@ func TestRule_IsRewrite(t *testing.T) { }) } -func Test_Parse(t *testing.T) { +func TestParse(t *testing.T) { t.Run("with illegal force", func(t *testing.T) { _, err := Parse(strings.NewReader(` /home / 301! @@ -97,3 +98,72 @@ func Test_Parse(t *testing.T) { assert.Contains(t, err.Error(), "redirects file size cannot exceed") }) } + +func FuzzParse(f *testing.F) { + testcases := []string{"/a /b 999\n", + "/redirect-one /one.html\n/301-redirect-one /one.html 301\n/302-redirect-two /two.html 302\n/200-index /index.html 200\n/posts/:year/:month/:day/:title /articles/:year/:month/:day/:title 301\n/splat/* /redirected-splat/:splat 301\n/not-found/* /404.html 404\n/* /index.html 200\n", + "/a /b 301!\n", + "/a200 /b200 200\n/a301 /b301 301\n/a302 /b302 302\n/a303 /b303 303\n/a307 /b307 307\n/a308 /b308 308\n/a404 /b404 404\n/a410 /b410 410\n/a451 /b451 451\n", + "hello\n", "/redirect-one /one.html\r\n/200-index /index.html 200\r\n", "a b 2\nc d 42", "/a/*/b blah", "/from https://example.com 200\n/a/:blah/yeah /b/:blah/yeah"} + for _, tc := range testcases { + f.Add(tc) // Use f.Add to provide a seed corpus + } + f.Fuzz(func(t *testing.T, orig string) { + rules, err := ParseString(orig) + if err != nil { + if len(rules) > 0 { + t.Errorf("should not return rules on error") + } + } + + s := bufio.NewScanner(strings.NewReader(orig)) + + for s.Scan() { + line := strings.TrimSpace(s.Text()) + fields := strings.Fields(line) + + // Skip comments so we don't have to special case + if strings.HasPrefix(line, "#") { + continue + } + + if err == nil && len(fields) < 2 && line != "" { + t.Errorf("should error with less than 2 fields. orig='%v'", orig) + continue + } + + if err == nil && len(fields) > 3 { + t.Errorf("should error with more than 3 fields. orig='%v'", orig) + continue + } + + if err == nil && len(fields) > 0 && !strings.HasPrefix(fields[0], "/") { + t.Errorf("should error for from path not starting with '/'. orig=%v", orig) + continue + } + + // we already handled these cases + if len(fields) < 3 { + continue + } + + if err == nil && strings.Contains(fields[0], "*") && !strings.HasSuffix(fields[0], "*") { + t.Errorf("asterisk in from not at end should error. orig=%v", orig) + continue + } + + if err == nil && strings.HasSuffix(fields[2], "!") { + t.Errorf("should error for forced redirects. orig=%v, err=%v", orig, err) + continue + } + + if err == nil { + for _, r := range rules { + if !isValidStatusCode(r.Status) { + t.Errorf("should error for invalid status code. orig=%v", orig) + } + } + } + } + }) +}