disable shellescape for rsync 3.2.4

rsync 3.2.4 broke backwards-compatability by preventing exposing filenames
to the shell. Made the rsync and gcrypt special remotes detect this and
disable shellescape.

An alternative fix would have been to always set RSYNC_OLD_ARGS=1.
Which would avoid the overhead of probing rsync --help for each affected
remote. But that is really very fast to run, and it seemed better to switch
to the modern code path rather than keeping on using the bad old code path.

Sponsored-by: Tobias Ammann on Patreon

CHANGELOG

@@ -11,6 +11,9 @@ git-annex (10.20220323) UNRELEASED; urgency=medium
   * Fix test failure on NFS when cleaning up gpg temp directory.
   * Fix a build failure with ghc 9.2.2.
     Thanks, gnezdo for the patch.
+  * rsync 3.2.4 broke backwards-compatability by preventing exposing
+    filenames to the shell. Made the rsync and gcrypt special remotes
+    detect this and disable shellescape. Closes: #1010397
 
  -- Joey Hess <id@joeyh.name>  Mon, 28 Mar 2022 14:46:10 -0400
 

Remote/GCrypt.hs

@@ -130,7 +130,8 @@ gen' r u c gc rs = do
 	cst <- remoteCost gc $
 		if repoCheap r then nearlyCheapRemoteCost else expensiveRemoteCost
 	let (rsynctransport, rsyncurl, accessmethod) = rsyncTransportToObjects r gc
-	let rsyncopts = Remote.Rsync.genRsyncOpts c gc rsynctransport rsyncurl
+	protectsargs <- liftIO Remote.Rsync.probeRsyncProtectsArgs
+	let rsyncopts = Remote.Rsync.genRsyncOpts protectsargs c gc rsynctransport rsyncurl
 	let this = Remote 
 		{ uuid = u
 		, cost = cst

Remote/Rsync.hs

@@ -16,7 +16,8 @@ module Remote.Rsync (
 	withRsyncScratchDir,
 	rsyncRemoteConfigs,
 	genRsyncOpts,
-	RsyncOpts
+	RsyncOpts,
+	probeRsyncProtectsArgs,
 ) where
 
 import Annex.Common
@@ -36,6 +37,7 @@ import Remote.Rsync.RsyncUrl
 import Crypto
 import Utility.Rsync
 import Utility.CopyFile
+import Utility.Process.Transcript
 import Messages.Progress
 import Utility.Metered
 import Types.Transfer
@@ -74,7 +76,8 @@ gen r u rc gc rs = do
 	cst <- remoteCost gc expensiveRemoteCost
 	(transport, url) <- rsyncTransport gc $
 		fromMaybe (giveup "missing rsyncurl") $ remoteAnnexRsyncUrl gc
-	let o = genRsyncOpts c gc transport url
+	protectsargs <- liftIO probeRsyncProtectsArgs
+	let o = genRsyncOpts protectsargs c gc transport url
 	let islocal = rsyncUrlIsPath $ rsyncUrl o
 	return $ Just $ specialRemote c
 		(fileStorer $ store o)
@@ -124,6 +127,18 @@ gen r u rc gc rs = do
 			, remoteStateHandle = rs
 			}
 
+-- | Since 3.2.4, rsync protects filenames from being exposed to the shell.
+newtype RsyncProtectsArgs = RsyncProtectsArgs Bool
+
+probeRsyncProtectsArgs :: IO RsyncProtectsArgs
+probeRsyncProtectsArgs = do
+	(helpoutput, _) <- processTranscript "rsync" ["--help"] Nothing
+	-- The --old-args option was added to disable the new arg
+	-- protection, so use it to detect when that feature is supported
+	-- by rsync, rather than parsing versions.
+	return (RsyncProtectsArgs $ "--old-args" `isInfixOf` helpoutput)
+
+
 -- Things used by genRsyncOpts
 rsyncRemoteConfigs :: [RemoteConfigFieldParser]
 rsyncRemoteConfigs = 
@@ -131,15 +146,17 @@ rsyncRemoteConfigs =
 		(FieldDesc "set to no to avoid usual shell escaping (not recommended)")
 	]
 
-genRsyncOpts :: ParsedRemoteConfig -> RemoteGitConfig -> Annex [CommandParam] -> RsyncUrl -> RsyncOpts
+genRsyncOpts :: RsyncProtectsArgs -> ParsedRemoteConfig -> RemoteGitConfig -> Annex [CommandParam] -> RsyncUrl -> RsyncOpts
-genRsyncOpts c gc transport url = RsyncOpts
+genRsyncOpts (RsyncProtectsArgs protectsargs) c gc transport url = RsyncOpts
 	{ rsyncUrl = url
 	, rsyncOptions = appendtransport $ opts []
 	, rsyncUploadOptions = appendtransport $
 		opts (remoteAnnexRsyncUploadOptions gc)
 	, rsyncDownloadOptions = appendtransport $
 		opts (remoteAnnexRsyncDownloadOptions gc)
-	, rsyncShellEscape = fromMaybe True (getRemoteConfigValue shellEscapeField c)
+	, rsyncShellEscape = if protectsargs
+		then False
+		else fromMaybe True (getRemoteConfigValue shellEscapeField c)
 	}
   where
 	appendtransport l = (++ l) <$> transport

doc/special_remotes/rsync.mdwn

@@ -26,13 +26,14 @@ These parameters can be passed to `git annex initremote` to configure rsync:
   by [[git-annex-export]]. It will not be usable as a general-purpose
   special remote.
 
-* `shellescape` - Optional. Set to "no" to avoid shell escaping normally
+* `shellescape` - Optional. This has no effect when using rsync 3.2.4 or
-  done when using rsync over ssh. That escaping is needed with typical
+  newer. Set to "no" to avoid shell escaping
-  setups, but not with some hosting providers that do not expose rsynced
+  normally done when using older versions of rsync over ssh. That escaping
-  filenames to the shell. You'll know you need this option if `git annex get`
+  is needed with typical setups, but not with some hosting providers that do
-  from the special remote fails with an error message containing a single
+  not expose rsynced filenames to the shell. You'll know you need this
-  quote (`'`) character. If that happens, you can run enableremote
+  option if `git annex get` from the special remote fails with an error
-  setting shellescape=no.
+  message containing a single quote (`'`) character. If that happens, you
+  can run enableremote setting shellescape=no.
 
 * `chunk` - Enables [[chunking]] when storing large files.  
   This is typically not a win for rsync, so no need to enable it.