diff options
18 files changed, 825 insertions, 26 deletions
diff --git a/acceptance_test.go b/acceptance_test.go index c4383c65..f8c2a546 100644 --- a/acceptance_test.go +++ b/acceptance_test.go @@ -485,6 +485,112 @@ func TestPrometheusMetricsCanBeScraped(t *testing.T) { // require.Contains(t, string(body), `gitlab_pages_httprange_zip_reader_requests_duration{status_code="200"}`) } +func TestDisabledRedirects(t *testing.T) { + skipUnlessEnabled(t) + + teardown := RunPagesProcessWithEnvs(t, true, *pagesBinary, listeners, "", []string{"FF_ENABLE_REDIRECTS=false"}) + defer teardown() + + // Test that redirects status page is forbidden + rsp, err := GetPageFromListener(t, httpListener, "group.redirects.gitlab-example.com", "/project-redirects/_redirects") + require.NoError(t, err) + defer rsp.Body.Close() + + require.Equal(t, http.StatusForbidden, rsp.StatusCode) + + // Test that redirects are disabled + rsp, err = GetRedirectPage(t, httpListener, "group.redirects.gitlab-example.com", "/project-redirects/redirect-portal.html") + require.NoError(t, err) + defer rsp.Body.Close() + + require.Equal(t, http.StatusNotFound, rsp.StatusCode) +} + +func TestRedirectStatusPage(t *testing.T) { + skipUnlessEnabled(t) + + teardown := RunPagesProcessWithEnvs(t, true, *pagesBinary, listeners, "", []string{"FF_ENABLE_REDIRECTS=true"}) + defer teardown() + + rsp, err := GetPageFromListener(t, httpListener, "group.redirects.gitlab-example.com", "/project-redirects/_redirects") + require.NoError(t, err) + + body, err := ioutil.ReadAll(rsp.Body) + require.NoError(t, err) + defer rsp.Body.Close() + + require.Contains(t, string(body), "11 rules") + require.Equal(t, http.StatusOK, rsp.StatusCode) +} + +func TestRedirect(t *testing.T) { + skipUnlessEnabled(t) + + teardown := RunPagesProcessWithEnvs(t, true, *pagesBinary, listeners, "", []string{"FF_ENABLE_REDIRECTS=true"}) + defer teardown() + + // Test that serving a file still works with redirects enabled + rsp, err := GetRedirectPage(t, httpListener, "group.redirects.gitlab-example.com", "/project-redirects/index.html") + require.NoError(t, err) + defer rsp.Body.Close() + + require.Equal(t, http.StatusOK, rsp.StatusCode) + + tests := []struct { + host string + path string + expectedStatus int + expectedLocation string + }{ + // Project domain + { + host: "group.redirects.gitlab-example.com", + path: "/project-redirects/redirect-portal.html", + expectedStatus: http.StatusFound, + expectedLocation: "/project-redirects/magic-land.html", + }, + // Make sure invalid rule does not redirect + { + host: "group.redirects.gitlab-example.com", + path: "/project-redirects/goto-domain.html", + expectedStatus: http.StatusNotFound, + expectedLocation: "", + }, + // Actual file on disk should override any redirects that match + { + host: "group.redirects.gitlab-example.com", + path: "/project-redirects/file-override.html", + expectedStatus: http.StatusOK, + expectedLocation: "", + }, + // Group-level domain + { + host: "group.redirects.gitlab-example.com", + path: "/redirect-portal.html", + expectedStatus: http.StatusFound, + expectedLocation: "/magic-land.html", + }, + // Custom domain + { + host: "redirects.custom-domain.com", + path: "/redirect-portal.html", + expectedStatus: http.StatusFound, + expectedLocation: "/magic-land.html", + }, + } + + for _, tt := range tests { + t.Run(fmt.Sprintf("%s%s -> %s (%d)", tt.host, tt.path, tt.expectedLocation, tt.expectedStatus), func(t *testing.T) { + rsp, err := GetRedirectPage(t, httpListener, tt.host, tt.path) + require.NoError(t, err) + defer rsp.Body.Close() + + require.Equal(t, tt.expectedLocation, rsp.Header.Get("Location")) + require.Equal(t, tt.expectedStatus, rsp.StatusCode) + }) + } +} + func TestStatusPage(t *testing.T) { skipUnlessEnabled(t) teardown := RunPagesProcess(t, *pagesBinary, listeners, "", "-pages-status=/@statuscheck") @@ -21,7 +21,9 @@ require ( github.com/rs/cors v1.7.0 github.com/sirupsen/logrus v1.4.2 github.com/stretchr/objx v0.2.0 // indirect - github.com/stretchr/testify v1.5.1 + github.com/stretchr/testify v1.6.1 + github.com/tj/assert v0.0.3 // indirect + github.com/tj/go-redirects v0.0.0-20180508180010-5c02ead0bbc5 github.com/tomasen/realip v0.0.0-20180522021738-f0c99a92ddce github.com/wadey/gocovmerge v0.0.0-20160331181800-b5bfa59ec0ad gitlab.com/gitlab-org/labkit v0.0.0-20200520155818-96e583c57891 @@ -32,6 +34,5 @@ require ( golang.org/x/sys v0.0.0-20200420163511-1957bb5e6d1f golang.org/x/tools v0.0.0-20200502202811-ed308ab3e770 gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect - gopkg.in/yaml.v2 v2.2.8 // indirect honnef.co/go/tools v0.0.1-2020.1.3 // indirect ) @@ -302,9 +302,13 @@ github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= -github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4= -github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= +github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= +github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/tinylib/msgp v1.0.2/go.mod h1:+d+yLhGm8mzTaHzB+wgMYrodPfmZrzkirds8fDWklFE= +github.com/tj/assert v0.0.3 h1:Df/BlaZ20mq6kuai7f5z2TvPFiwC3xaWJSDQNiIS3Rk= +github.com/tj/assert v0.0.3/go.mod h1:Ne6X72Q+TB1AteidzQncjw9PabbMp4PBMZ1k+vd1Pvk= +github.com/tj/go-redirects v0.0.0-20180508180010-5c02ead0bbc5 h1:1gWKekoYJSrFfE3r+Q4kfV+DkOWpwH+DHTFZvbbaelQ= +github.com/tj/go-redirects v0.0.0-20180508180010-5c02ead0bbc5/go.mod h1:E0E2H2gQA+uoi27VCSU+a/BULPtadQA78q3cpTjZbZw= github.com/tomasen/realip v0.0.0-20180522021738-f0c99a92ddce h1:fb190+cK2Xz/dvi9Hv8eCYJYvIGUTN2/KLq1pT6CjEc= github.com/tomasen/realip v0.0.0-20180522021738-f0c99a92ddce/go.mod h1:o8v6yHRoik09Xen7gje4m9ERNah1d1PPsVq1VEx9vE4= github.com/uber-go/atomic v1.3.2/go.mod h1:/Ct5t2lcmbJ4OSe/waGBoaVvVqtO0bmtfVNex1PFV8g= @@ -524,8 +528,9 @@ gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.5/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= -gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.0-20200605160147-a5ece683394c h1:grhR+C34yXImVGp7EzNk+DTIk+323eIUWOmEevy6bDo= +gopkg.in/yaml.v3 v3.0.0-20200605160147-a5ece683394c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/internal/redirects/redirects.go b/internal/redirects/redirects.go new file mode 100644 index 00000000..f6ec766e --- /dev/null +++ b/internal/redirects/redirects.go @@ -0,0 +1,200 @@ +// Package redirects provides functions for parsing and rewriting URLs +// according to Netlify style _redirects syntax +package redirects + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/url" + "regexp" + "strings" + + netlifyRedirects "github.com/tj/go-redirects" + + "gitlab.com/gitlab-org/labkit/log" + + "gitlab.com/gitlab-org/gitlab-pages/internal/vfs" +) + +const ( + // ConfigFile is the default name of the file containing the redirect rules. + // It follows Netlify's syntax but we don't support the special options yet like splats, placeholders, query parameters + // - https://docs.netlify.com/routing/redirects/ + // - https://docs.netlify.com/routing/redirects/redirect-options/ + ConfigFile = "_redirects" + maxConfigSize = 32 * 1024 +) + +var ( + // 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") + errConfigNotFound = errors.New("_redirects file not found") + errNeedRegularFile = errors.New("_redirects needs to be a regular file (not a directory)") + errFileTooLarge = errors.New("_redirects file too large") + errFailedToOpenConfig = errors.New("unable to open _redirects file") + errFailedToParseConfig = errors.New("failed to parse _redirects file") + errFailedToParseURL = errors.New("unable to parse URL") + errNoDomainLevelRedirects = errors.New("no domain-level redirects to outside sites") + errNoStartingForwardSlashInURLPath = errors.New("url path must start with forward slash /") + errNoSplats = errors.New("splats are not supported") + errNoPlaceholders = errors.New("placeholders are not supported") + errNoParams = errors.New("params not supported") + errUnsupportedStatus = errors.New("status not supported") + errNoForce = errors.New("force! not supported") + regexpPlaceholder = regexp.MustCompile(`(?i)/:[a-z]+`) +) + +type Redirects struct { + rules []netlifyRedirects.Rule + error error +} + +// Status maps over each redirect rule and returns any error message +func (r *Redirects) Status() string { + if r.error != nil { + return fmt.Sprintf("parse error: %s", r.error.Error()) + } + + messages := make([]string, 0, len(r.rules)+1) + messages = append(messages, fmt.Sprintf("%d rules", len(r.rules))) + + for i, rule := range r.rules { + if err := validateRule(rule); err != nil { + messages = append(messages, fmt.Sprintf("rule %d: error: %s", i+1, err.Error())) + } else { + messages = append(messages, fmt.Sprintf("rule %d: valid", i+1)) + } + } + + return strings.Join(messages, "\n") +} + +func validateURL(urlText string) error { + url, err := url.Parse(urlText) + if err != nil { + return errFailedToParseURL + } + + // No support for domain-level redirects to outside sites: + // - `https://google.com` + // - `//google.com` + if url.Host != "" || url.Scheme != "" { + return errNoDomainLevelRedirects + } + + // No parent traversing relative URL's with `./` or `../` + // No ambiguous URLs like bare domains `GitLab.com` + if !strings.HasPrefix(url.Path, "/") { + return errNoStartingForwardSlashInURLPath + } + + // No support for splats, https://docs.netlify.com/routing/redirects/redirect-options/#splats + if strings.Contains(url.Path, "*") { + return errNoSplats + } + + // No support for placeholders, https://docs.netlify.com/routing/redirects/redirect-options/#placeholders + if regexpPlaceholder.MatchString(url.Path) { + return errNoPlaceholders + } + + return nil +} + +func validateRule(r netlifyRedirects.Rule) error { + if err := validateURL(r.From); err != nil { + return err + } + + if err := validateURL(r.To); err != nil { + return err + } + + // No support for query parameters, https://docs.netlify.com/routing/redirects/redirect-options/#query-parameters + if r.Params != nil { + return errNoParams + } + + // We strictly validate return status codes + switch r.Status { + case http.StatusMovedPermanently, http.StatusFound: + // noop + default: + return errUnsupportedStatus + } + + // No support for rules that use ! force + if r.Force { + return errNoForce + } + + return nil +} + +func normalizePath(path string) string { + return strings.TrimSuffix(path, "/") + "/" +} + +func (r *Redirects) match(url *url.URL) *netlifyRedirects.Rule { + for _, rule := range r.rules { + // TODO: Likely this should include host comparison once we have domain-level redirects + if normalizePath(rule.From) == normalizePath(url.Path) && validateRule(rule) == nil { + return &rule + } + } + + return nil +} + +// Rewrite takes in a URL and uses the parsed Netlify rules to rewrite +// the URL to the new location if it matches any rule +func (r *Redirects) Rewrite(url *url.URL) (*url.URL, int, error) { + rule := r.match(url) + if rule == nil { + return nil, 0, ErrNoRedirect + } + + newURL, err := url.Parse(rule.To) + log.WithFields(log.Fields{ + "url": url, + "newURL": newURL, + "err": err, + "rule.From": rule.From, + "rule.To": rule.To, + "rule.Status": rule.Status, + }).Debug("Rewrite") + return newURL, rule.Status, err +} + +// ParseRedirects decodes Netlify style redirects from the projects `.../public/_redirects` +// https://docs.netlify.com/routing/redirects/#syntax-for-the-redirects-file +func ParseRedirects(ctx context.Context, root vfs.Root) *Redirects { + fi, err := root.Lstat(ctx, ConfigFile) + if err != nil { + return &Redirects{error: errConfigNotFound} + } + + if !fi.Mode().IsRegular() { + return &Redirects{error: errNeedRegularFile} + } + + if fi.Size() > maxConfigSize { + return &Redirects{error: errFileTooLarge} + } + + reader, err := root.Open(ctx, ConfigFile) + if err != nil { + return &Redirects{error: errFailedToOpenConfig} + } + defer reader.Close() + + redirectRules, err := netlifyRedirects.Parse(reader) + if err != nil { + return &Redirects{error: errFailedToParseConfig} + } + + return &Redirects{rules: redirectRules} +} diff --git a/internal/redirects/redirects_benchmark_test.go b/internal/redirects/redirects_benchmark_test.go new file mode 100644 index 00000000..1a05a00c --- /dev/null +++ b/internal/redirects/redirects_benchmark_test.go @@ -0,0 +1,69 @@ +package redirects + +import ( + "context" + "io/ioutil" + "net/url" + "path" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers" +) + +func generateRedirectsFile(dirPath string, count int) error { + content := strings.Repeat("/goto.html /target.html 301\n", count) + content = content + "/entrance.html /exit.html 301\n" + + return ioutil.WriteFile(path.Join(dirPath, ConfigFile), []byte(content), 0600) +} + +func benchmarkRedirectsRewrite(b *testing.B, redirectsCount int) { + ctx := context.Background() + + root, tmpDir, cleanup := testhelpers.TmpDir(nil, "ParseRedirects_benchmarks") + defer cleanup() + + err := generateRedirectsFile(tmpDir, redirectsCount) + require.NoError(b, err) + + url, err := url.Parse("/entrance.html") + require.NoError(b, err) + + redirects := ParseRedirects(ctx, root) + require.NoError(b, redirects.error) + + for i := 0; i < b.N; i++ { + _, _, err := redirects.Rewrite(url) + require.NoError(b, err) + } +} + +func BenchmarkRedirectsRewrite(b *testing.B) { + b.Run("10 redirects", func(b *testing.B) { benchmarkRedirectsRewrite(b, 10) }) + b.Run("100 redirects", func(b *testing.B) { benchmarkRedirectsRewrite(b, 100) }) + b.Run("1000 redirects", func(b *testing.B) { benchmarkRedirectsRewrite(b, 1000) }) +} + +func benchmarkRedirectsParseRedirects(b *testing.B, redirectsCount int) { + ctx := context.Background() + + root, tmpDir, cleanup := testhelpers.TmpDir(nil, "ParseRedirects_benchmarks") + defer cleanup() + + err := generateRedirectsFile(tmpDir, redirectsCount) + require.NoError(b, err) + + for i := 0; i < b.N; i++ { + redirects := ParseRedirects(ctx, root) + require.NoError(b, redirects.error) + } +} + +func BenchmarkRedirectsParseRedirects(b *testing.B) { + b.Run("10 redirects", func(b *testing.B) { benchmarkRedirectsParseRedirects(b, 10) }) + b.Run("100 redirects", func(b *testing.B) { benchmarkRedirectsParseRedirects(b, 100) }) + b.Run("1000 redirects", func(b *testing.B) { benchmarkRedirectsParseRedirects(b, 1000) }) +} diff --git a/internal/redirects/redirects_test.go b/internal/redirects/redirects_test.go new file mode 100644 index 00000000..f538fc4b --- /dev/null +++ b/internal/redirects/redirects_test.go @@ -0,0 +1,296 @@ +package redirects + +import ( + "context" + "io/ioutil" + "net/url" + "path" + "strings" + "testing" + + "github.com/stretchr/testify/require" + netlifyRedirects "github.com/tj/go-redirects" + + "gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers" +) + +func TestRedirectsValidateUrl(t *testing.T) { + tests := []struct { + name string + url string + expectedErr string + }{ + { + name: "Valid url", + url: "/goto.html", + expectedErr: "", + }, + { + name: "No domain-level redirects", + url: "https://GitLab.com", + expectedErr: errNoDomainLevelRedirects.Error(), + }, + { + name: "No Schema-less URL domain-level redirects", + url: "//GitLab.com/pages.html", + expectedErr: errNoDomainLevelRedirects.Error(), + }, + { + name: "No bare domain-level redirects", + url: "GitLab.com", + expectedErr: errNoStartingForwardSlashInURLPath.Error(), + }, + { + name: "No parent traversing relative URL", + url: "../target.html", + expectedErr: errNoStartingForwardSlashInURLPath.Error(), + }, + { + name: "No splats", + url: "/blog/*", + expectedErr: errNoSplats.Error(), + }, + { + name: "No Placeholders", + url: "/news/:year/:month/:date/:slug", + expectedErr: errNoPlaceholders.Error(), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateURL(tt.url) + if tt.expectedErr != "" { + require.EqualError(t, err, tt.expectedErr) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestRedirectsValidateRule(t *testing.T) { + tests := []struct { + name string + rule string + expectedErr string + }{ + { + name: "valid rule", + rule: "/goto.html /target.html 301", + expectedErr: "", + }, + { + name: "invalid From URL", + rule: "invalid.com /teapot.html 302", + expectedErr: errNoStartingForwardSlashInURLPath.Error(), + }, + { + name: "invalid To URL", + rule: "/goto.html invalid.com", + expectedErr: errNoStartingForwardSlashInURLPath.Error(), + }, + { + name: "No parameters", + rule: "/ /something 302 foo=bar", + expectedErr: errNoParams.Error(), + }, + { + name: "Invalid status", + rule: "/goto.html /target.html 418", + expectedErr: errUnsupportedStatus.Error(), + }, + { + name: "Force not supported", + rule: "/goto.html /target.html 302!", + expectedErr: errNoForce.Error(), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rules, err := netlifyRedirects.ParseString(tt.rule) + require.NoError(t, err) + + err = validateRule(rules[0]) + if tt.expectedErr != "" { + require.EqualError(t, err, tt.expectedErr) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestRedirectsRewrite(t *testing.T) { + tests := []struct { + name string + url string + rule string + expectedURL string + expectedStatus int + expectedErr string + }{ + { + name: "No rules given", + url: "/no-redirect/", + rule: "", + expectedURL: "", + expectedStatus: 0, + expectedErr: ErrNoRedirect.Error(), + }, + { + name: "No matching rules", + url: "/no-redirect/", + rule: "/cake-portal.html /still-alive.html 301", + expectedURL: "", + expectedStatus: 0, + expectedErr: ErrNoRedirect.Error(), + }, + { + name: "Matching rule redirects", + url: "/cake-portal.html", + rule: "/cake-portal.html /still-alive.html 301", + expectedURL: "/still-alive.html", + expectedStatus: 301, + expectedErr: "", + }, + { + name: "Does not redirect to invalid rule", + url: "/goto.html", + rule: "/goto.html GitLab.com 301", + expectedURL: "", + expectedStatus: 0, + expectedErr: ErrNoRedirect.Error(), + }, + { + name: "Matches trailing slash rule to no trailing slash URL", + url: "/cake-portal", + rule: "/cake-portal/ /still-alive/ 301", + expectedURL: "/still-alive/", + expectedStatus: 301, + expectedErr: "", + }, + { + name: "Matches trailing slash rule to trailing slash URL", + url: "/cake-portal/", + rule: "/cake-portal/ /still-alive/ 301", + expectedURL: "/still-alive/", + expectedStatus: 301, + expectedErr: "", + }, + { + name: "Matches no trailing slash rule to no trailing slash URL", + url: "/cake-portal", + rule: "/cake-portal /still-alive 301", + expectedURL: "/still-alive", + expectedStatus: 301, + expectedErr: "", + }, + { + name: "Matches no trailing slash rule to trailing slash URL", + url: "/cake-portal/", + rule: "/cake-portal /still-alive 301", + expectedURL: "/still-alive", + expectedStatus: 301, + expectedErr: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := Redirects{} + + if tt.rule != "" { + rules, err := netlifyRedirects.ParseString(tt.rule) + require.NoError(t, err) + r.rules = rules + } + + url, err := url.Parse(tt.url) + require.NoError(t, err) + + toURL, status, err := r.Rewrite(url) + + if tt.expectedURL != "" { + require.Equal(t, tt.expectedURL, toURL.String()) + } else { + require.Nil(t, toURL) + } + + require.Equal(t, tt.expectedStatus, status) + + if tt.expectedErr != "" { + require.EqualError(t, err, tt.expectedErr) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestRedirectsParseRedirects(t *testing.T) { + ctx := context.Background() + + root, tmpDir, cleanup := testhelpers.TmpDir(t, "ParseRedirects_tests") + defer cleanup() + + tests := []struct { + name string + redirectsFile string + expectedRules int + expectedErr string + }{ + { + name: "No `_redirects` file present", + redirectsFile: "", + expectedRules: 0, + expectedErr: errConfigNotFound.Error(), + }, + { + name: "Everything working as expected", + redirectsFile: `/goto.html /target.html 301`, + expectedRules: 1, + expectedErr: "", + }, + { + name: "Invalid _redirects syntax gives no rules", + redirectsFile: `foobar::baz`, + expectedRules: 0, + expectedErr: "", + }, + { + name: "Config file too big", + redirectsFile: strings.Repeat("a", 2*maxConfigSize), + expectedRules: 0, + expectedErr: errFileTooLarge.Error(), + }, + // In future versions of `github.com/tj/go-redirects`, + // this may not throw a parsing error and this test could be removed + { + name: "Parsing error is caught", + redirectsFile: "/store id=:id /blog/:id 301", + expectedRules: 0, + expectedErr: errFailedToParseConfig.Error(), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.redirectsFile != "" { + err := ioutil.WriteFile(path.Join(tmpDir, ConfigFile), []byte(tt.redirectsFile), 0600) + require.NoError(t, err) + } + + redirects := ParseRedirects(ctx, root) + + if tt.expectedErr != "" { + require.EqualError(t, redirects.error, tt.expectedErr) + } else { + require.NoError(t, redirects.error) + } + + require.Len(t, redirects.rules, tt.expectedRules) + }) + } +} diff --git a/internal/serving/disk/reader.go b/internal/serving/disk/reader.go index 0d11a184..350d036b 100644 --- a/internal/serving/disk/reader.go +++ b/internal/serving/disk/reader.go @@ -5,12 +5,14 @@ import ( "fmt" "io" "net/http" + "os" "strconv" "strings" "time" "github.com/prometheus/client_golang/prometheus" + "gitlab.com/gitlab-org/gitlab-pages/internal/redirects" "gitlab.com/gitlab-org/gitlab-pages/internal/serving" "gitlab.com/gitlab-org/gitlab-pages/internal/serving/disk/symlink" "gitlab.com/gitlab-org/gitlab-pages/internal/vfs" @@ -22,6 +24,34 @@ type Reader struct { vfs vfs.VFS } +// Show the user some validation messages for their _redirects file +func (reader *Reader) serveRedirectsStatus(h serving.Handler, redirects *redirects.Redirects) error { + h.Writer.Header().Set("Content-Type", "text/plain; charset=utf-8") + h.Writer.Header().Set("X-Content-Type-Options", "nosniff") + h.Writer.WriteHeader(http.StatusOK) + _, err := fmt.Fprintln(h.Writer, redirects.Status()) + return err +} + +func (reader *Reader) tryRedirects(h serving.Handler) error { + ctx := h.Request.Context() + root, err := reader.vfs.Root(ctx, h.LookupPath.Path) + if err != nil { + return err + } + + r := redirects.ParseRedirects(ctx, root) + + rewrittenURL, status, err := r.Rewrite(h.Request.URL) + if err != nil { + return err + } + + http.Redirect(h.Writer, h.Request, rewrittenURL.Path, status) + + return nil +} + func (reader *Reader) tryFile(h serving.Handler) error { ctx := h.Request.Context() @@ -62,6 +92,18 @@ func (reader *Reader) tryFile(h serving.Handler) error { return err } + // Serve status of `_redirects` under `_redirects` + // We check if the final resolved path is `_redirects` after symlink traversal + if fullPath == redirects.ConfigFile { + if os.Getenv("FF_ENABLE_REDIRECTS") == "true" { + r := redirects.ParseRedirects(ctx, root) + return reader.serveRedirectsStatus(h, r) + } + + h.Writer.WriteHeader(http.StatusForbidden) + return nil + } + return reader.serveFile(ctx, h.Writer, h.Request, root, fullPath, h.LookupPath.HasAccessControl) } diff --git a/internal/serving/disk/serving.go b/internal/serving/disk/serving.go index f95f983c..bb9b40d2 100644 --- a/internal/serving/disk/serving.go +++ b/internal/serving/disk/serving.go @@ -1,6 +1,8 @@ package disk import ( + "os" + "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" "gitlab.com/gitlab-org/gitlab-pages/internal/serving" "gitlab.com/gitlab-org/gitlab-pages/internal/vfs" @@ -15,7 +17,17 @@ type Disk struct { // ServeFileHTTP serves a file from disk and returns true. It returns false // when a file could not been found. func (s *Disk) ServeFileHTTP(h serving.Handler) bool { - return s.reader.tryFile(h) == nil + if s.reader.tryFile(h) == nil { + return true + } + + if os.Getenv("FF_ENABLE_REDIRECTS") == "true" { + if s.reader.tryRedirects(h) == nil { + return true + } + } + + return false } // ServeNotFoundHTTP tries to read a custom 404 page diff --git a/internal/serving/disk/symlink/path_test.go b/internal/serving/disk/symlink/path_test.go index 23978037..8c1d19b3 100644 --- a/internal/serving/disk/symlink/path_test.go +++ b/internal/serving/disk/symlink/path_test.go @@ -16,28 +16,13 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitlab-pages/internal/serving/disk/symlink" + "gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers" "gitlab.com/gitlab-org/gitlab-pages/internal/vfs" "gitlab.com/gitlab-org/gitlab-pages/internal/vfs/local" ) var fs = vfs.Instrumented(&local.VFS{}, "local") -func tmpDir(t *testing.T) (vfs.Root, string, func()) { - tmpDir, err := ioutil.TempDir("", "symlink_tests") - require.NoError(t, err) - - // On some systems `/tmp` can be a symlink - tmpDir, err = filepath.EvalSymlinks(tmpDir) - require.NoError(t, err) - - root, err := fs.Root(context.Background(), tmpDir) - require.NoError(t, err) - - return root, tmpDir, func() { - os.RemoveAll(tmpDir) - } -} - type EvalSymlinksTest struct { // If dest is empty, the path is created; otherwise the dest is symlinked to the path. path, dest string @@ -100,7 +85,7 @@ func testEvalSymlinks(t *testing.T, wd, path, want string) { } func TestEvalSymlinks(t *testing.T) { - _, tmpDir, cleanup := tmpDir(t) + _, tmpDir, cleanup := testhelpers.TmpDir(t, "symlink_tests") defer cleanup() // Create the symlink farm using relative paths. @@ -139,7 +124,7 @@ func TestEvalSymlinks(t *testing.T) { } func TestEvalSymlinksIsNotExist(t *testing.T) { - root, _, cleanup := tmpDir(t) + root, _, cleanup := testhelpers.TmpDir(t, "symlink_tests") defer cleanup() _, err := symlink.EvalSymlinks(context.Background(), root, "notexist") @@ -160,7 +145,7 @@ func TestEvalSymlinksIsNotExist(t *testing.T) { } func TestIssue13582(t *testing.T) { - root, tmpDir, cleanup := tmpDir(t) + root, tmpDir, cleanup := testhelpers.TmpDir(t, "symlink_tests") defer cleanup() dir := filepath.Join(tmpDir, "dir") diff --git a/internal/source/disk/map_test.go b/internal/source/disk/map_test.go index 4b9d6a23..2a5fd828 100644 --- a/internal/source/disk/map_test.go +++ b/internal/source/disk/map_test.go @@ -52,6 +52,8 @@ func TestReadProjects(t *testing.T) { "withacmechallenge.domain.com", "capitalgroup.test.io", "group.404.gitlab-example.com", + "group.redirects.test.io", + "redirects.custom-domain.com", } for _, expected := range domains { diff --git a/internal/testhelpers/tmpdir.go b/internal/testhelpers/tmpdir.go new file mode 100644 index 00000000..85201238 --- /dev/null +++ b/internal/testhelpers/tmpdir.go @@ -0,0 +1,38 @@ +package testhelpers + +import ( + "context" + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "gitlab.com/gitlab-org/gitlab-pages/internal/vfs" + "gitlab.com/gitlab-org/gitlab-pages/internal/vfs/local" +) + +var fs = vfs.Instrumented(&local.VFS{}, "local") + +func TmpDir(t *testing.T, pattern string) (vfs.Root, string, func()) { + tmpDir, err := ioutil.TempDir("", pattern) + if t != nil { + require.NoError(t, err) + } + + // On some systems `/tmp` can be a symlink + tmpDir, err = filepath.EvalSymlinks(tmpDir) + if t != nil { + require.NoError(t, err) + } + + root, err := fs.Root(context.Background(), tmpDir) + if t != nil { + require.NoError(t, err) + } + + return root, tmpDir, func() { + os.RemoveAll(tmpDir) + } +} diff --git a/shared/pages/group.redirects/custom-domain/config.json b/shared/pages/group.redirects/custom-domain/config.json new file mode 100644 index 00000000..06026e53 --- /dev/null +++ b/shared/pages/group.redirects/custom-domain/config.json @@ -0,0 +1,7 @@ +{ + "domains": [ + { + "domain": "redirects.custom-domain.com" + } + ] +} diff --git a/shared/pages/group.redirects/custom-domain/public/_redirects b/shared/pages/group.redirects/custom-domain/public/_redirects new file mode 100644 index 00000000..42913b7a --- /dev/null +++ b/shared/pages/group.redirects/custom-domain/public/_redirects @@ -0,0 +1,11 @@ +/redirect-portal.html /magic-land.html 302 +/cake-portal.html /still-alive.html 302 +/jobs/* /careers/:splat +/wardrobe.html /narnia.html 302 +/news/:year/:month/:date/:slug /blog/:year/:month/:date/:slug +/pit.html /spikes.html 302 +/goto-domain.html https://GitLab.com/pages.html 302 +/goto-bare-domain.html GitLab.com/pages.html 302 +/goto-schemaless.html //GitLab.com/pages.html 302 +/cake-portal/ /still-alive/ 302 +/file-override.html /should-not-be-here.html 302 diff --git a/shared/pages/group.redirects/group.redirects.gitlab-example.com/public/_redirects b/shared/pages/group.redirects/group.redirects.gitlab-example.com/public/_redirects new file mode 100644 index 00000000..42913b7a --- /dev/null +++ b/shared/pages/group.redirects/group.redirects.gitlab-example.com/public/_redirects @@ -0,0 +1,11 @@ +/redirect-portal.html /magic-land.html 302 +/cake-portal.html /still-alive.html 302 +/jobs/* /careers/:splat +/wardrobe.html /narnia.html 302 +/news/:year/:month/:date/:slug /blog/:year/:month/:date/:slug +/pit.html /spikes.html 302 +/goto-domain.html https://GitLab.com/pages.html 302 +/goto-bare-domain.html GitLab.com/pages.html 302 +/goto-schemaless.html //GitLab.com/pages.html 302 +/cake-portal/ /still-alive/ 302 +/file-override.html /should-not-be-here.html 302 diff --git a/shared/pages/group.redirects/project-redirects/public/_redirects b/shared/pages/group.redirects/project-redirects/public/_redirects new file mode 100644 index 00000000..04c44ee4 --- /dev/null +++ b/shared/pages/group.redirects/project-redirects/public/_redirects @@ -0,0 +1,11 @@ +/project-redirects/redirect-portal.html /project-redirects/magic-land.html 302 +/project-redirects/cake-portal.html /project-redirects/still-alive.html 302 +/project-redirects/jobs/* /project-redirects/careers/:splat +/project-redirects/wardrobe.html /project-redirects/narnia.html 302 +/project-redirects/news/:year/:month/:date/:slug /project-redirects/blog/:year/:month/:date/:slug +/project-redirects/pit.html /project-redirects/spikes.html 302 +/project-redirects/goto-domain.html https://GitLab.com/pages.html 302 +/project-redirects/goto-bare-domain.html GitLab.com/pages.html 302 +/project-redirects/goto-schemaless.html //GitLab.com/pages.html 302 +/project-redirects/cake-portal/ /project-redirects/still-alive/ 302 +/project-redirects/file-override.html /project-redirects/should-not-be-here.html 302 diff --git a/shared/pages/group.redirects/project-redirects/public/file-override.html b/shared/pages/group.redirects/project-redirects/public/file-override.html new file mode 100644 index 00000000..63f14c5e --- /dev/null +++ b/shared/pages/group.redirects/project-redirects/public/file-override.html @@ -0,0 +1 @@ +the file was served! diff --git a/shared/pages/group.redirects/project-redirects/public/index.html b/shared/pages/group.redirects/project-redirects/public/index.html new file mode 100644 index 00000000..985b2786 --- /dev/null +++ b/shared/pages/group.redirects/project-redirects/public/index.html @@ -0,0 +1 @@ +Visit <a href="http://group.redirects.pages.gdk.test:8090/project-redirects/redirect-portal.html">http://group.redirects.pages.gdk.test:8090/project-redirects/redirect-portal.html</a> and get redirected to <a href="http://group.redirects.pages.gdk.test:8090/project-redirects/magic-land.html">http://group.redirects.pages.gdk.test:8090/project-redirects/magic-land.html</a> diff --git a/shared/pages/group.redirects/project-redirects/public/magic-land.html b/shared/pages/group.redirects/project-redirects/public/magic-land.html new file mode 100644 index 00000000..f594fab7 --- /dev/null +++ b/shared/pages/group.redirects/project-redirects/public/magic-land.html @@ -0,0 +1 @@ +Magic land! |