diff options
| -rw-r--r-- | CHANGELOG | 8 | ||||
| -rw-r--r-- | Remote/External.hs | 15 | ||||
| -rw-r--r-- | Remote/External/Types.hs | 35 | ||||
| -rw-r--r-- | doc/bugs/external_special_remote_protocol_broken_by_key_with_spaces.mdwn | 11 | ||||
| -rw-r--r-- | doc/design/external_special_remote_protocol.mdwn | 4 |
5 files changed, 62 insertions, 11 deletions
@@ -23,6 +23,14 @@ git-annex (6.20170521) UNRELEASED; urgency=medium external special remote protocol. * migrate: WORM keys containing spaces will be migrated to not contain spaces anymore. + * External special remotes will refuse to operate on keys with spaces in + their names. That has never worked correctly due to the design of the + external special remote protocol. Display an error message suggesting + migration. + * Fix incorrect external special remote documentation, which said that + the filename parameter to the TRANSFER command could not contain + spaces. It can in fact contain spaces. Special remotes implementors + that relied on that may need to fix bugs in their special remotes. -- Joey Hess <id@joeyh.name> Sat, 17 Jun 2017 13:02:24 -0400 diff --git a/Remote/External.hs b/Remote/External.hs index 0ac381b8c..32b95e9bb 100644 --- a/Remote/External.hs +++ b/Remote/External.hs @@ -134,7 +134,7 @@ externalSetup _ mu _ c gc = do store :: External -> Storer store external = fileStorer $ \k f p -> - handleRequest external (TRANSFER Upload k f) (Just p) $ \resp -> + handleRequestKey external (\sk -> TRANSFER Upload sk f) k (Just p) $ \resp -> case resp of TRANSFER_SUCCESS Upload k' | k == k' -> Just $ return True @@ -146,7 +146,7 @@ store external = fileStorer $ \k f p -> retrieve :: External -> Retriever retrieve external = fileRetriever $ \d k p -> - handleRequest external (TRANSFER Download k d) (Just p) $ \resp -> + handleRequestKey external (\sk -> TRANSFER Download sk d) k (Just p) $ \resp -> case resp of TRANSFER_SUCCESS Download k' | k == k' -> Just $ return () @@ -156,7 +156,7 @@ retrieve external = fileRetriever $ \d k p -> remove :: External -> Remover remove external k = safely $ - handleRequest external (REMOVE k) Nothing $ \resp -> + handleRequestKey external REMOVE k Nothing $ \resp -> case resp of REMOVE_SUCCESS k' | k == k' -> Just $ return True @@ -169,7 +169,7 @@ remove external k = safely $ checkKey :: External -> CheckPresent checkKey external k = either giveup id <$> go where - go = handleRequest external (CHECKPRESENT k) Nothing $ \resp -> + go = handleRequestKey external CHECKPRESENT k Nothing $ \resp -> case resp of CHECKPRESENT_SUCCESS k' | k' == k -> Just $ return $ Right True @@ -180,7 +180,7 @@ checkKey external k = either giveup id <$> go _ -> Nothing whereis :: External -> Key -> Annex [String] -whereis external k = handleRequest external (WHEREIS k) Nothing $ \resp -> case resp of +whereis external k = handleRequestKey external WHEREIS k Nothing $ \resp -> case resp of WHEREIS_SUCCESS s -> Just $ return [s] WHEREIS_FAILURE -> Just $ return [] UNSUPPORTED_REQUEST -> Just $ return [] @@ -212,6 +212,11 @@ handleRequest external req mp responsehandler = withExternalState external $ \st -> handleRequest' st external req mp responsehandler +handleRequestKey :: External -> (SafeKey -> Request) -> Key -> Maybe MeterUpdate -> (Response -> Maybe (Annex a)) -> Annex a +handleRequestKey external mkreq k mp responsehandler = case mkSafeKey k of + Right sk -> handleRequest external (mkreq sk) mp responsehandler + Left e -> giveup e + handleRequest' :: ExternalState -> External -> Request -> Maybe MeterUpdate -> (Response -> Maybe (Annex a)) -> Annex a handleRequest' st external req mp responsehandler | needsPREPARE req = do diff --git a/Remote/External/Types.hs b/Remote/External/Types.hs index ef8724ee7..cda934220 100644 --- a/Remote/External/Types.hs +++ b/Remote/External/Types.hs @@ -18,6 +18,8 @@ module Remote.External.Types ( Proto.Sendable(..), Proto.Receivable(..), Request(..), + SafeKey, + mkSafeKey, needsPREPARE, Response(..), RemoteRequest(..), @@ -36,11 +38,13 @@ import Types.Transfer (Direction(..)) import Config.Cost (Cost) import Types.Remote (RemoteConfig) import Types.Availability (Availability(..)) +import Types.Key import Utility.Url (URLString) import qualified Utility.SimpleProtocol as Proto import Control.Concurrent.STM import Network.URI +import Data.Char data External = External { externalType :: ExternalType @@ -77,6 +81,29 @@ type PID = Int data PrepareStatus = Unprepared | Prepared | FailedPrepare ErrorMsg +-- The protocol does not support keys with spaces in their names; +-- SafeKey can only be constructed for keys that are safe to use with the +-- protocol. +newtype SafeKey = SafeKey Key + deriving (Show) + +mkSafeKey :: Key -> Either String SafeKey +mkSafeKey k + | any isSpace (keyName k) = Left $ concat + [ "Sorry, this file cannot be stored on an external special remote because its key's name contains a space. " + , "To avoid this problem, you can run: git-annex migrate --backend=" + , formatKeyVariety (keyVariety k) + , " and pass it the name of the file" + ] + | otherwise = Right (SafeKey k) + +fromSafeKey :: SafeKey -> Key +fromSafeKey (SafeKey k) = k + +instance Proto.Serializable SafeKey where + serialize = Proto.serialize . fromSafeKey + deserialize = fmap SafeKey . Proto.deserialize + -- Messages that can be sent to the external remote to request it do something. data Request = PREPARE @@ -85,10 +112,10 @@ data Request | GETAVAILABILITY | CLAIMURL URLString | CHECKURL URLString - | TRANSFER Direction Key FilePath - | CHECKPRESENT Key - | REMOVE Key - | WHEREIS Key + | TRANSFER Direction SafeKey FilePath + | CHECKPRESENT SafeKey + | REMOVE SafeKey + | WHEREIS SafeKey deriving (Show) -- Does PREPARE need to have been sent before this request? diff --git a/doc/bugs/external_special_remote_protocol_broken_by_key_with_spaces.mdwn b/doc/bugs/external_special_remote_protocol_broken_by_key_with_spaces.mdwn index a5e45b108..4180678c9 100644 --- a/doc/bugs/external_special_remote_protocol_broken_by_key_with_spaces.mdwn +++ b/doc/bugs/external_special_remote_protocol_broken_by_key_with_spaces.mdwn @@ -3,3 +3,14 @@ external special remote protocol, which uses whitespace to separate the key parameter from subsequent parameters. I think that this only causes problems for WORM keys. --[[Joey]] + +> Ok, went with my last approach, rather than complicating all special +> remotes due to this problem, we'll deprecate WORM keys with spaces in +> their name, and provide a migratipon path. +> +> The error message looks like this: + +> > Sorry, this file cannot be stored on an external special remote because its key's name contains a space. To avoid this problem, you can run: git-annex migrate --backend=WORM + +> This is fixed as well as is feasible, so while that kind of sucks, +> calling it [[done]]. --[[Joey]] diff --git a/doc/design/external_special_remote_protocol.mdwn b/doc/design/external_special_remote_protocol.mdwn index 8cb7ed3b6..87a838bd4 100644 --- a/doc/design/external_special_remote_protocol.mdwn +++ b/doc/design/external_special_remote_protocol.mdwn @@ -106,8 +106,8 @@ The following requests *must* all be supported by the special remote. * `TRANSFER STORE|RETRIEVE Key File` Requests the transfer of a key. For STORE, the File is the file to upload; for RETRIEVE the File is where to store the download. - Note that the File should not influence the filename used on the remote. - The filename will not contain any whitespace. + Note that the File should not influence the filename used on the remote. + Note that in some cases, the File may contain whitespace. Note that it's important that, while a Key is being stored, CHECKPRESENT not indicate it's present until all the data has been transferred. * `CHECKPRESENT Key` |