diff options
author | Sami Hiltunen <shiltunen@gitlab.com> | 2021-01-20 18:36:17 +0300 |
---|---|---|
committer | Sami Hiltunen <shiltunen@gitlab.com> | 2021-01-20 19:18:08 +0300 |
commit | 6a7ee4ed5fa77b4d7017b76a7d87b40875cc1590 (patch) | |
tree | accab56a4984af176695146fe4095d39543f4d21 | |
parent | 45611853ebf92819dcbff1b62e2f8c341c04ae8c (diff) |
fix path normalization deviation in UserCommitFiles Go portsmh-fix-user-commit-files-start-branch
The Ruby implementation's path normalization accepts double slashes
in the path as is. This later causes Rugged to error out when attempting
to add a file with such an invalid path to the index.
The Go port is cleaning the filepaths, which replaces double slashes
with single slashes. This makes the filepath valid and thus adding the
file to the index didn't error out.
This commit a special cases double slashes to workaround this quirk.
-rw-r--r-- | internal/gitaly/service/operations/commit_files.go | 12 | ||||
-rw-r--r-- | internal/gitaly/service/operations/commit_files_test.go | 12 |
2 files changed, 24 insertions, 0 deletions
diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go index 51ff91481..65cfeacea 100644 --- a/internal/gitaly/service/operations/commit_files.go +++ b/internal/gitaly/service/operations/commit_files.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + "strings" "time" "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" @@ -141,6 +142,17 @@ func (s *Server) UserCommitFiles(stream gitalypb.OperationService_UserCommitFile func validatePath(rootPath, relPath string) (string, error) { if relPath == "" { return "", indexError("You must provide a file path") + } else if strings.Contains(relPath, "//") { + // This is a workaround to address a quirk in porting the RPC from Ruby to Go. + // GitLab's QA pipeline runs tests with filepath 'invalid://file/name/here'. + // Go's filepath.Clean returns 'invalid:/file/name/here'. The Ruby implementation's + // filepath normalization accepted the path as is. Adding a file with this path to the + // index via Rugged failed with an invalid path error. As Go's cleaning resulted a valid + // filepath, adding the file succeeded, which made the QA pipeline's specs fail. + // + // The Rails code expects to receive an error prefixed with 'invalid path', which is done + // here to retain compatibility. + return "", indexError(fmt.Sprintf("invalid path: '%s'", relPath)) } path, err := storage.ValidateRelativePath(rootPath, relPath) diff --git a/internal/gitaly/service/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go index ed955b115..44b29a200 100644 --- a/internal/gitaly/service/operations/commit_files_test.go +++ b/internal/gitaly/service/operations/commit_files_test.go @@ -218,6 +218,18 @@ func testUserCommitFiles(t *testing.T, ctx context.Context) { }, }, { + desc: "create file with double slash", + steps: []step{ + { + actions: []*gitalypb.UserCommitFilesRequest{ + createFileHeaderRequest("invalid://file/name/here"), + actionContentRequest("content-1"), + }, + indexError: "invalid path: 'invalid://file/name/here'", + }, + }, + }, + { desc: "create file without content", steps: []step{ { |