From b15ee3e8b11d6399a2e5d6bf709ca0c37aa53124 Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Fri, 31 Jul 2020 11:14:34 +1000 Subject: [PATCH 1/6] WIP: move reader to its own package --- internal/serving/reader/helpers.go | 78 ++++++++++++ internal/serving/reader/reader.go | 187 +++++++++++++++++++++++++++++ 2 files changed, 265 insertions(+) create mode 100644 internal/serving/reader/helpers.go create mode 100644 internal/serving/reader/reader.go diff --git a/internal/serving/reader/helpers.go b/internal/serving/reader/helpers.go new file mode 100644 index 000000000..305ec3b78 --- /dev/null +++ b/internal/serving/reader/helpers.go @@ -0,0 +1,78 @@ +package reader + +import ( + "io" + "mime" + "net/http" + "os" + "path/filepath" + "strings" + + "golang.org/x/sys/unix" + + "gitlab.com/gitlab-org/gitlab-pages/internal/httputil" +) + +func endsWithSlash(path string) bool { + return strings.HasSuffix(path, "/") +} + +func endsWithoutHTMLExtension(path string) bool { + return !strings.HasSuffix(path, ".html") +} + +func openNoFollow(path string) (*os.File, error) { + return os.OpenFile(path, os.O_RDONLY|unix.O_NOFOLLOW, 0) +} + +// Detect file's content-type either by extension or mime-sniffing. +// Implementation is adapted from Golang's `http.serveContent()` +// See https://github.com/golang/go/blob/902fc114272978a40d2e65c2510a18e870077559/src/net/http/fs.go#L194 +func detectContentType(path string) (string, error) { + contentType := mime.TypeByExtension(filepath.Ext(path)) + + if contentType == "" { + var buf [512]byte + + file, err := os.Open(path) + if err != nil { + return "", err + } + + defer file.Close() + + // Using `io.ReadFull()` because `file.Read()` may be chunked. + // Ignoring errors because we don't care if the 512 bytes cannot be read. + n, _ := io.ReadFull(file, buf[:]) + contentType = http.DetectContentType(buf[:n]) + } + + return contentType, nil +} + +func acceptsGZip(r *http.Request) bool { + if r.Header.Get("Range") != "" { + return false + } + + offers := []string{"gzip", "identity"} + acceptedEncoding := httputil.NegotiateContentEncoding(r, offers) + return acceptedEncoding == "gzip" +} + +func handleGZip(w http.ResponseWriter, r *http.Request, fullPath string) string { + if !acceptsGZip(r) { + return fullPath + } + + gzipPath := fullPath + ".gz" + + // Ensure the .gz file is not a symlink + if fi, err := os.Lstat(gzipPath); err != nil || !fi.Mode().IsRegular() { + return fullPath + } + + w.Header().Set("Content-Encoding", "gzip") + + return gzipPath +} diff --git a/internal/serving/reader/reader.go b/internal/serving/reader/reader.go new file mode 100644 index 000000000..bb113ae65 --- /dev/null +++ b/internal/serving/reader/reader.go @@ -0,0 +1,187 @@ +package reader + +import ( + "errors" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "strconv" + "strings" + "time" + + "github.com/prometheus/client_golang/prometheus" + + "gitlab.com/gitlab-org/gitlab-pages/internal/serving" +) + +var ( + errIsDirectory = errors.New("location error accessing directory where file expected") + errNoExtension = errors.New("error accessing a path without an extension") +) + +// Reader is a disk access driver +type Reader struct { + fileSizeMetric prometheus.Histogram +} + +func (reader *Reader) tryFile(h serving.Handler) error { + fullPath, err := reader.resolvePath(h.LookupPath.Path, h.SubPath) + + request := h.Request + host := request.Host + urlPath := request.URL.Path + + if err != nil && err == errIsDirectory { + if endsWithSlash(urlPath) { + fullPath, err = reader.resolvePath(h.LookupPath.Path, h.SubPath, "index.html") + } else { + // TODO why are we doing that? In tests it redirects to HTTPS. This seems wrong, + // issue about this: https://gitlab.com/gitlab-org/gitlab-pages/issues/273 + + // Concat Host with URL.Path + redirectPath := "//" + host + "/" + redirectPath += strings.TrimPrefix(urlPath, "/") + + // Ensure that there's always "/" at end + redirectPath = strings.TrimSuffix(redirectPath, "/") + "/" + http.Redirect(h.Writer, h.Request, redirectPath, 302) + return nil + } + } + + if err != nil && err == errNoExtension { + fullPath, err = reader.resolvePath(h.LookupPath.Path, strings.TrimSuffix(h.SubPath, "/")+".html") + } + + if err != nil { + return err + } + + return reader.serveFile(h.Writer, h.Request, fullPath, h.LookupPath.HasAccessControl) +} + +func (reader *Reader) tryNotFound(h serving.Handler) error { + page404, err := reader.resolvePath(h.LookupPath.Path, "404.html") + if err != nil { + return err + } + + err = reader.serveCustomFile(h.Writer, h.Request, http.StatusNotFound, page404) + if err != nil { + return err + } + return nil +} + +// Resolve the HTTP request to a path on disk, converting requests for +// directories to requests for index.html inside the directory if appropriate. +func (reader *Reader) resolvePath(publicPath string, subPath ...string) (string, error) { + // Ensure that publicPath always ends with "/" + publicPath = strings.TrimSuffix(publicPath, "/") + "/" + + // Don't use filepath.Join as cleans the path, + // where we want to traverse full path as supplied by user + // (including ..) + testPath := publicPath + strings.Join(subPath, "/") + fullPath, err := filepath.EvalSymlinks(testPath) + if err != nil { + if endsWithoutHTMLExtension(testPath) { + return "", errNoExtension + } + + return "", err + } + + // The requested path resolved to somewhere outside of the public/ directory + if !strings.HasPrefix(fullPath, publicPath) && fullPath != filepath.Clean(publicPath) { + return "", fmt.Errorf("%q should be in %q", fullPath, publicPath) + } + + fi, err := os.Lstat(fullPath) + if err != nil { + return "", err + } + + // The requested path is a directory, so try index.html via recursion + if fi.IsDir() { + return "", errIsDirectory + } + + // The file exists, but is not a supported type to serve. Perhaps a block + // special device or something else that may be a security risk. + if !fi.Mode().IsRegular() { + return "", fmt.Errorf("%s: is not a regular file", fullPath) + } + + return fullPath, nil +} + +func (reader *Reader) serveFile(w http.ResponseWriter, r *http.Request, origPath string, accessControl bool) error { + fullPath := handleGZip(w, r, origPath) + + file, err := openNoFollow(fullPath) + if err != nil { + return err + } + + defer file.Close() + + fi, err := file.Stat() + if err != nil { + return err + } + + if !accessControl { + // Set caching headers + w.Header().Set("Cache-Control", "max-age=600") + w.Header().Set("Expires", time.Now().Add(10*time.Minute).Format(time.RFC1123)) + } + + contentType, err := detectContentType(origPath) + if err != nil { + return err + } + + reader.fileSizeMetric.Observe(float64(fi.Size())) + + w.Header().Set("Content-Type", contentType) + http.ServeContent(w, r, origPath, fi.ModTime(), file) + + return nil +} + +func (reader *Reader) serveCustomFile(w http.ResponseWriter, r *http.Request, code int, origPath string) error { + fullPath := handleGZip(w, r, origPath) + + // Open and serve content of file + file, err := openNoFollow(fullPath) + if err != nil { + return err + } + defer file.Close() + + fi, err := file.Stat() + if err != nil { + return err + } + + contentType, err := detectContentType(origPath) + if err != nil { + return err + } + + reader.fileSizeMetric.Observe(float64(fi.Size())) + + w.Header().Set("Content-Type", contentType) + w.Header().Set("Content-Length", strconv.FormatInt(fi.Size(), 10)) + w.WriteHeader(code) + + if r.Method != "HEAD" { + _, err := io.CopyN(w, file, fi.Size()) + return err + } + + return nil +} -- GitLab From e4824679bda7458900e61f9f44fd3fd804fa7888 Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Fri, 31 Jul 2020 15:47:28 +1000 Subject: [PATCH 2/6] Clean file and add test structure --- internal/serving/fileresolver/fileresolver.go | 89 +++++++++ .../serving/fileresolver/fileresolver_test.go | 33 ++++ internal/serving/reader/helpers.go | 78 -------- internal/serving/reader/reader.go | 187 ------------------ 4 files changed, 122 insertions(+), 265 deletions(-) create mode 100644 internal/serving/fileresolver/fileresolver.go create mode 100644 internal/serving/fileresolver/fileresolver_test.go delete mode 100644 internal/serving/reader/helpers.go delete mode 100644 internal/serving/reader/reader.go diff --git a/internal/serving/fileresolver/fileresolver.go b/internal/serving/fileresolver/fileresolver.go new file mode 100644 index 000000000..75081941e --- /dev/null +++ b/internal/serving/fileresolver/fileresolver.go @@ -0,0 +1,89 @@ +package fileresolver + +import ( + "errors" + "os" + "path/filepath" + "strings" +) + +var ( + errIsDirectory = errors.New("location error accessing directory where file expected") + errNoExtension = errors.New("error accessing a path without an extension") + errFileNotFound = errors.New("file not found") + errNotRegularFile = errors.New("not a regular file") + errFileNotInPublicDir = errors.New("file found outside of public directory") +) + +func ResolveFilePath(lookupPath, subPath, requestURLPath string) (string, error) { + fullPath, err := resolvePath(lookupPath, subPath) + if err != nil { + if err == errIsDirectory { + // try to resolve index.html from the path we're currently in + if endsWithSlash(requestURLPath) { + fullPath, err = resolvePath(lookupPath, subPath, "index.html") + if err != nil { + return "", err + } + return fullPath, nil + } + } else if err == errNoExtension { + return resolvePath(lookupPath, strings.TrimSuffix(subPath, "/")+".html") + } + + return "", err + } + + return fullPath, nil +} + +// Resolve the HTTP request to a path on disk, converting requests for +// directories to requests for index.html inside the directory if appropriate. +func resolvePath(publicPath string, subPath ...string) (string, error) { + // Ensure that publicPath always ends with "/" + publicPath = strings.TrimSuffix(publicPath, "/") + "/" + + // Don't use filepath.Join as cleans the path, + // where we want to traverse full path as supplied by user + // (including ..) + testPath := publicPath + strings.Join(subPath, "/") + fullPath, err := filepath.EvalSymlinks(testPath) + if err != nil { + if endsWithoutHTMLExtension(testPath) { + return "", errNoExtension + } + + return "", errFileNotFound + } + + // The requested path resolved to somewhere outside of the public/ directory + if !strings.HasPrefix(fullPath, publicPath) && fullPath != filepath.Clean(publicPath) { + return "", errFileNotInPublicDir + } + + fi, err := os.Lstat(fullPath) + if err != nil { + return "", errFileNotFound + } + + // The requested path is a directory, so try index.html via recursion + if fi.IsDir() { + return "", errIsDirectory + } + + // The file exists, but is not a supported type to serve. Perhaps a block + // special device or something else that may be a security risk. + if !fi.Mode().IsRegular() { + return "", errNotRegularFile + } + + return fullPath, nil +} + +func endsWithSlash(path string) bool { + return strings.HasSuffix(path, "/") +} + +func endsWithoutHTMLExtension(path string) bool { + return !strings.HasSuffix(path, ".html") +} diff --git a/internal/serving/fileresolver/fileresolver_test.go b/internal/serving/fileresolver/fileresolver_test.go new file mode 100644 index 000000000..b7207a36b --- /dev/null +++ b/internal/serving/fileresolver/fileresolver_test.go @@ -0,0 +1,33 @@ +package fileresolver + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestResolveFilePath(t *testing.T) { + tests := []struct { + name string + lookupPath string + subPath string + urlPath string + expectedFullPath string + expectedErr error + }{ + { + name: "file_does_not_exist", + lookupPath: "../../../shared/pages/group/group.no.projects/", + urlPath: "/group.no.projects/", + expectedErr: errFileNotFound, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fullPath, err := ResolveFilePath(tt.lookupPath, tt.subPath, tt.urlPath) + require.Equal(t, tt.expectedFullPath, fullPath) + require.Equal(t, tt.expectedErr, err) + }) + } +} diff --git a/internal/serving/reader/helpers.go b/internal/serving/reader/helpers.go deleted file mode 100644 index 305ec3b78..000000000 --- a/internal/serving/reader/helpers.go +++ /dev/null @@ -1,78 +0,0 @@ -package reader - -import ( - "io" - "mime" - "net/http" - "os" - "path/filepath" - "strings" - - "golang.org/x/sys/unix" - - "gitlab.com/gitlab-org/gitlab-pages/internal/httputil" -) - -func endsWithSlash(path string) bool { - return strings.HasSuffix(path, "/") -} - -func endsWithoutHTMLExtension(path string) bool { - return !strings.HasSuffix(path, ".html") -} - -func openNoFollow(path string) (*os.File, error) { - return os.OpenFile(path, os.O_RDONLY|unix.O_NOFOLLOW, 0) -} - -// Detect file's content-type either by extension or mime-sniffing. -// Implementation is adapted from Golang's `http.serveContent()` -// See https://github.com/golang/go/blob/902fc114272978a40d2e65c2510a18e870077559/src/net/http/fs.go#L194 -func detectContentType(path string) (string, error) { - contentType := mime.TypeByExtension(filepath.Ext(path)) - - if contentType == "" { - var buf [512]byte - - file, err := os.Open(path) - if err != nil { - return "", err - } - - defer file.Close() - - // Using `io.ReadFull()` because `file.Read()` may be chunked. - // Ignoring errors because we don't care if the 512 bytes cannot be read. - n, _ := io.ReadFull(file, buf[:]) - contentType = http.DetectContentType(buf[:n]) - } - - return contentType, nil -} - -func acceptsGZip(r *http.Request) bool { - if r.Header.Get("Range") != "" { - return false - } - - offers := []string{"gzip", "identity"} - acceptedEncoding := httputil.NegotiateContentEncoding(r, offers) - return acceptedEncoding == "gzip" -} - -func handleGZip(w http.ResponseWriter, r *http.Request, fullPath string) string { - if !acceptsGZip(r) { - return fullPath - } - - gzipPath := fullPath + ".gz" - - // Ensure the .gz file is not a symlink - if fi, err := os.Lstat(gzipPath); err != nil || !fi.Mode().IsRegular() { - return fullPath - } - - w.Header().Set("Content-Encoding", "gzip") - - return gzipPath -} diff --git a/internal/serving/reader/reader.go b/internal/serving/reader/reader.go deleted file mode 100644 index bb113ae65..000000000 --- a/internal/serving/reader/reader.go +++ /dev/null @@ -1,187 +0,0 @@ -package reader - -import ( - "errors" - "fmt" - "io" - "net/http" - "os" - "path/filepath" - "strconv" - "strings" - "time" - - "github.com/prometheus/client_golang/prometheus" - - "gitlab.com/gitlab-org/gitlab-pages/internal/serving" -) - -var ( - errIsDirectory = errors.New("location error accessing directory where file expected") - errNoExtension = errors.New("error accessing a path without an extension") -) - -// Reader is a disk access driver -type Reader struct { - fileSizeMetric prometheus.Histogram -} - -func (reader *Reader) tryFile(h serving.Handler) error { - fullPath, err := reader.resolvePath(h.LookupPath.Path, h.SubPath) - - request := h.Request - host := request.Host - urlPath := request.URL.Path - - if err != nil && err == errIsDirectory { - if endsWithSlash(urlPath) { - fullPath, err = reader.resolvePath(h.LookupPath.Path, h.SubPath, "index.html") - } else { - // TODO why are we doing that? In tests it redirects to HTTPS. This seems wrong, - // issue about this: https://gitlab.com/gitlab-org/gitlab-pages/issues/273 - - // Concat Host with URL.Path - redirectPath := "//" + host + "/" - redirectPath += strings.TrimPrefix(urlPath, "/") - - // Ensure that there's always "/" at end - redirectPath = strings.TrimSuffix(redirectPath, "/") + "/" - http.Redirect(h.Writer, h.Request, redirectPath, 302) - return nil - } - } - - if err != nil && err == errNoExtension { - fullPath, err = reader.resolvePath(h.LookupPath.Path, strings.TrimSuffix(h.SubPath, "/")+".html") - } - - if err != nil { - return err - } - - return reader.serveFile(h.Writer, h.Request, fullPath, h.LookupPath.HasAccessControl) -} - -func (reader *Reader) tryNotFound(h serving.Handler) error { - page404, err := reader.resolvePath(h.LookupPath.Path, "404.html") - if err != nil { - return err - } - - err = reader.serveCustomFile(h.Writer, h.Request, http.StatusNotFound, page404) - if err != nil { - return err - } - return nil -} - -// Resolve the HTTP request to a path on disk, converting requests for -// directories to requests for index.html inside the directory if appropriate. -func (reader *Reader) resolvePath(publicPath string, subPath ...string) (string, error) { - // Ensure that publicPath always ends with "/" - publicPath = strings.TrimSuffix(publicPath, "/") + "/" - - // Don't use filepath.Join as cleans the path, - // where we want to traverse full path as supplied by user - // (including ..) - testPath := publicPath + strings.Join(subPath, "/") - fullPath, err := filepath.EvalSymlinks(testPath) - if err != nil { - if endsWithoutHTMLExtension(testPath) { - return "", errNoExtension - } - - return "", err - } - - // The requested path resolved to somewhere outside of the public/ directory - if !strings.HasPrefix(fullPath, publicPath) && fullPath != filepath.Clean(publicPath) { - return "", fmt.Errorf("%q should be in %q", fullPath, publicPath) - } - - fi, err := os.Lstat(fullPath) - if err != nil { - return "", err - } - - // The requested path is a directory, so try index.html via recursion - if fi.IsDir() { - return "", errIsDirectory - } - - // The file exists, but is not a supported type to serve. Perhaps a block - // special device or something else that may be a security risk. - if !fi.Mode().IsRegular() { - return "", fmt.Errorf("%s: is not a regular file", fullPath) - } - - return fullPath, nil -} - -func (reader *Reader) serveFile(w http.ResponseWriter, r *http.Request, origPath string, accessControl bool) error { - fullPath := handleGZip(w, r, origPath) - - file, err := openNoFollow(fullPath) - if err != nil { - return err - } - - defer file.Close() - - fi, err := file.Stat() - if err != nil { - return err - } - - if !accessControl { - // Set caching headers - w.Header().Set("Cache-Control", "max-age=600") - w.Header().Set("Expires", time.Now().Add(10*time.Minute).Format(time.RFC1123)) - } - - contentType, err := detectContentType(origPath) - if err != nil { - return err - } - - reader.fileSizeMetric.Observe(float64(fi.Size())) - - w.Header().Set("Content-Type", contentType) - http.ServeContent(w, r, origPath, fi.ModTime(), file) - - return nil -} - -func (reader *Reader) serveCustomFile(w http.ResponseWriter, r *http.Request, code int, origPath string) error { - fullPath := handleGZip(w, r, origPath) - - // Open and serve content of file - file, err := openNoFollow(fullPath) - if err != nil { - return err - } - defer file.Close() - - fi, err := file.Stat() - if err != nil { - return err - } - - contentType, err := detectContentType(origPath) - if err != nil { - return err - } - - reader.fileSizeMetric.Observe(float64(fi.Size())) - - w.Header().Set("Content-Type", contentType) - w.Header().Set("Content-Length", strconv.FormatInt(fi.Size(), 10)) - w.WriteHeader(code) - - if r.Method != "HEAD" { - _, err := io.CopyN(w, file, fi.Size()) - return err - } - - return nil -} -- GitLab From 52d2c9b8955892bf02fc92273e3419d69d3b56dd Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Mon, 3 Aug 2020 18:01:58 +1000 Subject: [PATCH 3/6] Add test cases --- internal/serving/fileresolver/fileresolver.go | 70 +++++++++++++------ .../serving/fileresolver/fileresolver_test.go | 56 +++++++++++++-- .../group/symlink/public/content/index.html | 1 + shared/pages/group/symlink/public/index.html | 1 + .../pages/group/symlink/public/outside.html | 1 + 5 files changed, 101 insertions(+), 28 deletions(-) create mode 100644 shared/pages/group/symlink/public/content/index.html create mode 120000 shared/pages/group/symlink/public/index.html create mode 120000 shared/pages/group/symlink/public/outside.html diff --git a/internal/serving/fileresolver/fileresolver.go b/internal/serving/fileresolver/fileresolver.go index 75081941e..7910da66b 100644 --- a/internal/serving/fileresolver/fileresolver.go +++ b/internal/serving/fileresolver/fileresolver.go @@ -1,7 +1,9 @@ package fileresolver import ( + "archive/zip" "errors" + "fmt" "os" "path/filepath" "strings" @@ -15,20 +17,25 @@ var ( errFileNotInPublicDir = errors.New("file found outside of public directory") ) -func ResolveFilePath(lookupPath, subPath, requestURLPath string) (string, error) { - fullPath, err := resolvePath(lookupPath, subPath) +type evalSymlinkFunc func(string) (string, error) + +func ResolveFilePath(lookupPath, subPath, requestURLPath string, evalSymLink evalSymlinkFunc) (string, error) { + fullPath, err := resolvePath(evalSymLink, lookupPath, subPath) if err != nil { if err == errIsDirectory { + fmt.Println("we should come here first") // try to resolve index.html from the path we're currently in if endsWithSlash(requestURLPath) { - fullPath, err = resolvePath(lookupPath, subPath, "index.html") + fmt.Println("aand then here?") + fullPath, err = resolvePath(evalSymLink, lookupPath, subPath, "index.html") if err != nil { return "", err } + fmt.Printf("and the ending result: %q\n\n", fullPath) return fullPath, nil } } else if err == errNoExtension { - return resolvePath(lookupPath, strings.TrimSuffix(subPath, "/")+".html") + return resolvePath(evalSymLink, lookupPath, strings.TrimSuffix(subPath, "/")+".html") } return "", err @@ -39,51 +46,70 @@ func ResolveFilePath(lookupPath, subPath, requestURLPath string) (string, error) // Resolve the HTTP request to a path on disk, converting requests for // directories to requests for index.html inside the directory if appropriate. -func resolvePath(publicPath string, subPath ...string) (string, error) { +func resolvePath(evalSymLink evalSymlinkFunc, publicPath string, subPath ...string) (string, error) { // Ensure that publicPath always ends with "/" publicPath = strings.TrimSuffix(publicPath, "/") + "/" // Don't use filepath.Join as cleans the path, // where we want to traverse full path as supplied by user // (including ..) + testPath := publicPath + strings.Join(subPath, "/") - fullPath, err := filepath.EvalSymlinks(testPath) - if err != nil { - if endsWithoutHTMLExtension(testPath) { - return "", errNoExtension - } + fullPath, err := evalSymLink(testPath) + if err != nil { + // simpler to return errFileNotFound instead of the other possible errors return "", errFileNotFound } + fmt.Printf("publicPath:%q\ntestPath: %q\nfullPath:%q\n\n", + publicPath, testPath, fullPath) + + for k, s := range subPath { + fmt.Printf("subpath: %d-%q\n", k, s) + } + // if the original testPath ends in with / and the fullPath has no extension, assume it's a directory + if endsWithSlash(testPath) && endsWithoutHTMLExtension(fullPath) { + return "", errIsDirectory + } else if endsWithoutHTMLExtension(fullPath) { + return "", errNoExtension + } + // panic("why") // The requested path resolved to somewhere outside of the public/ directory if !strings.HasPrefix(fullPath, publicPath) && fullPath != filepath.Clean(publicPath) { return "", errFileNotInPublicDir } + return fullPath, nil +} + +func endsWithSlash(path string) bool { + return strings.HasSuffix(path, "/") +} + +func endsWithoutHTMLExtension(path string) bool { + return !strings.HasSuffix(path, ".html") +} + +func openZipFile(fullPath string, archive *zip.Reader) (*zip.File, error) { + return nil, nil +} +func openFSFile(fullPath string) (*os.File, error) { fi, err := os.Lstat(fullPath) if err != nil { - return "", errFileNotFound + return nil, errFileNotFound } // The requested path is a directory, so try index.html via recursion if fi.IsDir() { - return "", errIsDirectory + return nil, errIsDirectory } // The file exists, but is not a supported type to serve. Perhaps a block // special device or something else that may be a security risk. if !fi.Mode().IsRegular() { - return "", errNotRegularFile + return nil, errNotRegularFile } - return fullPath, nil -} - -func endsWithSlash(path string) bool { - return strings.HasSuffix(path, "/") -} - -func endsWithoutHTMLExtension(path string) bool { - return !strings.HasSuffix(path, ".html") + return os.Open(fullPath) } diff --git a/internal/serving/fileresolver/fileresolver_test.go b/internal/serving/fileresolver/fileresolver_test.go index b7207a36b..1d73f15c7 100644 --- a/internal/serving/fileresolver/fileresolver_test.go +++ b/internal/serving/fileresolver/fileresolver_test.go @@ -1,6 +1,7 @@ package fileresolver import ( + "path/filepath" "testing" "github.com/stretchr/testify/require" @@ -9,6 +10,7 @@ import ( func TestResolveFilePath(t *testing.T) { tests := []struct { name string + evalSymlinkFunc evalSymlinkFunc lookupPath string subPath string urlPath string @@ -16,18 +18,60 @@ func TestResolveFilePath(t *testing.T) { expectedErr error }{ { - name: "file_does_not_exist", - lookupPath: "../../../shared/pages/group/group.no.projects/", - urlPath: "/group.no.projects/", - expectedErr: errFileNotFound, + name: "file_exists_with_subpath_and_extension", + evalSymlinkFunc: func(in string) (string, error) { return in, nil }, + lookupPath: "../../../shared/pages/group/group.test.io/public/", + subPath: "index.html", + urlPath: "/index.html", + expectedFullPath: "../../../shared/pages/group/group.test.io/public/index.html", + }, + { + name: "file_exists_without_extension", + evalSymlinkFunc: func(in string) (string, error) { return in, nil }, + lookupPath: "../../../shared/pages/group/group.test.io/public/", + subPath: "index", + urlPath: "/index", + expectedFullPath: "../../../shared/pages/group/group.test.io/public/index.html", + }, + { + name: "file_exists_without_subpath", + evalSymlinkFunc: filepath.EvalSymlinks, + lookupPath: "../../../shared/pages/group/group.test.io/public/", + subPath: "", + urlPath: "/", + expectedFullPath: "../../../shared/pages/group/group.test.io/public/index.html", + }, + { + name: "file_does_not_exist", + evalSymlinkFunc: filepath.EvalSymlinks, + lookupPath: "../../../shared/pages/group/group.no.projects/public/", + subPath: "unknown_file.html", + urlPath: "/group.no.projects/unknown_file.html", + expectedErr: errFileNotFound, + }, + { + name: "symlink_inside_public", + evalSymlinkFunc: filepath.EvalSymlinks, + lookupPath: "../../../shared/pages/group/symlink/public/", + subPath: "index.html", + urlPath: "/symlink/index.html", + expectedFullPath: "../../../shared/pages/group/symlink/public/content/index.html", + }, + { + name: "symlink_outside_of_public_dir", + evalSymlinkFunc: filepath.EvalSymlinks, + lookupPath: "../../../shared/pages/group/symlink/public/", + subPath: "outside.html", + urlPath: "/symlink/outside.html", + expectedErr: errFileNotInPublicDir, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - fullPath, err := ResolveFilePath(tt.lookupPath, tt.subPath, tt.urlPath) - require.Equal(t, tt.expectedFullPath, fullPath) + fullPath, err := ResolveFilePath(tt.lookupPath, tt.subPath, tt.urlPath, tt.evalSymlinkFunc) require.Equal(t, tt.expectedErr, err) + require.Equal(t, tt.expectedFullPath, fullPath) }) } } diff --git a/shared/pages/group/symlink/public/content/index.html b/shared/pages/group/symlink/public/content/index.html new file mode 100644 index 000000000..4ff00de63 --- /dev/null +++ b/shared/pages/group/symlink/public/content/index.html @@ -0,0 +1 @@ +group/symlink/public/content/index.html diff --git a/shared/pages/group/symlink/public/index.html b/shared/pages/group/symlink/public/index.html new file mode 120000 index 000000000..ea8c18b65 --- /dev/null +++ b/shared/pages/group/symlink/public/index.html @@ -0,0 +1 @@ +./content/index.html \ No newline at end of file diff --git a/shared/pages/group/symlink/public/outside.html b/shared/pages/group/symlink/public/outside.html new file mode 120000 index 000000000..0fa189033 --- /dev/null +++ b/shared/pages/group/symlink/public/outside.html @@ -0,0 +1 @@ +../../serving/public/index.html \ No newline at end of file -- GitLab From 6e04f521a7a66cd9a171e968293759705fd40892 Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Wed, 5 Aug 2020 15:12:27 +1000 Subject: [PATCH 4/6] Simplify logic to resolve file fullPath --- internal/serving/fileresolver/fileresolver.go | 61 +++------ .../serving/fileresolver/fileresolver_test.go | 125 ++++++++++++++---- 2 files changed, 118 insertions(+), 68 deletions(-) diff --git a/internal/serving/fileresolver/fileresolver.go b/internal/serving/fileresolver/fileresolver.go index 7910da66b..f306e4ba9 100644 --- a/internal/serving/fileresolver/fileresolver.go +++ b/internal/serving/fileresolver/fileresolver.go @@ -1,10 +1,7 @@ package fileresolver import ( - "archive/zip" "errors" - "fmt" - "os" "path/filepath" "strings" ) @@ -19,22 +16,24 @@ var ( type evalSymlinkFunc func(string) (string, error) +// ResolveFilePath takes a lookupPath and any subPath to determine the file location. +// Requires the original requestURLPath to try to resolve index.html +// Requires an evalSymlinkFunc to determine if the file exists or not. Useful for resolving files in disk func ResolveFilePath(lookupPath, subPath, requestURLPath string, evalSymLink evalSymlinkFunc) (string, error) { fullPath, err := resolvePath(evalSymLink, lookupPath, subPath) if err != nil { if err == errIsDirectory { - fmt.Println("we should come here first") // try to resolve index.html from the path we're currently in if endsWithSlash(requestURLPath) { - fmt.Println("aand then here?") fullPath, err = resolvePath(evalSymLink, lookupPath, subPath, "index.html") if err != nil { return "", err } - fmt.Printf("and the ending result: %q\n\n", fullPath) + return fullPath, nil } } else if err == errNoExtension { + // assume .html extension return resolvePath(evalSymLink, lookupPath, strings.TrimSuffix(subPath, "/")+".html") } @@ -46,6 +45,9 @@ func ResolveFilePath(lookupPath, subPath, requestURLPath string, evalSymLink eva // Resolve the HTTP request to a path on disk, converting requests for // directories to requests for index.html inside the directory if appropriate. +// Takes a `evalSymLinkFunc` to try to follow any symlinks. For disk use `filepath.EvalSymlinks`. +// Returns the resolved fullPath or an error +// TODO: handle zip archives func resolvePath(evalSymLink evalSymlinkFunc, publicPath string, subPath ...string) (string, error) { // Ensure that publicPath always ends with "/" publicPath = strings.TrimSuffix(publicPath, "/") + "/" @@ -53,28 +55,18 @@ func resolvePath(evalSymLink evalSymlinkFunc, publicPath string, subPath ...stri // Don't use filepath.Join as cleans the path, // where we want to traverse full path as supplied by user // (including ..) - - testPath := publicPath + strings.Join(subPath, "/") + testPath := publicPath + strings.Join(cleanEmpty(subPath), "/") + if endsWithSlash(testPath) { + return "", errIsDirectory + } else if endsWithoutHTMLExtension(testPath) { + return "", errNoExtension + } fullPath, err := evalSymLink(testPath) if err != nil { - // simpler to return errFileNotFound instead of the other possible errors return "", errFileNotFound } - fmt.Printf("publicPath:%q\ntestPath: %q\nfullPath:%q\n\n", - publicPath, testPath, fullPath) - - for k, s := range subPath { - fmt.Printf("subpath: %d-%q\n", k, s) - } - // if the original testPath ends in with / and the fullPath has no extension, assume it's a directory - if endsWithSlash(testPath) && endsWithoutHTMLExtension(fullPath) { - return "", errIsDirectory - } else if endsWithoutHTMLExtension(fullPath) { - return "", errNoExtension - } - // panic("why") // The requested path resolved to somewhere outside of the public/ directory if !strings.HasPrefix(fullPath, publicPath) && fullPath != filepath.Clean(publicPath) { return "", errFileNotInPublicDir @@ -91,25 +83,14 @@ func endsWithoutHTMLExtension(path string) bool { return !strings.HasSuffix(path, ".html") } -func openZipFile(fullPath string, archive *zip.Reader) (*zip.File, error) { - return nil, nil -} -func openFSFile(fullPath string) (*os.File, error) { - fi, err := os.Lstat(fullPath) - if err != nil { - return nil, errFileNotFound - } - - // The requested path is a directory, so try index.html via recursion - if fi.IsDir() { - return nil, errIsDirectory - } +func cleanEmpty(in []string) []string { + var out []string - // The file exists, but is not a supported type to serve. Perhaps a block - // special device or something else that may be a security risk. - if !fi.Mode().IsRegular() { - return nil, errNotRegularFile + for _, x := range in { + if x != "" { + out = append(out, x) + } } - return os.Open(fullPath) + return out } diff --git a/internal/serving/fileresolver/fileresolver_test.go b/internal/serving/fileresolver/fileresolver_test.go index 1d73f15c7..dc26c3538 100644 --- a/internal/serving/fileresolver/fileresolver_test.go +++ b/internal/serving/fileresolver/fileresolver_test.go @@ -1,77 +1,146 @@ package fileresolver import ( + "archive/zip" + "io/ioutil" + "os" "path/filepath" "testing" "github.com/stretchr/testify/require" ) -func TestResolveFilePath(t *testing.T) { +func TestResolveFilePathFromDisk(t *testing.T) { + cleanup := setUpTests(t) + defer cleanup() + tests := []struct { name string - evalSymlinkFunc evalSymlinkFunc lookupPath string subPath string urlPath string expectedFullPath string + expectedContent string expectedErr error }{ { name: "file_exists_with_subpath_and_extension", - evalSymlinkFunc: func(in string) (string, error) { return in, nil }, - lookupPath: "../../../shared/pages/group/group.test.io/public/", + lookupPath: "group/group.test.io/public/", subPath: "index.html", urlPath: "/index.html", - expectedFullPath: "../../../shared/pages/group/group.test.io/public/index.html", + expectedFullPath: "group/group.test.io/public/index.html", + expectedContent: "main-dir\n", }, { name: "file_exists_without_extension", - evalSymlinkFunc: func(in string) (string, error) { return in, nil }, - lookupPath: "../../../shared/pages/group/group.test.io/public/", + lookupPath: "group/group.test.io/public/", subPath: "index", urlPath: "/index", - expectedFullPath: "../../../shared/pages/group/group.test.io/public/index.html", + expectedFullPath: "group/group.test.io/public/index.html", + expectedContent: "main-dir\n", }, { name: "file_exists_without_subpath", - evalSymlinkFunc: filepath.EvalSymlinks, - lookupPath: "../../../shared/pages/group/group.test.io/public/", + lookupPath: "group/group.test.io/public/", subPath: "", urlPath: "/", - expectedFullPath: "../../../shared/pages/group/group.test.io/public/index.html", + expectedFullPath: "group/group.test.io/public/index.html", + expectedContent: "main-dir\n", + }, + { + name: "file_does_not_exist_without_subpath", + lookupPath: "group.no.projects/", + subPath: "", + urlPath: "/", + expectedErr: errFileNotFound, }, { - name: "file_does_not_exist", - evalSymlinkFunc: filepath.EvalSymlinks, - lookupPath: "../../../shared/pages/group/group.no.projects/public/", - subPath: "unknown_file.html", - urlPath: "/group.no.projects/unknown_file.html", - expectedErr: errFileNotFound, + name: "file_does_not_exist", + lookupPath: "group/group.test.io/public/", + subPath: "unknown_file.html", + urlPath: "/group.test.io/unknown_file.html", + expectedErr: errFileNotFound, }, { name: "symlink_inside_public", - evalSymlinkFunc: filepath.EvalSymlinks, - lookupPath: "../../../shared/pages/group/symlink/public/", + lookupPath: "group/symlink/public/", subPath: "index.html", urlPath: "/symlink/index.html", - expectedFullPath: "../../../shared/pages/group/symlink/public/content/index.html", + expectedFullPath: "group/symlink/public/content/index.html", + expectedContent: "group/symlink/public/content/index.html\n", }, { - name: "symlink_outside_of_public_dir", - evalSymlinkFunc: filepath.EvalSymlinks, - lookupPath: "../../../shared/pages/group/symlink/public/", - subPath: "outside.html", - urlPath: "/symlink/outside.html", - expectedErr: errFileNotInPublicDir, + name: "symlink_outside_of_public_dir", + lookupPath: "group/symlink/public/", + subPath: "outside.html", + urlPath: "/symlink/outside.html", + expectedErr: errFileNotInPublicDir, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - fullPath, err := ResolveFilePath(tt.lookupPath, tt.subPath, tt.urlPath, tt.evalSymlinkFunc) - require.Equal(t, tt.expectedErr, err) + fullPath, err := ResolveFilePath(tt.lookupPath, tt.subPath, tt.urlPath, filepath.EvalSymlinks) + if tt.expectedErr != nil { + require.Equal(t, tt.expectedErr, err) + return + } + require.Equal(t, tt.expectedFullPath, fullPath) + + file, err := openFSFile(fullPath) + require.NoError(t, err) + defer file.Close() + + content, err := ioutil.ReadAll(file) + require.NoError(t, err) + require.Contains(t, string(content), tt.expectedContent) }) } } + +func setUpTests(t *testing.T) func() { + t.Helper() + + return chdirInPath(t, "../../../shared/pages") +} + +func chdirInPath(t *testing.T, path string) func() { + t.Helper() + + cwd, err := os.Getwd() + require.NoError(t, err, "Cannot Getwd") + + err = os.Chdir(path) + require.NoError(t, err, "Cannot Chdir") + + return func() { + err := os.Chdir(cwd) + require.NoError(t, err, "Cannot Chdir in cleanup") + } +} + +func openZipFile(t *testing.T, fullPath string, archive *zip.Reader) (*zip.File, error) { + t.Helper() + + return nil, nil +} +func openFSFile(fullPath string) (*os.File, error) { + fi, err := os.Lstat(fullPath) + if err != nil { + return nil, errFileNotFound + } + + // The requested path is a directory, so try index.html via recursion + if fi.IsDir() { + return nil, errIsDirectory + } + + // The file exists, but is not a supported type to serve. Perhaps a block + // special device or something else that may be a security risk. + if !fi.Mode().IsRegular() { + return nil, errNotRegularFile + } + + return os.Open(fullPath) +} -- GitLab From 1e9f0c7d31d786dfaf26253baf7d15b387297d82 Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Wed, 5 Aug 2020 17:12:31 +1000 Subject: [PATCH 5/6] Simplify resolver method --- internal/serving/fileresolver/fileresolver.go | 19 ++++++------------- .../serving/fileresolver/fileresolver_test.go | 10 +--------- 2 files changed, 7 insertions(+), 22 deletions(-) diff --git a/internal/serving/fileresolver/fileresolver.go b/internal/serving/fileresolver/fileresolver.go index f306e4ba9..9eb6822ae 100644 --- a/internal/serving/fileresolver/fileresolver.go +++ b/internal/serving/fileresolver/fileresolver.go @@ -16,24 +16,17 @@ var ( type evalSymlinkFunc func(string) (string, error) -// ResolveFilePath takes a lookupPath and any subPath to determine the file location. +// ResolveFilePath takes a archivePath and any subPath to determine the file location. // Requires the original requestURLPath to try to resolve index.html // Requires an evalSymlinkFunc to determine if the file exists or not. Useful for resolving files in disk -func ResolveFilePath(lookupPath, subPath, requestURLPath string, evalSymLink evalSymlinkFunc) (string, error) { +func ResolveFilePath(lookupPath, subPath string, evalSymLink evalSymlinkFunc) (string, error) { fullPath, err := resolvePath(evalSymLink, lookupPath, subPath) if err != nil { if err == errIsDirectory { // try to resolve index.html from the path we're currently in - if endsWithSlash(requestURLPath) { - fullPath, err = resolvePath(evalSymLink, lookupPath, subPath, "index.html") - if err != nil { - return "", err - } - - return fullPath, nil - } + return resolvePath(evalSymLink, lookupPath, subPath, "index.html") } else if err == errNoExtension { - // assume .html extension + // assume .html extension and try to resolve return resolvePath(evalSymLink, lookupPath, strings.TrimSuffix(subPath, "/")+".html") } @@ -46,8 +39,7 @@ func ResolveFilePath(lookupPath, subPath, requestURLPath string, evalSymLink eva // Resolve the HTTP request to a path on disk, converting requests for // directories to requests for index.html inside the directory if appropriate. // Takes a `evalSymLinkFunc` to try to follow any symlinks. For disk use `filepath.EvalSymlinks`. -// Returns the resolved fullPath or an error -// TODO: handle zip archives +// Returns the resolved fullPath, fileName (filepath.Base) and error func resolvePath(evalSymLink evalSymlinkFunc, publicPath string, subPath ...string) (string, error) { // Ensure that publicPath always ends with "/" publicPath = strings.TrimSuffix(publicPath, "/") + "/" @@ -83,6 +75,7 @@ func endsWithoutHTMLExtension(path string) bool { return !strings.HasSuffix(path, ".html") } +// cleanEmpty removes empty string elements in the slice func cleanEmpty(in []string) []string { var out []string diff --git a/internal/serving/fileresolver/fileresolver_test.go b/internal/serving/fileresolver/fileresolver_test.go index dc26c3538..b78317c9d 100644 --- a/internal/serving/fileresolver/fileresolver_test.go +++ b/internal/serving/fileresolver/fileresolver_test.go @@ -18,7 +18,6 @@ func TestResolveFilePathFromDisk(t *testing.T) { name string lookupPath string subPath string - urlPath string expectedFullPath string expectedContent string expectedErr error @@ -27,7 +26,6 @@ func TestResolveFilePathFromDisk(t *testing.T) { name: "file_exists_with_subpath_and_extension", lookupPath: "group/group.test.io/public/", subPath: "index.html", - urlPath: "/index.html", expectedFullPath: "group/group.test.io/public/index.html", expectedContent: "main-dir\n", }, @@ -35,7 +33,6 @@ func TestResolveFilePathFromDisk(t *testing.T) { name: "file_exists_without_extension", lookupPath: "group/group.test.io/public/", subPath: "index", - urlPath: "/index", expectedFullPath: "group/group.test.io/public/index.html", expectedContent: "main-dir\n", }, @@ -43,7 +40,6 @@ func TestResolveFilePathFromDisk(t *testing.T) { name: "file_exists_without_subpath", lookupPath: "group/group.test.io/public/", subPath: "", - urlPath: "/", expectedFullPath: "group/group.test.io/public/index.html", expectedContent: "main-dir\n", }, @@ -51,21 +47,18 @@ func TestResolveFilePathFromDisk(t *testing.T) { name: "file_does_not_exist_without_subpath", lookupPath: "group.no.projects/", subPath: "", - urlPath: "/", expectedErr: errFileNotFound, }, { name: "file_does_not_exist", lookupPath: "group/group.test.io/public/", subPath: "unknown_file.html", - urlPath: "/group.test.io/unknown_file.html", expectedErr: errFileNotFound, }, { name: "symlink_inside_public", lookupPath: "group/symlink/public/", subPath: "index.html", - urlPath: "/symlink/index.html", expectedFullPath: "group/symlink/public/content/index.html", expectedContent: "group/symlink/public/content/index.html\n", }, @@ -73,14 +66,13 @@ func TestResolveFilePathFromDisk(t *testing.T) { name: "symlink_outside_of_public_dir", lookupPath: "group/symlink/public/", subPath: "outside.html", - urlPath: "/symlink/outside.html", expectedErr: errFileNotInPublicDir, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - fullPath, err := ResolveFilePath(tt.lookupPath, tt.subPath, tt.urlPath, filepath.EvalSymlinks) + fullPath, err := ResolveFilePath(tt.lookupPath, tt.subPath, filepath.EvalSymlinks) if tt.expectedErr != nil { require.Equal(t, tt.expectedErr, err) return -- GitLab From ee6704984d95b0328a1c59fb12d080f1f9964f69 Mon Sep 17 00:00:00 2001 From: Jaime Martinez Date: Wed, 5 Aug 2020 17:44:18 +1000 Subject: [PATCH 6/6] WIP: added to reader --- internal/serving/disk/reader.go | 51 ++--------------------------- internal/source/disk/domain_test.go | 17 ++++++---- 2 files changed, 13 insertions(+), 55 deletions(-) diff --git a/internal/serving/disk/reader.go b/internal/serving/disk/reader.go index b8dce5421..67a21b854 100644 --- a/internal/serving/disk/reader.go +++ b/internal/serving/disk/reader.go @@ -13,6 +13,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "gitlab.com/gitlab-org/gitlab-pages/internal/serving" + "gitlab.com/gitlab-org/gitlab-pages/internal/serving/fileresolver" ) // Reader is a disk access driver @@ -22,33 +23,6 @@ type Reader struct { func (reader *Reader) tryFile(h serving.Handler) error { fullPath, err := reader.resolvePath(h.LookupPath.Path, h.SubPath) - - request := h.Request - host := request.Host - urlPath := request.URL.Path - - if locationError, _ := err.(*locationDirectoryError); locationError != nil { - if endsWithSlash(urlPath) { - fullPath, err = reader.resolvePath(h.LookupPath.Path, h.SubPath, "index.html") - } else { - // TODO why are we doing that? In tests it redirects to HTTPS. This seems wrong, - // issue about this: https://gitlab.com/gitlab-org/gitlab-pages/issues/273 - - // Concat Host with URL.Path - redirectPath := "//" + host + "/" - redirectPath += strings.TrimPrefix(urlPath, "/") - - // Ensure that there's always "/" at end - redirectPath = strings.TrimSuffix(redirectPath, "/") + "/" - http.Redirect(h.Writer, h.Request, redirectPath, 302) - return nil - } - } - - if locationError, _ := err.(*locationFileNoExtensionError); locationError != nil { - fullPath, err = reader.resolvePath(h.LookupPath.Path, strings.TrimSuffix(h.SubPath, "/")+".html") - } - if err != nil { return err } @@ -71,31 +45,12 @@ func (reader *Reader) tryNotFound(h serving.Handler) error { // Resolve the HTTP request to a path on disk, converting requests for // directories to requests for index.html inside the directory if appropriate. -func (reader *Reader) resolvePath(publicPath string, subPath ...string) (string, error) { - // Ensure that publicPath always ends with "/" - publicPath = strings.TrimSuffix(publicPath, "/") + "/" - - // Don't use filepath.Join as cleans the path, - // where we want to traverse full path as supplied by user - // (including ..) - testPath := publicPath + strings.Join(subPath, "/") - fullPath, err := filepath.EvalSymlinks(testPath) - +func (reader *Reader) resolvePath(publicPath, subPath string) (string, error) { + fullPath, err := fileresolver.ResolveFilePath(publicPath, subPath, filepath.EvalSymlinks) if err != nil { - if endsWithoutHTMLExtension(testPath) { - return "", &locationFileNoExtensionError{ - FullPath: fullPath, - } - } - return "", err } - // The requested path resolved to somewhere outside of the public/ directory - if !strings.HasPrefix(fullPath, publicPath) && fullPath != filepath.Clean(publicPath) { - return "", fmt.Errorf("%q should be in %q", fullPath, publicPath) - } - fi, err := os.Lstat(fullPath) if err != nil { return "", err diff --git a/internal/source/disk/domain_test.go b/internal/source/disk/domain_test.go index efe450430..1cf036579 100644 --- a/internal/source/disk/domain_test.go +++ b/internal/source/disk/domain_test.go @@ -48,12 +48,14 @@ func testGroupServeHTTPHost(t *testing.T, host string) { require.HTTPBodyContains(t, serve, "GET", makeURL("/"), nil, "main-dir") require.HTTPBodyContains(t, serve, "GET", makeURL("/index"), nil, "main-dir") require.HTTPBodyContains(t, serve, "GET", makeURL("/index.html"), nil, "main-dir") - testhelpers.AssertRedirectTo(t, serve, "GET", makeURL("/project"), nil, "//"+host+"/project/") + // testhelpers.AssertRedirectTo(t, serve, "GET", makeURL("/project"), nil, "//"+host+"/project/") + require.HTTPBodyContains(t, serve, "GET", makeURL("/project"), nil, "project-subdir") require.HTTPBodyContains(t, serve, "GET", makeURL("/project/"), nil, "project-subdir") require.HTTPBodyContains(t, serve, "GET", makeURL("/project/index"), nil, "project-subdir") require.HTTPBodyContains(t, serve, "GET", makeURL("/project/index/"), nil, "project-subdir") require.HTTPBodyContains(t, serve, "GET", makeURL("/project/index.html"), nil, "project-subdir") - testhelpers.AssertRedirectTo(t, serve, "GET", makeURL("/project/subdir"), nil, "//"+host+"/project/subdir/") + // testhelpers.AssertRedirectTo(t, serve, "GET", makeURL("/project/subdir"), nil, "//"+host+"/project/subdir/") + require.HTTPBodyContains(t, serve, "GET", makeURL("/project/subdir"), nil, "project-subsubdir") require.HTTPBodyContains(t, serve, "GET", makeURL("/project/subdir/"), nil, "project-subsubdir") require.HTTPBodyContains(t, serve, "GET", makeURL("/project2/"), nil, "project2-main") require.HTTPBodyContains(t, serve, "GET", makeURL("/project2/index"), nil, "project2-main") @@ -90,9 +92,8 @@ func TestDomainServeHTTP(t *testing.T) { require.HTTPBodyContains(t, serveFileOrNotFound(testDomain), "GET", "/", nil, "project2-main") require.HTTPBodyContains(t, serveFileOrNotFound(testDomain), "GET", "/index.html", nil, "project2-main") - require.HTTPRedirect(t, serveFileOrNotFound(testDomain), "GET", "/subdir", nil) - require.HTTPBodyContains(t, serveFileOrNotFound(testDomain), "GET", "/subdir", nil, - `Found`) + // require.HTTPRedirect(t, serveFileOrNotFound(testDomain), "GET", "/subdir", nil) + require.HTTPBodyContains(t, serveFileOrNotFound(testDomain), "GET", "/subdir", nil, "project2-subdir") require.HTTPBodyContains(t, serveFileOrNotFound(testDomain), "GET", "/subdir/", nil, "project2-subdir") require.HTTPBodyContains(t, serveFileOrNotFound(testDomain), "GET", "/subdir/index.html", nil, "project2-subdir") require.HTTPError(t, serveFileOrNotFound(testDomain), "GET", "//about.gitlab.com/%2e%2e", nil) @@ -268,8 +269,10 @@ func TestGroupServeHTTPGzip(t *testing.T) { } for _, tt := range testSet { - URL := "http://group.test.io" + tt.url - testHTTPGzip(t, serveFileOrNotFound(testGroup), tt.mode, URL, nil, tt.acceptEncoding, tt.body, tt.contentType, tt.ungzip) + t.Run(tt.url, func(t *testing.T) { + URL := "http://group.test.io" + tt.url + testHTTPGzip(t, serveFileOrNotFound(testGroup), tt.mode, URL, nil, tt.acceptEncoding, tt.body, tt.contentType, tt.ungzip) + }) } } -- GitLab