Better sanitization of problem characters when generating URL and WORM keys.

FAT has a lot of characters it does not allow in filenames, like ? and *
It's probably the worst offender, but other filesystems also have
limitiations.

In 2011, I made keyFile escape : to handle FAT, but missed the other
characters. It also turns out that when I did that, I was also living
dangerously; any existing keys that contained a : had their object
location change. Oops.

So, adding new characters to escape to keyFile is out. Well, it would be
possible to make keyFile behave differently on a per-filesystem basis, but
this would be a real nightmare to get right. Consider that a rsync special
remote uses keyFile to determine the filenames to use, and we don't know
the underlying filesystem on the rsync server..

Instead, I have gone for a solution that is backwards compatable and
simple. Its only downside is that already generated URL and WORM keys
might not be able to be stored on FAT or some other filesystem that
dislikes a character used in the key. (In this case, the user can just
migrate the problem keys to a checksumming backend. If this became a big
problem, fsck could be made to detect these and suggest a migration.)

Going forward, new keys that are created will escape all characters that
are likely to cause problems. And if some filesystem comes along that's
even worse than FAT (seems unlikely, but here it is 2013, and people are
still using FAT!), additional characters can be added to the set that are
escaped without difficulty.

(Also, made WORM limit the part of the filename that is embedded in the key,
to deal with filesystem filename length limits. This could have already
been a problem, but is more likely now, since the escaping of the filename
can make it longer.)

This commit was sponsored by Ian Downes

1 files changed, 32 insertions, 2 deletions

diff --git a/Locations.hs b/Locations.hs
index 36104458e..9b7bb575b 100644
--- a/Locations.hs
+++ b/Locations.hs
@@ -51,6 +51,7 @@ module Locations (
 	annexHashes,
 	hashDirMixed,
 	hashDirLower,
+	preSanitizeKeyName,
 
 	prop_idempotent_fileKey
 ) where
@@ -58,6 +59,7 @@ module Locations (
 import Data.Bits
 import Data.Word
 import Data.Hash.MD5
+import Data.Char
 
 import Common
 import Types
@@ -284,6 +286,32 @@ gitAnnexAssistantDefaultDir = "annex"
 isLinkToAnnex :: FilePath -> Bool
 isLinkToAnnex s = (pathSeparator:objectDir) `isInfixOf` s
 
+{- Sanitizes a String that will be used as part of a Key's keyName,
+ - dealing with characters that cause problems on substandard filesystems.
+ -
+ - This is used when a new Key is initially being generated, eg by getKey.
+ - Unlike keyFile and fileKey, it does not need to be a reversable
+ - escaping. Also, it's ok to change this to add more problimatic
+ - characters later. Unlike changing keyFile, which could result in the
+ - filenames used for existing keys changing and contents getting lost.
+ -
+ - It is, however, important that the input and output of this function
+ - have a 1:1 mapping, to avoid two different inputs from mapping to the
+ - same key.
+ -}
+preSanitizeKeyName :: String -> String
+preSanitizeKeyName = concatMap escape
+  where
+  	escape c
+		| isAsciiUpper c || isAsciiLower c || isDigit c = [c]
+		| c `elem` ".-_ " = [c] -- common, assumed safe
+		| c `elem` "/%:" = [c] -- handled by keyFile
+		-- , is safe and uncommon, so will be used to escape
+		-- other characters. By itself, it is escaped to 
+		-- doubled form.
+		| c == ',' = ",,"
+		| otherwise = ',' : show(ord(c))
+
 {- Converts a key into a filename fragment without any directory.
  -
  - Escape "/" in the key name, to keep a flat tree of files and avoid
@@ -293,8 +321,10 @@ isLinkToAnnex s = (pathSeparator:objectDir) `isInfixOf` s
  -     a slash
  - "%" is escaped to "&s", and "&" to "&a"; this ensures that the mapping
  -     is one to one.
- - ":" is escaped to "&c", because despite it being 20XX people still care
- -     about FAT.
+ - ":" is escaped to "&c", because it seemed like a good idea at the time.
+ -
+ - Changing what this function escapes and how is not a good idea, as it
+ - can cause existing objects to get lost.
  -}
 keyFile :: Key -> FilePath
 keyFile key = replace "/" "%" $ replace ":" "&c" $