Merge pull request git-lfs#6252 from chrisd8088/fix-standalone-adapter-crash

Fix standalone adapter crash and ignore invalid custom adapter names

Author: chrisd8088

commands/command_migrate_export.go

@@ -121,7 +121,7 @@ func migrateExportCommand(cmd *cobra.Command, args []string) {
 
 	// If we have a valid remote, pre-download all objects using the Transfer Queue
 	if remoteURL != "" {
-		q := newDownloadQueue(getTransferManifestOperationRemote("Download", remote), remote)
+		q := newDownloadQueue(getTransferManifestOperationRemote("download", remote), remote)
 		gs := lfs.NewGitScanner(cfg, func(p *lfs.WrappedPointer, err error) {
 			if err != nil {
 				return

docs/custom-transfers.md

@@ -37,7 +37,8 @@ without querying the server, by using the following config option:
   configure a custom transfer agent for individual remotes.
   `lfs.standalonetransferagent` unconditionally configures a custom transfer
   agent for all remotes.  The custom transfer agent must be specified in
-  a `lfs.customtransfer.<name>` settings group.
+  a `lfs.customtransfer.<name>` settings group, or else must be the internal
+  `lfs-standalone-file` transfer adapter.  Other values will be ignored.
 
 ## Defining a Custom Transfer Type
 

docs/man/git-lfs-config.adoc

@@ -158,8 +158,13 @@ removed, and tus.io uploads will be available for all clients.
 +
 Allows the specified custom transfer agent to be used directly for
 transferring files, without asking the server how the transfers should
-be made. The custom transfer agent has to be defined in a
-`lfs.customtransfer.<name>` settings group.
+be made. The custom transfer agent must be defined in a
+`lfs.customtransfer.<name>` settings group, or else must be the internal
+`lfs-standalone-file` transfer adapter. Other values will be ignored.
++
+See git-lfs-standalone-file(1) for details of the internal
+`lfs-standalone-file` custom transfer adapter, which is intended for
+use with local `file:///` URLs.
 * `lfs.customtransfer.<name>.path`
 +
 `lfs.customtransfer.<name>` is a settings group which defines a custom
@@ -169,13 +174,23 @@ should point to the process you wish to invoke. The protocol between the
 git-lfs client and the custom transfer process is documented at
 https://github.com/git-lfs/git-lfs/blob/main/docs/custom-transfers.md
 +
-The `<name>` must be a unique identifier that the LFS server understands. When
+The `<name>` must be a unique identifier that the LFS server understands
+and which does not conflict with the name of any pre-defined internal
+transfer adapter in the client such as "basic" or "ssh", as these will
+always override a custom transfer adapter with the same name. When
 calling the LFS API the client will include a list of supported transfer
 types. If the server also supports this named transfer type, it will
 select it and actions returned from the API will be in relation to that
 transfer type (may not be traditional URLs for example). Only if the
 server accepts as a transfer it supports will this custom transfer
 process be invoked.
++
+If multiple `lfs.customtransfer.<name>` settings are defined, they
+must all be contained within the same configuration sub-section,
+meaning that the sub-section portion of their names must be identical.
+For instance, if `lfs.customTransfer.myAdapter.path` is defined,
+then `lfs.customTransfer.myAdapter.args` will be accepted, while
+`lfs.customtransfer.myadapter.args` will not.
 * `lfs.customtransfer.<name>.args`
 +
 If the custom transfer process requires any arguments, these can be
@@ -469,6 +484,6 @@ The set of keys allowed in this file is restricted for security reasons.
 
 == SEE ALSO
 
-git-config(1), git-lfs-install(1), gitattributes(5), gitignore(5).
+git-config(1), git-lfs-install(1), git-lfs-standalone-file(1), gitattributes(5), gitignore(5).
 
 Part of the git-lfs(1) suite.

t/t-custom-transfers.sh

@@ -285,3 +285,302 @@ begin_test "custom-transfer-standalone-urlmatch"
   git lfs fsck
 )
 end_test
+
+begin_test "standalone agent without custom group (file transfers allowed)"
+(
+  set -e
+
+  reponame="custom-transfer-standalone-valid-file"
+  setup_remote_repo "$reponame"
+
+  # Clone directly, not through lfstest-gitserver.
+  clone_repo_url "$REMOTEDIR/$reponame.git" "$reponame"
+
+  # When the remote has a file:// URL, we permit the standalone transfer
+  # agent to be set to the internal "lfs-standalone-file" adapter without
+  # any "lfs.customtransfer.*" settings.
+  git config lfs.standaloneTransferAgent lfs-standalone-file
+
+  git lfs track "*.bin"
+
+  contents="a"
+  contents_oid="$(calc_oid "$contents")"
+  printf "%s" "$contents" >a.bin
+
+  git add .gitattributes a.bin
+  git commit -m "initial commit"
+
+  GIT_TRACE=1 git push origin main 2>&1 | tee push.log
+  [ 0 -eq "${PIPESTATUS[0]}" ]
+
+  assert_remote_object "$reponame" "$contents_oid" "${#contents}"
+
+  rm -rf .git/lfs/objects
+
+  GIT_TRACE=1 git lfs pull 2>&1 | tee pull.log
+  [ 0 -eq "${PIPESTATUS[0]}" ]
+
+  [ 0 -eq "$(grep -c "not a registered custom transfer adapter" pull.log)" ]
+
+  assert_local_object "$contents_oid" "${#contents}"
+)
+end_test
+
+begin_test "standalone agent without custom group (file transfers disallowed)"
+(
+  set -e
+
+  reponame="custom-transfer-standalone-invalid-file"
+  setup_remote_repo "$reponame"
+  clone_repo "$reponame" "$reponame"
+
+  git lfs track "*.bin"
+
+  contents="a"
+  contents_oid="$(calc_oid "$contents")"
+  printf "%s" "$contents" >a.bin
+
+  git add .gitattributes a.bin
+  git commit -m "initial commit"
+
+  git push origin main
+
+  # When the remote has an HTTP URL, we expect an error if the standalone
+  # transfer agent is set to the internal "lfs-standalone-file" adapter.
+  git config lfs.standaloneTransferAgent lfs-standalone-file
+
+  rm -rf .git/lfs/objects
+
+  GIT_TRACE=1 git lfs pull 2>&1 | tee pull.log
+  [ 0 -ne "${PIPESTATUS[0]}" ]
+
+  grep "error initializing custom adapter \"lfs-standalone-file\"" pull.log
+  grep "error creating handler: no valid file:// URLs found" pull.log
+  grep "Failed to fetch some objects" pull.log
+
+  [ 0 -eq "$(grep -c "not a registered custom transfer adapter" pull.log)" ]
+
+  refute_local_object "$contents_oid"
+)
+end_test
+
+begin_test "standalone agent without custom group ignored (basic transfers)"
+(
+  set -e
+
+  reponame="custom-transfer-standalone-ignored-basic"
+  setup_remote_repo "$reponame"
+  clone_repo "$reponame" "$reponame"
+
+  # The standalone transfer agent should be ignored if it is set to the
+  # internal "basic" adapter, regardless of whether that adapter is
+  # actually used for HTTP requests to the Batch API.
+  git config lfs.standaloneTransferAgent basic
+
+  git lfs track "*.bin"
+
+  contents="a"
+  contents_oid="$(calc_oid "$contents")"
+  printf "%s" "$contents" >a.bin
+
+  git add .gitattributes a.bin
+  git commit -m "initial commit"
+
+  GIT_TRACE=1 git push origin main 2>&1 | tee push.log
+  [ 0 -eq "${PIPESTATUS[0]}" ]
+
+  grep "standalone agent \"basic\" is not a registered custom transfer adapter; ignoring" push.log
+
+  assert_server_object "$reponame" "$contents_oid"
+
+  rm -rf .git/lfs/objects
+
+  GIT_TRACE=1 git lfs pull 2>&1 | tee pull.log
+  [ 0 -eq "${PIPESTATUS[0]}" ]
+
+  grep "standalone agent \"basic\" is not a registered custom transfer adapter; ignoring" pull.log
+
+  assert_local_object "$contents_oid" "${#contents}"
+
+  # The standalone transfer agent should still be ignored even when
+  # a custom transfer adapter is registered with the same name as the
+  # internal "basic" adapter.
+  git config lfs.customTransfer.basic.path path-to-nothing
+
+  contents="b"
+  contents_oid="$(calc_oid "$contents")"
+  printf "%s" "$contents" >b.bin
+
+  git add b.bin
+  git commit -m "second commit"
+
+  GIT_TRACE=1 git push origin main 2>&1 | tee push.log
+  [ 0 -eq "${PIPESTATUS[0]}" ]
+
+  grep "standalone agent \"basic\" is not a registered custom transfer adapter; ignoring" push.log
+  grep "custom upload transfer adapter \"basic\" ignored due to conflict with standard adapter" push.log
+  grep "custom download transfer adapter \"basic\" ignored due to conflict with standard adapter" push.log
+
+  assert_server_object "$reponame" "$contents_oid"
+
+  rm -rf .git/lfs/objects
+
+  GIT_TRACE=1 git lfs pull 2>&1 | tee pull.log
+  [ 0 -eq "${PIPESTATUS[0]}" ]
+
+  grep "standalone agent \"basic\" is not a registered custom transfer adapter; ignoring" pull.log
+  grep "custom upload transfer adapter \"basic\" ignored due to conflict with standard adapter" pull.log
+  grep "custom download transfer adapter \"basic\" ignored due to conflict with standard adapter" pull.log
+
+  assert_local_object "$contents_oid" "${#contents}"
+)
+end_test
+
+begin_test "standalone agent without custom group ignored (basic transfers not ssh)"
+(
+  set -e
+
+  reponame="custom-transfer-standalone-ignored-basic-ssh"
+  setup_remote_repo "$reponame"
+  clone_repo "$reponame" "$reponame"
+
+  # The standalone transfer agent should be ignored if it is set to the
+  # internal "ssh" adapter, regardless of whether the internal "basic"
+  # adapter will actually be used for HTTP requests to the Batch API.
+  git config lfs.standaloneTransferAgent ssh
+
+  git lfs track "*.bin"
+
+  contents="a"
+  contents_oid="$(calc_oid "$contents")"
+  printf "%s" "$contents" >a.bin
+
+  git add .gitattributes a.bin
+  git commit -m "initial commit"
+
+  GIT_TRACE=1 git push origin main 2>&1 | tee push.log
+  [ 0 -eq "${PIPESTATUS[0]}" ]
+
+  grep "standalone agent \"ssh\" is not a registered custom transfer adapter; ignoring" push.log
+
+  assert_server_object "$reponame" "$contents_oid"
+
+  rm -rf .git/lfs/objects
+
+  GIT_TRACE=1 git lfs pull 2>&1 | tee pull.log
+  [ 0 -eq "${PIPESTATUS[0]}" ]
+
+  grep "standalone agent \"ssh\" is not a registered custom transfer adapter; ignoring" pull.log
+
+  assert_local_object "$contents_oid" "${#contents}"
+
+  # The standalone transfer agent should still be ignored even when
+  # a custom transfer adapter is registered with the same name as the
+  # internal "ssh" adapter.
+  git config lfs.customTransfer.ssh.path path-to-nothing
+
+  contents="b"
+  contents_oid="$(calc_oid "$contents")"
+  printf "%s" "$contents" >b.bin
+
+  git add b.bin
+  git commit -m "second commit"
+
+  GIT_TRACE=1 git push origin main 2>&1 | tee push.log
+  [ 0 -eq "${PIPESTATUS[0]}" ]
+
+  grep "standalone agent \"ssh\" is not a registered custom transfer adapter; ignoring" push.log
+  grep "custom upload transfer adapter \"ssh\" ignored due to conflict with standard adapter" push.log
+  grep "custom download transfer adapter \"ssh\" ignored due to conflict with standard adapter" push.log
+
+  assert_server_object "$reponame" "$contents_oid"
+
+  rm -rf .git/lfs/objects
+
+  GIT_TRACE=1 git lfs pull 2>&1 | tee pull.log
+  [ 0 -eq "${PIPESTATUS[0]}" ]
+
+  grep "standalone agent \"ssh\" is not a registered custom transfer adapter; ignoring" pull.log
+  grep "custom upload transfer adapter \"ssh\" ignored due to conflict with standard adapter" pull.log
+  grep "custom download transfer adapter \"ssh\" ignored due to conflict with standard adapter" pull.log
+
+  assert_local_object "$contents_oid" "${#contents}"
+)
+end_test
+
+begin_test "standalone agent without custom group ignored (ssh transfers)"
+(
+  set -e
+
+  setup_pure_ssh
+
+  reponame="custom-transfer-standalone-ignored-ssh"
+  setup_remote_repo "$reponame"
+  clone_repo "$reponame" "$reponame"
+
+  sshurl=$(ssh_remote "$reponame")
+  git config lfs.url "$sshurl"
+
+  # The standalone transfer agent should be ignored if it is set to the
+  # internal "ssh" adapter, regardless of whether that adapter is
+  # actually used for SSH requests to the Batch API.
+  git config lfs.standaloneTransferAgent ssh
+
+  git lfs track "*.bin"
+
+  contents="a"
+  contents_oid="$(calc_oid "$contents")"
+  printf "%s" "$contents" >a.bin
+
+  git add .gitattributes a.bin
+  git commit -m "initial commit"
+
+  GIT_TRACE=1 git push origin main 2>&1 | tee push.log
+  [ 0 -eq "${PIPESTATUS[0]}" ]
+
+  grep "standalone agent \"ssh\" is not a registered custom transfer adapter; ignoring" push.log
+
+  assert_remote_object "$reponame" "$contents_oid" "${#contents}"
+
+  rm -rf .git/lfs/objects
+
+  GIT_TRACE=1 git lfs pull 2>&1 | tee pull.log
+  [ 0 -eq "${PIPESTATUS[0]}" ]
+
+  grep "standalone agent \"ssh\" is not a registered custom transfer adapter; ignoring" pull.log
+
+  assert_local_object "$contents_oid" "${#contents}"
+
+  # The standalone transfer agent should still be ignored even when
+  # a custom transfer adapter is registered with the same name as the
+  # internal "ssh" adapter.
+  git config lfs.customTransfer.ssh.path path-to-nothing
+
+  contents="b"
+  contents_oid="$(calc_oid "$contents")"
+  printf "%s" "$contents" >b.bin
+
+  git add b.bin
+  git commit -m "second commit"
+
+  GIT_TRACE=1 git push origin main 2>&1 | tee push.log
+  [ 0 -eq "${PIPESTATUS[0]}" ]
+
+  grep "standalone agent \"ssh\" is not a registered custom transfer adapter; ignoring" push.log
+  grep "custom upload transfer adapter \"ssh\" ignored due to conflict with standard adapter" push.log
+  grep "custom download transfer adapter \"ssh\" ignored due to conflict with standard adapter" push.log
+
+  assert_remote_object "$reponame" "$contents_oid" "${#contents}"
+
+  rm -rf .git/lfs/objects
+
+  GIT_TRACE=1 git lfs pull 2>&1 | tee pull.log
+  [ 0 -eq "${PIPESTATUS[0]}" ]
+
+  grep "standalone agent \"ssh\" is not a registered custom transfer adapter; ignoring" pull.log
+  grep "custom upload transfer adapter \"ssh\" ignored due to conflict with standard adapter" pull.log
+  grep "custom download transfer adapter \"ssh\" ignored due to conflict with standard adapter" pull.log
+
+  assert_local_object "$contents_oid" "${#contents}"
+)
+end_test

tq/basic_download.go

@@ -327,7 +327,7 @@ func (a *basicDownloadAdapter) download(context *basicDownloadAdapterWorkerConte
 }
 
 func configureBasicDownloadAdapter(m *concreteManifest) {
-	m.RegisterNewAdapterFunc(BasicAdapterName, Download, func(name string, dir Direction) Adapter {
+	m.RegisterNewAdapterFunc(BasicAdapterName, Download, false, func(name string, dir Direction) Adapter {
 		switch dir {
 		case Download:
 			bd := &basicDownloadAdapter{newAdapterBase(m.fs, name, dir, nil)}

tq/basic_upload.go

@@ -211,7 +211,7 @@ func newStartCallbackReader(r lfsapi.ReadSeekCloser, cb func() error) *startCall
 }
 
 func configureBasicUploadAdapter(m *concreteManifest) {
-	m.RegisterNewAdapterFunc(BasicAdapterName, Upload, func(name string, dir Direction) Adapter {
+	m.RegisterNewAdapterFunc(BasicAdapterName, Upload, false, func(name string, dir Direction) Adapter {
 		switch dir {
 		case Upload:
 			bu := &basicUploadAdapter{newAdapterBase(m.fs, name, dir, nil)}