Package: git-annex
Version: 10.20230126-2
Severity: important

git-annex has this bug, which can result in data loss when using an 
external special remote with exporttree=yes
https://git-annex.branchable.com/bugs/external_remote_export_sent_to_wrong_process/

This is fixed in version 10.20230329.

I've attached a patch series that fixes this. Note that the second patch
is not strictly necessary, but it's all that's needed to support
VERSION 2, which some external special remote programs will now be
using to avoid being used with the buggy git-annex. So, I strongly
recommend including it.

(The patches are lightly trimmed versions of upstream commits 
02662f52920e84cd9464641ada84f6c3bbe3f86a and
18d326cb6f27912542f41fbb9525eefdbd553d09)

-- 
see shy jo
From 02662f52920e84cd9464641ada84f6c3bbe3f86a Mon Sep 17 00:00:00 2001
From: Joey Hess <jo...@joeyh.name>
Date: Tue, 28 Mar 2023 15:21:10 -0400
Subject: [PATCH 1/6] fix concurrency bug causing EXPORT to be sent to the
 wrong external

Fix bug that caused broken protocol to be used with external remotes that
use exporttree=yes. In some cases this could result in the wrong content
being exported to, or retrieved from the remote.

Sponsored-by: Nicholas Golder-Manning on Patreon

diff --git a/Remote/External.hs b/Remote/External.hs
index 4077030fa3..352b9eb68d 100644
--- a/Remote/External.hs
+++ b/Remote/External.hs
@@ -377,19 +377,27 @@ handleRequest external req mp responsehandler =
 		handleRequest' st external req mp responsehandler
 
 handleRequestKey :: External -> (SafeKey -> Request) -> Key -> Maybe MeterUpdate -> ResponseHandler a -> Annex a
-handleRequestKey external mkreq k mp responsehandler = case mkSafeKey k of
-	Right sk -> handleRequest external (mkreq sk) mp responsehandler
+handleRequestKey external mkreq k mp responsehandler = 
+	withSafeKey k $ \sk -> handleRequest external (mkreq sk) mp responsehandler
+
+withSafeKey :: Key -> (SafeKey -> Annex a) -> Annex a
+withSafeKey k a = case mkSafeKey k of
+	Right sk -> a sk
 	Left e -> giveup e
 
 {- Export location is first sent in an EXPORT message before
  - the main request. This is done because the ExportLocation can
  - contain spaces etc. -}
 handleRequestExport :: External -> ExportLocation -> (SafeKey -> Request) -> Key -> Maybe MeterUpdate -> ResponseHandler a -> Annex a
-handleRequestExport external loc mkreq k mp responsehandler = do
-	withExternalState external $ \st -> do
-		checkPrepared st external
-		sendMessage st (EXPORT loc)
-	handleRequestKey external mkreq k mp responsehandler
+handleRequestExport external loc mkreq k mp responsehandler = 
+	withSafeKey k $ \sk ->
+		-- Both the EXPORT and subsequent request must be sent to the
+		-- same external process, so run both with the same external
+		-- state.
+		withExternalState external $ \st -> do
+			checkPrepared st external
+			sendMessage st (EXPORT loc)
+			handleRequest' st external (mkreq sk) mp responsehandler
 
 handleRequest' :: ExternalState -> External -> Request -> Maybe MeterUpdate -> ResponseHandler a -> Annex a
 handleRequest' st external req mp responsehandler
-- 
2.39.1

From 18d326cb6f27912542f41fbb9525eefdbd553d09 Mon Sep 17 00:00:00 2001
From: Joey Hess <jo...@joeyh.name>
Date: Tue, 28 Mar 2023 17:00:08 -0400
Subject: [PATCH 4/6] external protocol VERSION 2

Support VERSION 2 in the external special remote protocol, which is
identical to VERSION 1, but avoids external remote programs neededing to
work around the above bug. External remote program that support
exporttree=yes are recommended to be updated to send VERSION 2.

Sponsored-by: Kevin Mueller on Patreon

diff --git a/Remote/External/Types.hs b/Remote/External/Types.hs
index 894f09dc30..633dc641bd 100644
--- a/Remote/External/Types.hs
+++ b/Remote/External/Types.hs
@@ -415,7 +415,7 @@ newtype JobId = JobId Integer
 	deriving (Eq, Ord, Show)
 
 supportedProtocolVersions :: [ProtocolVersion]
-supportedProtocolVersions = [1]
+supportedProtocolVersions = [1, 2]
 
 instance Proto.Serializable JobId where
 	serialize (JobId n) = show n
diff --git a/doc/design/external_special_remote_protocol.mdwn b/doc/design/external_special_remote_protocol.mdwn
index 3d2f0588ae..665687f5be 100644
--- a/doc/design/external_special_remote_protocol.mdwn
+++ b/doc/design/external_special_remote_protocol.mdwn
@@ -39,7 +39,7 @@ empty, but the separating spaces are still required in that case.
 The special remote is responsible for sending the first message, indicating
 the version of the protocol it is using.
 
-	VERSION 1
+	VERSION 2
 
 Recent versions of git-annex respond with a message indicating
 protocol extensions that it supports. Older versions of
@@ -271,7 +271,7 @@ These messages may be sent by the special remote at any time that it's
 handling a request.
 
 * `VERSION Int`  
-  Supported protocol version. Current version is 1. Must be sent first
+  Supported protocol version. Current version is 2. Must be sent first
   thing at startup, as until it sees this git-annex does not know how to
   talk with the special remote program!  
   (git-annex does not send a reply to this message, but may give up if it
@@ -428,6 +428,18 @@ remote.
   git-annex will not talk to it any further. If the program receives
   an ERROR from git-annex, it can exit with its own ERROR.
 
+## protocol versions
+
+Currently git-annex supports `VERSION 1` and `VERSION 2`.
+The two protocol versions are actually identical. 
+
+Old versions of git-annex that supported only `VERSION 1`
+had a bug in their implementation of the 
+part of the protocol documented in the [[export_and_import_appendix]].
+The bug could result in ontent being exported to the wrong file.
+External special remotes that implement that should use `VERSION 2` to
+avoid talking to the buggy old version of git-annex.
+
 ## extensions
 
 These protocol extensions are currently supported.
diff --git a/doc/special_remotes/external/example.sh b/doc/special_remotes/external/example.sh
index cfae74ae25..413065d1e0 100755
--- a/doc/special_remotes/external/example.sh
+++ b/doc/special_remotes/external/example.sh
@@ -150,7 +150,7 @@ doremove () {
 }
 
 # This has to come first, to get the protocol started.
-echo VERSION 1
+echo VERSION 2
 
 while read line; do
 	set -- $line
diff --git a/doc/special_remotes/external/git-annex-remote-ipfs b/doc/special_remotes/external/git-annex-remote-ipfs
index f255f9bc59..f546a1ae81 100755
--- a/doc/special_remotes/external/git-annex-remote-ipfs
+++ b/doc/special_remotes/external/git-annex-remote-ipfs
@@ -54,7 +54,7 @@ getaddrs () {
 }
 
 # This has to come first, to get the protocol started.
-echo VERSION 1
+echo VERSION 2
 
 while read line; do
 	set -- $line
diff --git a/doc/special_remotes/external/git-annex-remote-torrent b/doc/special_remotes/external/git-annex-remote-torrent
index a3b2bb8580..f41585b7c2 100755
--- a/doc/special_remotes/external/git-annex-remote-torrent
+++ b/doc/special_remotes/external/git-annex-remote-torrent
@@ -100,7 +100,7 @@ downloadtorrent () {
 }
 
 # This has to come first, to get the protocol started.
-echo VERSION 1
+echo VERSION 2
 
 while read line; do
 	set -- $line
-- 
2.39.1

Attachment: signature.asc
Description: PGP signature

Reply via email to