"git apply" was not very careful about reading from, removing,
updating and creating paths outside the working tree (under
--index/--cached) or the current directory (when used as a
replacement for GNU patch).
* jc/apply-beyond-symlink:
apply: do not touch a file beyond a symbolic link
apply: do not read from beyond a symbolic link
apply: do not read from the filesystem under --index
apply: reject input that touches outside the working area
Because Git tracks symbolic links as symbolic links, a path that
has a symbolic link in its leading part (e.g. path/to/dir/file,
where path/to/dir is a symbolic link to somewhere else, be it
inside or outside the working tree) can never appear in a patch
that validly applies, unless the same patch first removes the
symbolic link to allow a directory to be created there.
Detect and reject such a patch.
Things to note:
- Unfortunately, we cannot reuse the has_symlink_leading_path()
from dir.c, as that is only about the working tree, but "git
apply" can be told to apply the patch only to the index or to
both the index and to the working tree.
- We cannot directly use has_symlink_leading_path() even when we
are applying only to the working tree, as an early patch of a
valid input may remove a symbolic link path/to/dir and then a
later patch of the input may create a path path/to/dir/file, but
"git apply" first checks the input without touching either the
index or the working tree. The leading symbolic link check must
be done on the interim result we compute in-core (i.e. after the
first patch, there is no path/to/dir symbolic link and it is
perfectly valid to create path/to/dir/file).
Similarly, when an input creates a symbolic link path/to/dir and
then creates a file path/to/dir/file, we need to flag it as an
error without actually creating path/to/dir symbolic link in the
filesystem.
Instead, for any patch in the input that leaves a path (i.e. a non
deletion) in the result, we check all leading paths against the
resulting tree that the patch would create by inspecting all the
patches in the input and then the target of patch application
(either the index or the working tree).
This way, we catch a mischief or a mistake to add a symbolic link
path/to/dir and a file path/to/dir/file at the same time, while
allowing a valid patch that removes a symbolic link path/to/dir and
then adds a file path/to/dir/file.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We should reject a patch, whether it renames/copies dir/file to
elsewhere with or without modificiation, or updates dir/file in
place, if "dir/" part is actually a symbolic link to elsewhere,
by making sure that the code to read the preimage does not read
from a path that is beyond a symbolic link.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are many instances where the treatment of symbolic links in the
object model and the algorithms are tested, but where it is not
necessary to actually have a symbolic link in the worktree. Make
adjustments to the tests and remove the SYMLINKS prerequisite when
appropriate in trivial cases, where "trivial" means:
- merely a replacement of 'ln -s a b && git add b' by test_ln_s_add
is needed;
- a test for symbolic link on the file system can be split off (and
remains protected by SYMLINKS);
- existing code is equivalent to test_ln_s_add.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change the tests that skipped due to unavailable SYMLINKS support to
use the three-arg prereq form of test_expect_success.
Now we get an indication of how many tests that need symlinks are
being skipped on platforms that don't support them.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
SKIP messages are now part of the TAP plan. A TAP harness now knows
why a particular test was skipped and can report that information. The
non-TAP harness built into Git's test-lib did nothing special with
these messages, and is unaffected by these changes.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Many tests depend on that symbolic links work. This introduces a check
that sets the prerequisite tag SYMLINKS if the file system supports
symbolic links. Since so many tests have to check for this prerequisite,
we do the check in test-lib.sh, so that we don't need to repeat the test
in many scripts.
To check for 'ln -s' failures, you can use a FAT partition on Linux:
$ mkdosfs -C git-on-fat 1000000
$ sudo mount -o loop,uid=j6t,gid=users,shortname=winnt git-on-fat /mnt
Clone git to /mnt and
$ GIT_SKIP_TESTS='t0001.1[34] t0010 t1301 t403[34] t4129.[47] t5701.7
t7701.3 t9100 t9101.26 t9119 t9124.[67] t9200.10 t9600.6' \
make test
(These additionally skipped tests depend on POSIX permissions that FAT on
Linux does not provide.)
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
This uses "git-apply --whitespace=strip" to fix whitespace errors that have
crept in to our source files over time. There are a few files that need
to have trailing whitespaces (most notably, test vectors). The results
still passes the test, and build result in Documentation/ area is unchanged.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When switching from a branch with both x86_64/boot/Makefile and
i386/boot/Makefile to another branch that has x86_64/boot as a
symlink pointing at ../i386/boot, the code incorrectly removed
i386/boot/Makefile.
This was because we first removed everything under x86_64/boot
to make room to create a symbolic link x86_64/boot, then removed
x86_64/boot/Makefile which no longer exists but now is pointing
at i386/boot/Makefile, thanks to the symlink we just created.
This fixes it by using the has_symlink_leading_path() function
introduced previously for git-apply in the checkout codepath.
Earlier, "git checkout" was broken in t4122 test due to this
bug, and the test had an extra "git reset --hard" as a
workaround, which is removed because it is not needed anymore.
Signed-off-by: Junio C Hamano <junkio@cox.net>
HPA noticed that git-rebase fails when changes involve symlinks
in the middle of the hierarchy. Consider:
* The tree state before the patch is applied has arch/x86_64/boot
as a symlink pointing at ../i386/boot/
* The patch tries to remove arch/x86_64/boot symlink, and
create bunch of files there: .gitignore, Makefile, etc.
git-apply tries to be careful while applying patches; it never
touches the working tree until it is convinced that the patch
would apply cleanly. One of the check it does is that when it
knows a path is going to be created by the patch, it runs
lstat() on the path to make sure it does not exist.
This leads to a false alarm. Because we do not touch the
working tree before all the check passes, when we try to make
sure that arch/x86_64/boot/.gitignore does not exist yet, we
haven't removed the arch/x86_64/boot symlink. The lstat() check
ends up seeing arch/i386/boot/.gitignore through the
yet-to-be-removed symlink, and says "Hey, you already have a
file there, but what you fed me is a patch to create a new
file. I am not going to clobber what you have in the working
tree."
We have similar checks to see a file we are going to modify does
exist and match the preimage of the diff, which is done by
directly opening and reading the file.
For a file we are going to delete, we make sure that it does
exist and matches what is going to be removed (a removal patch
records the full preimage, so we check what you have in your
working tree matches it in full -- otherwise we would risk
losing your local changes), which again is done by directly
opening and reading the file.
These checks need to be adjusted so that they are not fooled by
symlinks in the middle.
- To make sure something does not exist, first lstat(). If it
does not exist, it does not, so be happy. If it _does_, we
might be getting fooled by a symlink in the middle, so break
leading paths and see if there are symlinks involved. When
we are checking for a path a/b/c/d, if any of a, a/b, a/b/c
is a symlink, then a/b/c/d does _NOT_ exist, for the purpose
of our test.
This would fix this particular case you saw, and would not
add extra overhead in the usual case.
- To make sure something already exists, first lstat(). If it
does not exist, barf (up to this, we already do). Even if it
does seem to exist, we might be getting fooled by a symlink
in the middle, so make sure leading paths are not symlinks.
This would make the normal codepath much more expensive for
deep trees, which is a bit worrisome.
This patch implements the first side of the check "making sure
it does not exist". The latter "making sure it exists" check is
not done yet, so applying the patch in reverse would still
fail, but we have to start from somewhere.
Signed-off-by: Junio C Hamano <junkio@cox.net>
Junio C Hamano
Johannes Sixt
Ævar Arnfjörð Bjarmason
Jeff King
Junio C Hamano