diff options
author | Krasimir Angelov <kangelov@gitlab.com> | 2019-12-11 07:59:37 +0300 |
---|---|---|
committer | Krasimir Angelov <kangelov@gitlab.com> | 2019-12-12 11:49:46 +0300 |
commit | a0fe1ec1de472439430679330b374e1cbe60652e (patch) | |
tree | bb4143741c6fcdf244865895447167cd4fab9352 | |
parent | 860072e9807e8ab8ce6b213f4f72f42d91c1ad70 (diff) |
Fix different issues with slashes
Update code and tests to handle the fact thet API will always return
prefix surrounded with slashes (e.g. `/prefix/`) and source.path with
trailing slash (e.g. `path/to/public/`).
-rw-r--r-- | internal/serving/disk/reader.go | 5 | ||||
-rw-r--r-- | internal/source/disk/domain_test.go | 2 | ||||
-rw-r--r-- | internal/source/disk/group.go | 2 | ||||
-rw-r--r-- | internal/source/gitlab/client/testdata/test.gitlab.io.json | 10 | ||||
-rw-r--r-- | internal/source/gitlab/gitlab.go | 14 | ||||
-rw-r--r-- | internal/source/gitlab/gitlab_test.go | 35 | ||||
-rw-r--r-- | shared/lookups/new-source-test.gitlab.io.json | 4 |
7 files changed, 53 insertions, 19 deletions
diff --git a/internal/serving/disk/reader.go b/internal/serving/disk/reader.go index ce4f1d8b..533c6d4e 100644 --- a/internal/serving/disk/reader.go +++ b/internal/serving/disk/reader.go @@ -72,8 +72,9 @@ func (reader *Reader) resolvePath(publicPath string, subPath ...string) (string, // 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 +86,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 7977e35a..d4a6998c 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 ad5144f5..0b6488fe 100644 --- a/internal/source/gitlab/gitlab_test.go +++ b/internal/source/gitlab/gitlab_test.go @@ -36,19 +36,46 @@ func TestResolve(t *testing.T) { client := client.StubClient{File: "client/testdata/test.gitlab.io.json"} source := Gitlab{client: client, cache: cache.New()} - 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) @@ -68,7 +95,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" } } |