diff --git a/cmd/gitannex/gitannex.go b/cmd/gitannex/gitannex.go index e716cfe34..1bbef31d7 100644 --- a/cmd/gitannex/gitannex.go +++ b/cmd/gitannex/gitannex.go @@ -35,6 +35,7 @@ import ( "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/cache" "github.com/rclone/rclone/fs/config" + "github.com/rclone/rclone/fs/fspath" "github.com/rclone/rclone/fs/operations" "github.com/spf13/cobra" ) @@ -262,38 +263,58 @@ func (s *server) run() error { } // Idempotently handle an incoming INITREMOTE message. This should perform -// one-time setup operations, but we may receive the command again, e.g. when -// this git-annex remote is initialized in a different repository. +// one-time setup operations for the remote, such as validating or rejecting +// config values. We may receive the INITREMOTE message again in later sessions, +// e.g. when the same git-annex remote is initialized in a different repository. +// However, we are *not* guaranteed to receive the INITREMOTE message once per +// session, so do not mutate state here and expect it to always be available in +// other handler functions. func (s *server) handleInitRemote() error { if err := s.queryConfigs(); err != nil { return fmt.Errorf("failed to get configs: %w", err) } - // Explicitly check that a remote with the given name exists. If we just - // relied on `cache.Get()` to return `fs.ErrorNotFoundInConfigFile`, this - // function would incorrectly succeed when the given remote name is actually - // a file path. + // Explicitly check whether [server.configRcloneRemoteName] names a remote. // - // The :local: backend does not correspond to a remote named by the rclone - // config, but is permitted to enable testing. Technically, a user might hit - // this code path, but it would be a strange choice because git-annex - // natively supports a "directory" special remote. - + // - We do not permit file paths in the remote name; that's what + // [s.configPrefix] is for. If we simply checked whether [cache.Get] + // returns [fs.ErrorNotFoundInConfigFile], we would incorrectly identify + // file names as valid remote names. + // + // - In order to support remotes defined by environment variables, we must + // use [config.GetRemoteNames] instead of [config.FileSections]. trimmedName := strings.TrimSuffix(s.configRcloneRemoteName, ":") - - if s.configRcloneRemoteName != ":local" { - var remoteExists bool - if slices.Contains(config.FileSections(), trimmedName) { - remoteExists = true - } - if !remoteExists { - s.sendMsg("INITREMOTE-FAILURE remote does not exist: " + s.configRcloneRemoteName) - return fmt.Errorf("remote does not exist: %s", s.configRcloneRemoteName) - } + if slices.Contains(config.GetRemoteNames(), trimmedName) { + s.sendMsg("INITREMOTE-SUCCESS") + return nil } - s.configRcloneRemoteName = trimmedName + ":" - + // Otherwise, check whether [server.configRcloneRemoteName] is actually a + // backend string such as ":local:". These are not remote names, per se, but + // they are permitted for compatibility with [fstest]. We could guard this + // behavior behind [testing.Testing] to prevent users from specifying + // backend strings, but there's no obvious harm in permitting it. + maybeBackend := strings.HasPrefix(s.configRcloneRemoteName, ":") + if !maybeBackend { + s.sendMsg("INITREMOTE-FAILURE remote does not exist: " + s.configRcloneRemoteName) + return fmt.Errorf("remote does not exist: %s", s.configRcloneRemoteName) + } + parsed, err := fspath.Parse(s.configRcloneRemoteName) + if err != nil { + s.sendMsg("INITREMOTE-FAILURE remote could not be parsed as a backend: " + s.configRcloneRemoteName) + return fmt.Errorf("remote could not be parsed as a backend: %s", s.configRcloneRemoteName) + } + if parsed.Path != "" { + s.sendMsg("INITREMOTE-FAILURE backend must not have a path: " + s.configRcloneRemoteName) + return fmt.Errorf("backend must not have a path: %s", s.configRcloneRemoteName) + } + // Strip the leading colon and options before searching for the backend, + // i.e. search for "local" instead of ":local,description=hello:/tmp/foo". + trimmedBackendName := strings.TrimPrefix(parsed.Name, ":") + if _, err = fs.Find(trimmedBackendName); err != nil { + s.sendMsg("INITREMOTE-FAILURE backend does not exist: " + trimmedBackendName) + return fmt.Errorf("backend does not exist: %s", trimmedBackendName) + } s.sendMsg("INITREMOTE-SUCCESS") return nil } diff --git a/cmd/gitannex/gitannex_test.go b/cmd/gitannex/gitannex_test.go index 5aaa74620..ed6cd2d5f 100644 --- a/cmd/gitannex/gitannex_test.go +++ b/cmd/gitannex/gitannex_test.go @@ -534,6 +534,127 @@ var fstestTestCases = []testCase{ }, expectedError: "remote does not exist:", }, + { + label: "HandlesPrepareWithNonexistentBackendAsRemote", + testProtocolFunc: func(t *testing.T, h *testState) { + h.requireReadLineExact("VERSION 1") + h.requireWriteLine("PREPARE") + h.requireReadLineExact("GETCONFIG rcloneremotename") + h.requireWriteLine("VALUE :nonexistentBackend:") + h.requireReadLineExact("GETCONFIG rcloneprefix") + h.requireWriteLine("VALUE /foo") + h.requireReadLineExact("GETCONFIG rclonelayout") + h.requireWriteLine("VALUE foo") + h.requireReadLineExact("PREPARE-SUCCESS") + + require.Equal(t, ":nonexistentBackend:", h.server.configRcloneRemoteName) + require.Equal(t, "/foo", h.server.configPrefix) + require.True(t, h.server.configsDone) + + h.requireWriteLine("INITREMOTE") + h.requireReadLineExact("INITREMOTE-FAILURE backend does not exist: nonexistentBackend") + + require.NoError(t, h.mockStdinW.Close()) + }, + expectedError: "backend does not exist:", + }, + { + label: "HandlesPrepareWithBackendAsRemote", + testProtocolFunc: func(t *testing.T, h *testState) { + h.requireReadLineExact("VERSION 1") + h.requireWriteLine("PREPARE") + h.requireReadLineExact("GETCONFIG rcloneremotename") + h.requireWriteLine("VALUE :local:") + h.requireReadLineExact("GETCONFIG rcloneprefix") + h.requireWriteLine("VALUE /foo") + h.requireReadLineExact("GETCONFIG rclonelayout") + h.requireWriteLine("VALUE foo") + h.requireReadLineExact("PREPARE-SUCCESS") + + require.Equal(t, ":local:", h.server.configRcloneRemoteName) + require.Equal(t, "/foo", h.server.configPrefix) + require.True(t, h.server.configsDone) + + h.requireWriteLine("INITREMOTE") + h.requireReadLineExact("INITREMOTE-SUCCESS") + + require.NoError(t, h.mockStdinW.Close()) + }, + }, + { + label: "HandlesPrepareWithBackendMissingTrailingColonAsRemote", + testProtocolFunc: func(t *testing.T, h *testState) { + h.requireReadLineExact("VERSION 1") + h.requireWriteLine("PREPARE") + h.requireReadLineExact("GETCONFIG rcloneremotename") + h.requireWriteLine("VALUE :local") + h.requireReadLineExact("GETCONFIG rcloneprefix") + h.requireWriteLine("VALUE /foo") + h.requireReadLineExact("GETCONFIG rclonelayout") + h.requireWriteLine("VALUE foo") + h.requireReadLineExact("PREPARE-SUCCESS") + + require.Equal(t, ":local", h.server.configRcloneRemoteName) + require.Equal(t, "/foo", h.server.configPrefix) + require.True(t, h.server.configsDone) + + h.requireWriteLine("INITREMOTE") + h.requireReadLineExact("INITREMOTE-FAILURE remote could not be parsed as a backend: :local") + + require.NoError(t, h.mockStdinW.Close()) + }, + expectedError: "remote could not be parsed as a backend:", + }, + { + label: "HandlesPrepareWithBackendContainingOptionsAsRemote", + testProtocolFunc: func(t *testing.T, h *testState) { + h.requireReadLineExact("VERSION 1") + h.requireWriteLine("PREPARE") + h.requireReadLineExact("GETCONFIG rcloneremotename") + h.requireWriteLine("VALUE :local,description=banana:") + h.requireReadLineExact("GETCONFIG rcloneprefix") + h.requireWriteLine("VALUE /foo") + h.requireReadLineExact("GETCONFIG rclonelayout") + h.requireWriteLine("VALUE foo") + h.requireReadLineExact("PREPARE-SUCCESS") + + require.Equal(t, ":local,description=banana:", h.server.configRcloneRemoteName) + require.Equal(t, "/foo", h.server.configPrefix) + require.True(t, h.server.configsDone) + + h.requireWriteLine("INITREMOTE") + h.requireReadLineExact("INITREMOTE-SUCCESS") + + require.NoError(t, h.mockStdinW.Close()) + }, + }, + { + label: "HandlesPrepareWithBackendContainingOptionsAndIllegalPathAsRemote", + testProtocolFunc: func(t *testing.T, h *testState) { + h.requireReadLineExact("VERSION 1") + h.requireWriteLine("PREPARE") + h.requireReadLineExact("GETCONFIG rcloneremotename") + h.requireWriteLine("VALUE :local,description=banana:/bad/path") + h.requireReadLineExact("GETCONFIG rcloneprefix") + h.requireWriteLine("VALUE /foo") + h.requireReadLineExact("GETCONFIG rclonelayout") + h.requireWriteLine("VALUE foo") + h.requireReadLineExact("PREPARE-SUCCESS") + + require.Equal(t, ":local,description=banana:/bad/path", h.server.configRcloneRemoteName) + require.Equal(t, "/foo", h.server.configPrefix) + require.True(t, h.server.configsDone) + + h.requireWriteLine("INITREMOTE") + require.Regexp(t, + regexp.MustCompile("^INITREMOTE-FAILURE backend must not have a path: "), + h.requireReadLine(), + ) + + require.NoError(t, h.mockStdinW.Close()) + }, + expectedError: "backend must not have a path:", + }, { label: "HandlesPrepareWithSynonyms", testProtocolFunc: func(t *testing.T, h *testState) { @@ -1178,11 +1299,11 @@ func TestGitAnnexFstestBackendCases(t *testing.T) { require.NoError(t, err) // The gitannex command requires the `rcloneremotename` is the name - // of a remote or exactly ":local", so the empty string will not - // suffice. + // of a remote or a colon-prefixed backend name like ":local:", so + // the empty string will not suffice. if remoteName == "" { require.True(t, r.Fremote.Features().IsLocal) - remoteName = ":local" + remoteName = ":local:" } handle := makeTestState(t)