[darcs-users] [patch404] resolve issue1951: refuse to add files outside repository

Reinier Lamers bugs at darcs.net
Sat Oct 2 19:47:54 UTC 2010


Reinier Lamers <tux_rocker at reinier.de> added the comment:

Here is a new version of the patch that fixes the pattern match failure in 
annotate and adds haddock to fixSubPaths.

As for the unclear error message with 'darcs move', I suggest not fixing it 
for 2.5. The error is already present in older darcsen if you use absolute 
repository paths (like 'darcs move GNUmakefile /var/lib') so it's not a 
regression.

(The pattern match failure in annotate was also not a regression, but it looks 
bad enough to fix it immediately)

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch404>
__________________________________
-------------- next part --------------
7 patches for repository http://darcs.net/releases/branch-2.5:

Sun Sep 26 19:29:01 CEST 2010  Reinier Lamers <tux_rocker at reinier.de>
  * resolve issue1951: refuse to add files outside repository
  
  Instead of passing on relative paths to hashed-storage if they are not
  meaningful relative to the repository root, we now turn all relative paths into
  absolute paths. We do that by chdir'ing to them from either the current working
  directory, or if that fails, the repository root.

Sun Sep 26 19:52:13 CEST 2010  Reinier Lamers <tux_rocker at reinier.de>
  * Elaborate issue1951 test (for adding files outside the repository)

Sun Sep 26 19:56:04 CEST 2010  Reinier Lamers <tux_rocker at reinier.de>
  * Don't act like user gave no file arguments to add when he gave only invalid ones

Sun Sep 26 20:08:13 CEST 2010  Reinier Lamers <tux_rocker at reinier.de>
  * Mark issue1951 test as passing

Sun Sep 26 20:14:10 CEST 2010  Reinier Lamers <tux_rocker at reinier.de>
  * Make issue1951 test clean up (needed for 2.5 branch)

Sat Oct  2 21:06:00 CEST 2010  Reinier Lamers <tux_rocker at reinier.de>
  * Make annotate handle fixSubPath's rejecting paths

Sat Oct  2 21:34:47 CEST 2010  Reinier Lamers <tux_rocker at reinier.de>
  * Haddock fixSubPaths


New patches:

[resolve issue1951: refuse to add files outside repository
Reinier Lamers <tux_rocker at reinier.de>**20100926172901
 Ignore-this: 906eca5d6a62c103b3e423967b37570d
 
 Instead of passing on relative paths to hashed-storage if they are not
 meaningful relative to the repository root, we now turn all relative paths into
 absolute paths. We do that by chdir'ing to them from either the current working
 directory, or if that fails, the repository root.
] hunk ./src/Darcs/Arguments.lhs 122
 import Darcs.Repository.State ( restrictBoring, applyTreeFilter, readRecordedAndPending )
 import Darcs.URL ( isFile )
 import Darcs.RepoPath ( AbsolutePath, AbsolutePathOrStd, SubPath, toFilePath,
-                        makeSubPathOf, simpleSubPath,
-                        ioAbsolute, ioAbsoluteOrStd,
+                        makeSubPathOf, ioAbsolute, ioAbsoluteOrStd,
                         makeAbsolute, makeAbsoluteOrStd, rootDirectory )
 import Darcs.Patch.MatchData ( patchMatch )
 import Darcs.Flags ( DarcsFlag(..), maxCount, defaultFlag )
hunk ./src/Darcs/Arguments.lhs 499
     fixit p = do ap <- ioAbsolute p
                  case makeSubPathOf r ap of
                    Just sp -> return $ Right sp
-                   Nothing -> return $ maybe (Left p) Right $ simpleSubPath p
+                   Nothing -> withCurrentDirectory r $ do
+                     absolutePathByRepodir <- ioAbsolute p
+                     return $ case makeSubPathOf r absolutePathByRepodir of
+                                Just sp -> Right sp
+                                Nothing -> Left p
 
 partitionEither :: [Either a b] -> ([b],[a])
 partitionEither es = ( [b | Right b <- es]
[Elaborate issue1951 test (for adding files outside the repository)
Reinier Lamers <tux_rocker at reinier.de>**20100926175213
 Ignore-this: 460ee08b38d8a247379c3fa2d8edc471
] hunk ./tests/failing-issue1951-add-outside-repo.sh 29
 
 . lib                           # Load some portability helpers.
 darcs init      --repo R        # Create our test repos.
+darcs init      --repo R2
+darcs init      --repo R3
+darcs init      --repo R4
+darcs init      --repo R5
+darcs init      --repo R6
+darcs init      --repo R7
+darcs init      --repo R8
+darcs init      --repo R9
+darcs init      --repo R10
+
+# test if adding files outside the repository fails
 
 OUTSIDE=`pwd`
 
hunk ./tests/failing-issue1951-add-outside-repo.sh 52
 darcs whatsnew > log
 not grep junk-for-issue1951 log
 cd ..
+
+# test adding a file that:
+#   * is in a subdirectory of the repository root
+#   * is referred to with a path relative to the cwd, when the cwd is the
+#     directory that the file is in
+cd R2
+mkdir subdir
+cd subdir
+touch f
+darcs add f
+darcs whatsnew > log
+grep 'subdir/f' log
+cd ../..
+
+# same as above, but now the relative path is valid both from the cwd and from
+# the repository root. Darcs should add the file in the cwd, not the one in the
+# repository root
+cd R3
+touch f
+mkdir subdir
+cd subdir
+touch f
+darcs add f
+darcs whatsnew > log
+grep 'subdir/f' log
+cd ../..
+
+# now test that adding fails on a file that
+#  * is in the repository root
+#  * is referred to with a path relative to the repository root, when the cwd is
+#    not the repository root
+cd R4
+touch myfilename
+mkdir subdir
+cd subdir
+not darcs add myfilename
+cd ../..
+
+# test adding a file by relative path from the repo root, when the cwd is
+# outside the repo
+# It may seem counterintuitive that this succeeds and the cases above and below
+# do not, but that's the way it is. We use this feature ourselves in our test
+# suite.
+
+touch R5/myfilename
+darcs add --repo R5 myfilename
+darcs whatsnew --repo R5 > log
+grep 'myfilename' log
+not grep '\.\./myfilename' log
+
+# The case below makes the R4 case (of using a repo-root-relative path in a
+# subdir of the repo) look even more like the R5 case (of using a
+# repo-root-relative path outside the repo) by usign the --repo flag. It still
+# failed on darcs 2.4.
+
+cd R6
+touch myfilename
+mkdir subdir
+cd subdir
+not darcs add --repo .. myfilename
+cd ../..
+
+# Test adding a file by relative path from the repo root, when the cwd is
+# outside the repo, and the relative path also exists from the cwd
+
+touch myfilename
+touch R7/myfilename
+darcs add --repo R7 myfilename
+darcs whatsnew --repo R7 > log
+grep 'myfilename' log
+not grep '\.\.myfilename' log
+
+# Like the R4 case: try to use a path relative to the repo root from a cwd that
+# is a subdir of the repo root. In this case the path relative to the repo root
+# also includes a directory however.
+
+cd R8
+mkdir subdir1
+mkdir subdir2
+touch subdir2/myfilename
+cd subdir1
+not darcs add subdir2/myfilename
+cd ../..
+
+# Try adding a non-repository file using a non-existent repo subdirectory name
+# followed by two ..'s
+touch myfilename
+cd R9
+not darcs add nonexistentsubdir/../../myfilename
+cd ..
+
+# Try adding a non-repository file using an existing repo subdirectory name
+# followed by two ..'s
+touch myfilename
+cd R10
+mkdir subdir
+not darcs add subdir/../../myfilename
+cd ..
[Don't act like user gave no file arguments to add when he gave only invalid ones
Reinier Lamers <tux_rocker at reinier.de>**20100926175604
 Ignore-this: 9fa00f5230d7be070973e7e3a7abb048
] hunk ./src/Darcs/Commands/Add.lhs 105
  do -- TODO do not expand here, and use findM/findIO or such later
     -- (needs adding to hashed-storage first though)
     cur <- expand =<< readRecordedAndPending repository
-    origfiles <- fixSubPaths opts args
-    when (null origfiles) $
+    when (null args) $
        putStrLn "Nothing specified, nothing added." >>
        putStrLn "Maybe you wanted to say `darcs add --recursive .'?"
hunk ./src/Darcs/Commands/Add.lhs 108
+    origfiles <- fixSubPaths opts args
     let parlist = getParents cur (map toFilePath origfiles)
     flist' <- if Recursive `elem` opts
               then expandDirs origfiles
[Mark issue1951 test as passing
Reinier Lamers <tux_rocker at reinier.de>**20100926180813
 Ignore-this: 5bb72adeb44e4fc6fe68098cbe799ce3
] move ./tests/failing-issue1951-add-outside-repo.sh ./tests/issue1951-add-outside-repo.sh
[Make issue1951 test clean up (needed for 2.5 branch)
Reinier Lamers <tux_rocker at reinier.de>**20100926181410
 Ignore-this: e35a4e4997ecc009cf3377806168249
] hunk ./tests/issue1951-add-outside-repo.sh 28
 ## SOFTWARE.
 
 . lib                           # Load some portability helpers.
+
+rm -rf R R2 R3 R4 R5 R6 R7 R8 R9 R10
 darcs init      --repo R        # Create our test repos.
 darcs init      --repo R2
 darcs init      --repo R3
[Make annotate handle fixSubPath's rejecting paths
Reinier Lamers <tux_rocker at reinier.de>**20101002190600
 Ignore-this: 336e50760bc14332436e2e1ac93a0d8c
] hunk ./src/Darcs/Commands/Annotate.lhs 149
 that patch was applied.  If a directory and a tag name are given, the
 details of the patches involved in the specified tagged version will be output.
 \begin{code}
-annotateCmd opts args@[_] = withRepository opts $- \repository -> do
+annotateCmd opts [file] = withRepository opts $- \repository -> do
   r <- readRepo repository
hunk ./src/Darcs/Commands/Annotate.lhs 151
-  (rel_file_or_directory:_) <- fixSubPaths opts args
+  fixed_args <- fixSubPaths opts [file]
+  (rel_file_or_directory:_) <- case fixed_args of
+                                 [] -> fail ("The supplied path " ++ file ++ " is not usable")
+                                 fs -> return fs
   let file_or_directory = rel_file_or_directory
   pinfo <- if haveNonrangeMatch opts
            then return $ patch2patchinfo `unseal2` (matchPatch opts r)
[Haddock fixSubPaths
Reinier Lamers <tux_rocker at reinier.de>**20101002193447
 Ignore-this: e636f81773470f918d6aaffff477014f
] hunk ./src/Darcs/Arguments.lhs 484
                 then toFilePath `fmap` fixFilePath opts f
                 else return f
 
+-- | @fixSubPaths files@ returns the @SubPath at s for the paths in @files@ that
+-- are inside the repository, preserving their order. Paths in @files@ that are
+-- outside the repository directory are not in the result.
+--
+-- When converting a relative path to an absolute one, this function first tries
+-- to interpret the relative path with respect to the current working directory.
+-- If that fails, it tries to interpret it with respect to the repository
+-- directory. Only when that fails does it omit the path from the result.
+--
+-- It is intended for validating file arguments to darcs commands.
 fixSubPaths :: [DarcsFlag] -> [FilePath] -> IO [SubPath]
 fixSubPaths flags fs =
     withCurrentDirectory o $

Context:

[Accept issue1951: add outside of current repository.
Eric Kow <kowey at darcs.net>**20100908095448
 Ignore-this: 5085d7647e9408ea7b5282e1cb0d1079
 Regression between darcs 2.4 and 2.4.98.5.
] 
[TAG 2.4.98.5
Reinier Lamers <tux_rocker at reinier.de>**20100905170906
 Ignore-this: 848f746fa6a939cef069abe6722d4206
] 
Patch bundle hash:
1b957005f5b495c5c5a9931328e516a24870cab6


More information about the darcs-users mailing list