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

diff --git a/CHANGELOG b/CHANGELOG
index 263da8e6eadb830379bbf3189c03e919b8c2f48a..6f53e3e1ac9dc6da42d56fba40b85b4c709ab8de 100644 (file)
--- a/CHANGELOG
+++ b/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
 

diff --git a/Remote/GCrypt.hs b/Remote/GCrypt.hs
index 9662e75d4ac2719cbb3f6f65c6876e0f36836eb4..0b120806adde17b49e0604ac7497f083afcd59bf 100644 (file)
--- a/Remote/GCrypt.hs
+++ b/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

diff --git a/Remote/Rsync.hs b/Remote/Rsync.hs
index 81d60311e4830d9703adc73ed5ca7e1f28e43368..e7e9ff740f5b42048108dace0dc1ffd4177b49b6 100644 (file)
--- a/Remote/Rsync.hs
+++ b/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 c gc transport url = RsyncOpts
+genRsyncOpts :: RsyncProtectsArgs -> ParsedRemoteConfig -> RemoteGitConfig -> Annex [CommandParam] -> RsyncUrl -> 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

diff --git a/doc/special_remotes/rsync.mdwn b/doc/special_remotes/rsync.mdwn
index 96e452b103e8ed496c006134a81360ffbec8ec17..e9f54dc6845e7d4e37965734eedd1192f2f097d5 100644 (file)
--- a/doc/special_remotes/rsync.mdwn
+++ b/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
-  done when using rsync over ssh. That escaping is needed with typical
-  setups, but not with some hosting providers that do not expose rsynced
-  filenames to the shell. You'll know you need this option if `git annex get`
-  from the special remote fails with an error message containing a single
-  quote (`'`) character. If that happens, you can run enableremote
-  setting shellescape=no.
+* `shellescape` - Optional. This has no effect when using rsync 3.2.4 or
+  newer. Set to "no" to avoid shell escaping
+  normally done when using older versions of rsync over ssh. That escaping
+  is needed with typical setups, but not with some hosting providers that do
+  not expose rsynced filenames to the shell. You'll know you need this
+  option if `git annex get` from the special remote fails with an error
+  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.