summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--CHANGELOG8
-rw-r--r--Remote/External.hs15
-rw-r--r--Remote/External/Types.hs35
-rw-r--r--doc/bugs/external_special_remote_protocol_broken_by_key_with_spaces.mdwn11
-rw-r--r--doc/design/external_special_remote_protocol.mdwn4
5 files changed, 62 insertions, 11 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 91a1e3488..bb3a73a1c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -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`