Merge branch 'fix-slash-issues' into 'master'

Fix different issues with slashes

See merge request gitlab-org/gitlab-pages!209

7 files changed, 56 insertions, 19 deletions

diff --git a/internal/serving/disk/reader.go b/internal/serving/disk/reader.go
index ce4f1d8b..0bc72646 100644
--- a/internal/serving/disk/reader.go
+++ b/internal/serving/disk/reader.go
@@ -69,11 +69,15 @@ 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, "/")
+	testPath := publicPath + strings.Join(subPath, "/")
 	fullPath, err := filepath.EvalSymlinks(testPath)
+
 	if err != nil {
 		if endsWithoutHTMLExtension(testPath) {
 			return "", &locationFileNoExtensionError{
@@ -85,7 +89,7 @@ func (reader *Reader) resolvePath(publicPath string, subPath ...string) (string,
 	}
 
 	// The requested path resolved to somewhere outside of the public/ directory
-	if !strings.HasPrefix(fullPath, publicPath+"/") && fullPath != publicPath {
+	if !strings.HasPrefix(fullPath, publicPath) && fullPath != filepath.Clean(publicPath) {
 		return "", fmt.Errorf("%q should be in %q", fullPath, publicPath)
 	}
 
diff --git a/internal/source/disk/domain_test.go b/internal/source/disk/domain_test.go
index ba4fb161..d114ff8a 100644
--- a/internal/source/disk/domain_test.go
+++ b/internal/source/disk/domain_test.go
@@ -303,7 +303,7 @@ func TestDomain404ServeHTTP(t *testing.T) {
 
 	testDomain := &domain.Domain{
 		Resolver: &customProjectResolver{
-			path:   "group.404/domain.404/public",
+			path:   "group.404/domain.404/public/",
 			config: &domainConfig{Domain: "domain.404.com"},
 		},
 	}
diff --git a/internal/source/disk/group.go b/internal/source/disk/group.go
index 7094e7a2..9f466bc4 100644
--- a/internal/source/disk/group.go
+++ b/internal/source/disk/group.go
@@ -86,7 +86,7 @@ func (g *Group) Resolve(r *http.Request) (*serving.LookupPath, string, error) {
 
 	lookupPath := &serving.LookupPath{
 		Prefix:             prefix,
-		Path:               filepath.Join(g.name, projectPath, "public"),
+		Path:               filepath.Join(g.name, projectPath, "public") + "/",
 		IsNamespaceProject: projectConfig.NamespaceProject,
 		IsHTTPSOnly:        projectConfig.HTTPSOnly,
 		HasAccessControl:   projectConfig.AccessControl,
diff --git a/internal/source/gitlab/client/testdata/test.gitlab.io.json b/internal/source/gitlab/client/testdata/test.gitlab.io.json
index 923c7344..e3430119 100644
--- a/internal/source/gitlab/client/testdata/test.gitlab.io.json
+++ b/internal/source/gitlab/client/testdata/test.gitlab.io.json
@@ -5,20 +5,20 @@
         {
             "access_control": false,
             "https_only": true,
-            "prefix": "/my/pages/project",
+            "prefix": "/my/pages/project/",
             "project_id": 123,
             "source": {
-                "path": "/some/path/to/project/",
+                "path": "some/path/to/project/",
                 "type": "file"
             }
         },
         {
             "access_control": false,
             "https_only": true,
-            "prefix": "/my/second-project",
+            "prefix": "/my/second-project/",
             "project_id": 124,
             "source": {
-                "path": "/some/path/to/project-2/",
+                "path": "some/path/to/project-2/",
                 "type": "file"
             }
         },
@@ -28,7 +28,7 @@
             "prefix": "/",
             "project_id": 125,
             "source": {
-                "path": "/some/path/to/project-3/",
+                "path": "some/path/to/project-3/",
                 "type": "file"
             }
         }
diff --git a/internal/source/gitlab/gitlab.go b/internal/source/gitlab/gitlab.go
index 5bb5427e..cce70733 100644
--- a/internal/source/gitlab/gitlab.go
+++ b/internal/source/gitlab/gitlab.go
@@ -65,10 +65,13 @@ func (g *Gitlab) Resolve(r *http.Request) (*serving.LookupPath, string, error) {
 		return nil, "", response.Error
 	}
 
+	urlPath := path.Clean(r.URL.Path)
+
 	for _, lookup := range response.Domain.LookupPaths {
-		urlPath := path.Clean(r.URL.Path)
+		isSubPath := strings.HasPrefix(urlPath, lookup.Prefix)
+		isRootPath := urlPath == path.Clean(lookup.Prefix)
 
-		if strings.HasPrefix(urlPath, lookup.Prefix) {
+		if isSubPath || isRootPath {
 			lookupPath := &serving.LookupPath{
 				Prefix:             lookup.Prefix,
 				Path:               strings.TrimPrefix(lookup.Source.Path, "/"),
@@ -78,9 +81,12 @@ func (g *Gitlab) Resolve(r *http.Request) (*serving.LookupPath, string, error) {
 				ProjectID:          uint64(lookup.ProjectID),
 			}
 
-			requestPath := strings.TrimPrefix(urlPath, lookup.Prefix)
+			subPath := ""
+			if isSubPath {
+				subPath = strings.TrimPrefix(urlPath, lookup.Prefix)
+			}
 
-			return lookupPath, strings.TrimPrefix(requestPath, "/"), nil
+			return lookupPath, subPath, nil
 		}
 	}
 
diff --git a/internal/source/gitlab/gitlab_test.go b/internal/source/gitlab/gitlab_test.go
index d6a81269..0e855f10 100644
--- a/internal/source/gitlab/gitlab_test.go
+++ b/internal/source/gitlab/gitlab_test.go
@@ -35,19 +35,46 @@ func TestResolve(t *testing.T) {
 	client := client.StubClient{File: "client/testdata/test.gitlab.io.json"}
 	source := Gitlab{client: client}
 
-	t.Run("when requesting a nested group project", func(t *testing.T) {
+	t.Run("when requesting nested group project with root path", func(t *testing.T) {
+		target := "https://test.gitlab.io:443/my/pages/project/"
+		request := httptest.NewRequest("GET", target, nil)
+
+		lookup, subpath, err := source.Resolve(request)
+		require.NoError(t, err)
+
+		require.Equal(t, "/my/pages/project/", lookup.Prefix)
+		require.Equal(t, "some/path/to/project/", lookup.Path)
+		require.Equal(t, "", subpath)
+		require.False(t, lookup.IsNamespaceProject)
+	})
+
+	t.Run("when requesting a nested group project with full path", func(t *testing.T) {
 		target := "https://test.gitlab.io:443/my/pages/project/path/index.html"
 		request := httptest.NewRequest("GET", target, nil)
 
 		lookup, subpath, err := source.Resolve(request)
 		require.NoError(t, err)
 
-		require.Equal(t, "/my/pages/project", lookup.Prefix)
+		require.Equal(t, "/my/pages/project/", lookup.Prefix)
+		require.Equal(t, "some/path/to/project/", lookup.Path)
 		require.Equal(t, "path/index.html", subpath)
 		require.False(t, lookup.IsNamespaceProject)
 	})
 
-	t.Run("when request a nested group project", func(t *testing.T) {
+	t.Run("when requesting the group root project with root path", func(t *testing.T) {
+		target := "https://test.gitlab.io:443/"
+		request := httptest.NewRequest("GET", target, nil)
+
+		lookup, subpath, err := source.Resolve(request)
+		require.NoError(t, err)
+
+		require.Equal(t, "/", lookup.Prefix)
+		require.Equal(t, "some/path/to/project-3/", lookup.Path)
+		require.Equal(t, "", subpath)
+		require.True(t, lookup.IsNamespaceProject)
+	})
+
+	t.Run("when requesting the group root project with full path", func(t *testing.T) {
 		target := "https://test.gitlab.io:443/path/to/index.html"
 		request := httptest.NewRequest("GET", target, nil)
 
@@ -67,7 +94,7 @@ func TestResolve(t *testing.T) {
 		lookup, subpath, err := source.Resolve(request)
 		require.NoError(t, err)
 
-		require.Equal(t, "/my/pages/project", lookup.Prefix)
+		require.Equal(t, "/my/pages/project/", lookup.Prefix)
 		require.Equal(t, "index.html", subpath)
 	})
 }
diff --git a/shared/lookups/new-source-test.gitlab.io.json b/shared/lookups/new-source-test.gitlab.io.json
index 0332b6c2..f84fde35 100644
--- a/shared/lookups/new-source-test.gitlab.io.json
+++ b/shared/lookups/new-source-test.gitlab.io.json
@@ -5,10 +5,10 @@
         {
             "access_control": false,
             "https_only": false,
-            "prefix": "/my/pages/project",
+            "prefix": "/my/pages/project/",
             "project_id": 123,
             "source": {
-                "path": "/group/new-source-test.gitlab.io/public",
+                "path": "group/new-source-test.gitlab.io/public/",
                 "type": "file"
             }
         }