summaryrefslogtreecommitdiff
path: root/t
AgeCommit message (Collapse)AuthorFilesLines
2021-02-12Sync with 2.29.3Libravatar Johannes Schindelin3-3/+144
* maint-2.29: Git 2.29.3 Git 2.28.1 Git 2.27.1 Git 2.26.3 Git 2.25.5 Git 2.24.4 Git 2.23.4 Git 2.22.5 Git 2.21.4 Git 2.20.5 Git 2.19.6 Git 2.18.5 Git 2.17.6 unpack_trees(): start with a fresh lstat cache run-command: invalidate lstat cache after a command finished checkout: fix bug that makes checkout follow symlinks in leading path
2021-02-12Sync with 2.28.1Libravatar Johannes Schindelin3-3/+145
* maint-2.28: Git 2.28.1 Git 2.27.1 Git 2.26.3 Git 2.25.5 Git 2.24.4 Git 2.23.4 Git 2.22.5 Git 2.21.4 Git 2.20.5 Git 2.19.6 Git 2.18.5 Git 2.17.6 unpack_trees(): start with a fresh lstat cache run-command: invalidate lstat cache after a command finished checkout: fix bug that makes checkout follow symlinks in leading path
2021-02-12Sync with 2.27.1Libravatar Johannes Schindelin3-3/+145
* maint-2.27: Git 2.27.1 Git 2.26.3 Git 2.25.5 Git 2.24.4 Git 2.23.4 Git 2.22.5 Git 2.21.4 Git 2.20.5 Git 2.19.6 Git 2.18.5 Git 2.17.6 unpack_trees(): start with a fresh lstat cache run-command: invalidate lstat cache after a command finished checkout: fix bug that makes checkout follow symlinks in leading path
2021-02-12Sync with 2.26.3Libravatar Johannes Schindelin3-3/+145
* maint-2.26: Git 2.26.3 Git 2.25.5 Git 2.24.4 Git 2.23.4 Git 2.22.5 Git 2.21.4 Git 2.20.5 Git 2.19.6 Git 2.18.5 Git 2.17.6 unpack_trees(): start with a fresh lstat cache run-command: invalidate lstat cache after a command finished checkout: fix bug that makes checkout follow symlinks in leading path
2021-02-12Sync with 2.25.5Libravatar Johannes Schindelin3-3/+145
* maint-2.25: Git 2.25.5 Git 2.24.4 Git 2.23.4 Git 2.22.5 Git 2.21.4 Git 2.20.5 Git 2.19.6 Git 2.18.5 Git 2.17.6 unpack_trees(): start with a fresh lstat cache run-command: invalidate lstat cache after a command finished checkout: fix bug that makes checkout follow symlinks in leading path
2021-02-12Sync with 2.24.4Libravatar Johannes Schindelin3-3/+145
* maint-2.24: Git 2.24.4 Git 2.23.4 Git 2.22.5 Git 2.21.4 Git 2.20.5 Git 2.19.6 Git 2.18.5 Git 2.17.6 unpack_trees(): start with a fresh lstat cache run-command: invalidate lstat cache after a command finished checkout: fix bug that makes checkout follow symlinks in leading path
2021-02-12Sync with 2.23.4Libravatar Johannes Schindelin3-3/+145
* maint-2.23: Git 2.23.4 Git 2.22.5 Git 2.21.4 Git 2.20.5 Git 2.19.6 Git 2.18.5 Git 2.17.6 unpack_trees(): start with a fresh lstat cache run-command: invalidate lstat cache after a command finished checkout: fix bug that makes checkout follow symlinks in leading path
2021-02-12Sync with 2.22.5Libravatar Johannes Schindelin3-3/+145
* maint-2.22: Git 2.22.5 Git 2.21.4 Git 2.20.5 Git 2.19.6 Git 2.18.5 Git 2.17.6 unpack_trees(): start with a fresh lstat cache run-command: invalidate lstat cache after a command finished checkout: fix bug that makes checkout follow symlinks in leading path
2021-02-12Sync with 2.21.4Libravatar Johannes Schindelin3-3/+145
* maint-2.21: Git 2.21.4 Git 2.20.5 Git 2.19.6 Git 2.18.5 Git 2.17.6 unpack_trees(): start with a fresh lstat cache run-command: invalidate lstat cache after a command finished checkout: fix bug that makes checkout follow symlinks in leading path
2021-02-12Sync with 2.20.5Libravatar Johannes Schindelin3-3/+145
* maint-2.20: Git 2.20.5 Git 2.19.6 Git 2.18.5 Git 2.17.6 unpack_trees(): start with a fresh lstat cache run-command: invalidate lstat cache after a command finished checkout: fix bug that makes checkout follow symlinks in leading path
2021-02-12Sync with 2.19.6Libravatar Johannes Schindelin3-3/+145
* maint-2.19: Git 2.19.6 Git 2.18.5 Git 2.17.6 unpack_trees(): start with a fresh lstat cache run-command: invalidate lstat cache after a command finished checkout: fix bug that makes checkout follow symlinks in leading path
2021-02-12Sync with 2.18.5Libravatar Johannes Schindelin3-3/+145
* maint-2.18: Git 2.18.5 Git 2.17.6 unpack_trees(): start with a fresh lstat cache run-command: invalidate lstat cache after a command finished checkout: fix bug that makes checkout follow symlinks in leading path
2021-02-12Sync with 2.17.6Libravatar Johannes Schindelin3-3/+145
* maint-2.17: Git 2.17.6 unpack_trees(): start with a fresh lstat cache run-command: invalidate lstat cache after a command finished checkout: fix bug that makes checkout follow symlinks in leading path
2021-02-12run-command: invalidate lstat cache after a command finishedLibravatar Johannes Schindelin1-0/+36
In the previous commit, we intercepted calls to `rmdir()` to invalidate the lstat cache in the successful case, so that the lstat cache could not have the idea that a directory exists where there is none. The same situation can arise, of course, when a separate process is spawned (most notably, this is the case in `submodule_move_head()`). Obviously, we cannot know whether a directory was removed in that process, therefore we must invalidate the lstat cache afterwards. Note: in contrast to `lstat_cache_aware_rmdir()`, we invalidate the lstat cache even in case of an error: the process might have removed a directory and still have failed afterwards. Co-authored-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2021-02-12checkout: fix bug that makes checkout follow symlinks in leading pathLibravatar Matheus Tavares3-3/+109
Before checking out a file, we have to confirm that all of its leading components are real existing directories. And to reduce the number of lstat() calls in this process, we cache the last leading path known to contain only directories. However, when a path collision occurs (e.g. when checking out case-sensitive files in case-insensitive file systems), a cached path might have its file type changed on disk, leaving the cache on an invalid state. Normally, this doesn't bring any bad consequences as we usually check out files in index order, and therefore, by the time the cached path becomes outdated, we no longer need it anyway (because all files in that directory would have already been written). But, there are some users of the checkout machinery that do not always follow the index order. In particular: checkout-index writes the paths in the same order that they appear on the CLI (or stdin); and the delayed checkout feature -- used when a long-running filter process replies with "status=delayed" -- postpones the checkout of some entries, thus modifying the checkout order. When we have to check out an out-of-order entry and the lstat() cache is invalid (due to a previous path collision), checkout_entry() may end up using the invalid data and thrusting that the leading components are real directories when, in reality, they are not. In the best case scenario, where the directory was replaced by a regular file, the user will get an error: "fatal: unable to create file 'foo/bar': Not a directory". But if the directory was replaced by a symlink, checkout could actually end up following the symlink and writing the file at a wrong place, even outside the repository. Since delayed checkout is affected by this bug, it could be used by an attacker to write arbitrary files during the clone of a maliciously crafted repository. Some candidate solutions considered were to disable the lstat() cache during unordered checkouts or sort the entries before passing them to the checkout machinery. But both ideas include some performance penalty and they don't future-proof the code against new unordered use cases. Instead, we now manually reset the lstat cache whenever we successfully remove a directory. Note: We are not even checking whether the directory was the same as the lstat cache points to because we might face a scenario where the paths refer to the same location but differ due to case folding, precomposed UTF-8 issues, or the presence of `..` components in the path. Two regression tests, with case-collisions and utf8-collisions, are also added for both checkout-index and delayed checkout. Note: to make the previously mentioned clone attack unfeasible, it would be sufficient to reset the lstat cache only after the remove_subtree() call inside checkout_entry(). This is the place where we would remove a directory whose path collides with the path of another entry that we are currently trying to check out (possibly a symlink). However, in the interest of a thorough fix that does not leave Git open to similar-but-not-identical attack vectors, we decided to intercept all `rmdir()` calls in one fell swoop. This addresses CVE-2021-21300. Co-authored-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
2021-02-08Merge branch 'pb/blame-funcname-range-userdiff' into maintLibravatar Junio C Hamano1-4/+4
Test fix. * pb/blame-funcname-range-userdiff: annotate-tests: quote variable expansions containing path names
2021-02-08Merge branch 'jk/p5303-sed-portability-fix' into maintLibravatar Junio C Hamano1-4/+8
A perf script was made more portable. * jk/p5303-sed-portability-fix: p5303: avoid sed GNU-ism
2021-02-08Merge branch 'ab/branch-sort' into maintLibravatar Junio C Hamano1-1/+50
The implementation of "git branch --sort" wrt the detached HEAD display has always been hacky, which has been cleaned up. * ab/branch-sort: branch: show "HEAD detached" first under reverse sort branch: sort detached HEAD based on a flag ref-filter: move ref_sorting flags to a bitfield ref-filter: move "cmp_fn" assignment into "else if" arm ref-filter: add braces to if/else if/else chain branch tests: add to --sort tests branch: change "--local" to "--list" in comment
2021-02-08Merge branch 'ar/t6016-modernise' into maintLibravatar Junio C Hamano1-187/+167
Test update. * ar/t6016-modernise: t6016: move to lib-log-graph.sh framework
2021-02-08Merge branch 'ma/t1300-cleanup' into maintLibravatar Junio C Hamano1-40/+32
Code clean-up. * ma/t1300-cleanup: t1300: don't needlessly work with `core.foo` configs t1300: remove duplicate test for `--file no-such-file` t1300: remove duplicate test for `--file ../foo`
2021-02-08Merge branch 'fc/t6030-bisect-reset-removes-auxiliary-files' into maintLibravatar Junio C Hamano1-8/+8
A 3-year old test that was not testing anything useful has been corrected. * fc/t6030-bisect-reset-removes-auxiliary-files: test: bisect-porcelain: fix location of files
2021-02-05Merge branch 'jk/log-cherry-pick-duplicate-patches' into maintLibravatar Junio C Hamano1-0/+12
When more than one commit with the same patch ID appears on one side, "git log --cherry-pick A...B" did not exclude them all when a commit with the same patch ID appears on the other side. Now it does. * jk/log-cherry-pick-duplicate-patches: patch-ids: handle duplicate hashmap entries
2021-02-05Merge branch 'jk/forbid-lf-in-git-url' into maintLibravatar Junio C Hamano2-0/+20
Newline characters in the host and path part of git:// URL are now forbidden. * jk/forbid-lf-in-git-url: fsck: reject .gitmodules git:// urls with newlines git_connect_git(): forbid newlines in host and path
2021-02-05Merge branch 'ad/t4129-setfacl-target-fix' into maintLibravatar Junio C Hamano1-1/+1
Test fix. * ad/t4129-setfacl-target-fix: t4129: fix setfacl-related permissions failure
2021-02-05Merge branch 'jk/t5516-deflake' into maintLibravatar Junio C Hamano1-1/+3
Test fix. * jk/t5516-deflake: t5516: loosen "not our ref" error check
2021-02-05Merge branch 'pb/mergetool-tool-help-fix' into maintLibravatar Junio C Hamano1-0/+11
Fix 2.29 regression where "git mergetool --tool-help" fails to list all the available tools. * pb/mergetool-tool-help-fix: mergetool--lib: fix '--tool-help' to correctly show available tools
2021-02-05Merge branch 'ds/for-each-repo-noopfix' into maintLibravatar Junio C Hamano1-0/+6
"git for-each-repo --config=<var> <cmd>" should not run <cmd> for any repository when the configuration variable <var> is not defined even once. * ds/for-each-repo-noopfix: for-each-repo: do nothing on empty config
2021-02-05Merge branch 'mt/t4129-with-setgid-dir' into maintLibravatar Junio C Hamano1-2/+7
Some tests expect that "ls -l" output has either '-' or 'x' for group executable bit, but setgid bit can be inherited from parent directory and make these fields 'S' or 's' instead, causing test failures. * mt/t4129-with-setgid-dir: t4129: don't fail if setgid is set in the test directory
2021-02-05Merge branch 'en/stash-apply-sparse-checkout' into maintLibravatar Junio C Hamano2-8/+96
"git stash" did not work well in a sparsely checked out working tree. * en/stash-apply-sparse-checkout: stash: fix stash application in sparse-checkouts stash: remove unnecessary process forking t7012: add a testcase demonstrating stash apply bugs in sparse checkouts
2021-02-05Merge branch 'nk/perf-fsmonitor-cleanup' into maintLibravatar Junio C Hamano1-1/+6
Test fix. * nk/perf-fsmonitor-cleanup: p7519: allow running without watchman prereq
2021-02-05Merge branch 'pk/subsub-fetch-fix-take-2' into maintLibravatar Junio C Hamano1-0/+117
"git fetch --recurse-submodules" fix (second attempt). * pk/subsub-fetch-fix-take-2: submodules: fix of regression on fetching of non-init subsub-repo
2021-01-30annotate-tests: quote variable expansions containing path namesLibravatar Johannes Sixt1-4/+4
The test case added by 9466e3809d ("blame: enable funcname blaming with userdiff driver", 2020-11-01) forgot to quote variable expansions. This causes failures when the current directory contains blanks. One variable that the test case introduces will not have IFS characters and could remain without quotes, but let's quote all expansions for consistency, not just the one that has the path name. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Acked-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-29p5303: avoid sed GNU-ismLibravatar Jeff King1-4/+8
Using "1~5" isn't portable. Nobody seems to have noticed, since perhaps people don't tend to run the perf suite on more exotic platforms. Still, it's better to set a good example. We can use: perl -ne 'print if $. % 5 == 1' instead. But we can further observe that perl does a good job of the other parts of this pipeline, and fold the whole thing together. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-12patch-ids: handle duplicate hashmap entriesLibravatar Jeff King1-0/+12
This fixes a bug introduced in dfb7a1b4d0 (patch-ids: stop using a hand-rolled hashmap implementation, 2016-07-29) in which git rev-list --cherry-pick A...B will fail to suppress commits reachable from A even if a commit with matching patch-id appears in B. Around the time of that commit, the algorithm for "--cherry-pick" looked something like this: 0. Traverse all of the commits, marking them as being on the left or right side of the symmetric difference. 1. Iterate over the left-hand commits, inserting a patch-id struct for each into a hashmap, and pointing commit->util to the patch-id struct. 2. Iterate over the right-hand commits, checking which are present in the hashmap. If so, we exclude the commit from the output _and_ we mark the patch-id as "seen". 3. Iterate again over the left-hand commits, checking whether commit->util->seen is set; if so, exclude them from the output. At the end, we'll have eliminated commits from both sides that have a matching patch-id on the other side. But there's a subtle assumption here: for any given patch-id, we must have exactly one struct representing it. If two commits from A both have the same patch-id and we allow duplicates in the hashmap, then we run into a problem: a. In step 1, we insert two patch-id structs into the hashmap. b. In step 2, our lookups will find only one of these structs, so only one "seen" flag is marked. c. In step 3, one of the commits in A will have its commit->util->seen set, but the other will not. We'll erroneously output the latter. Prior to dfb7a1b4d0, our hashmap did not allow duplicates. Afterwards, it used hashmap_add(), which explicitly does allow duplicates. At that point, the solution would have been easy: when we are about to add a duplicate, skip doing so and return the existing entry which matches. But it gets more complicated. In 683f17ec44 (patch-ids: replace the seen indicator with a commit pointer, 2016-07-29), our step 3 goes away entirely. Instead, in step 2, when the right-hand side finds a matching patch_id from the left-hand side, we can directly mark the left-hand patch_id->commit to be omitted. Solving that would be easy, too; there's a one-to-many relationship of patch-ids to commits, so we just need to keep a list. But there's more. Commit b3dfeebb92 (rebase: avoid computing unnecessary patch IDs, 2016-07-29) built on that by lazily computing the full patch-ids. So we don't even know when adding to the hashmap whether two commits truly have the same id. We'd have to tentatively assign them a list, and then possibly split them apart (possibly into N new structs) at the moment we compute the real patch-ids. This could work, but it's complicated and error-prone. Instead, let's accept that we may store duplicates, and teach the lookup side to be more clever. Rather than asking for a single matching patch-id, it will need to iterate over all matching patch-ids. This does mean examining every entry in a single hash bucket, but the worst-case for a hash lookup was already doing that. We'll keep the hashmap details out of the caller by providing a simple iteration interface. We can retain the simple has_commit_patch_id() interface for the other callers, but we'll simplify its return value into an integer, rather than returning the patch_id struct. That way they won't be tempted to look at the "commit" field of the return value without iterating. Reported-by: Arnaud Morin <arnaud.morin@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-09t5516: loosen "not our ref" error checkLibravatar Jeff King1-1/+3
Commit 014ade7484 (upload-pack: send ERR packet for non-tip objects, 2019-04-13) added a test that greps the output of a failed fetch to make sure that upload-pack sent us the ERR packet we expected. But checking this is racy; despite the argument in that commit, the client may still be sending a "done" line after the server exits, causing it to die() on a failed write() and never see the ERR packet at all. This fails quite rarely on Linux, but more often on macOS. However, it can be triggered reliably with: diff --git a/fetch-pack.c b/fetch-pack.c index 876f90c759..cf40de9092 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -489,6 +489,7 @@ static int find_common(struct fetch_negotiator *negotiator, done: trace2_region_leave("fetch-pack", "negotiation_v0_v1", the_repository); if (!got_ready || !no_done) { + sleep(1); packet_buf_write(&req_buf, "done\n"); send_request(args, fd[1], &req_buf); } This is a real user-visible race that it would be nice to fix, but it's tricky to do so: the client would have to speculatively try to read an ERR packet after hitting a write() error. And at least for this error, it's specific to v0 (since v2 does not enforce reachability at all). So let's loosen the test to avoid annoying racy failures. If we eventually do the read-after-failed-write thing, we can tighten it. And if not, v0 will grow increasingly obsolete as servers support v2, so the utility of this test will decrease over time anyway. Note that we can still check stderr to make sure upload-pack bailed for the reason we expected. It writes a similar message to stderr, and because the server side is just another process connected by pipes, we'll reliably see it. This would not be the case for git://, or for ssh servers that do not relay stderr (e.g., GitHub's custom endpoint does not). Helped-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-09t4129: fix setfacl-related permissions failureLibravatar Adam Dinwoodie1-1/+1
When running this test in Cygwin, it's necessary to remove the inherited access control lists from the Git working directory in order for later permissions tests to work as expected. As such, fix an error in the test script so that the ACLs are set for the working directory, not a nonexistent subdirectory. Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org> Reviewed-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-07for-each-repo: do nothing on empty configLibravatar Derrick Stolee1-0/+6
'git for-each-repo --config=X' should return success without calling any subcommands when the config key 'X' has no value. The current implementation instead segfaults. A user could run into this issue if they used 'git maintenance start' to initialize their cron schedule using 'git for-each-repo --config=maintenance.repo ...' but then using 'git maintenance unregister' to remove the config option. (Note: 'git maintenance stop' would remove the config _and_ remove the cron schedule.) Add a simple test to ensure this works. Use 'git help --no-such-option' as the potential subcommand to ensure that we will hit a failure if the subcommand is ever run. Reported-by: Andreas Bühmann <dev@uuml.de> Helped-by: Eric Sunshine <sunshine@sunshineco.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-07branch: show "HEAD detached" first under reverse sortLibravatar Ævar Arnfjörð Bjarmason1-3/+3
Change the output of the likes of "git branch -l --sort=-objectsize" to show the "(HEAD detached at <hash>)" message at the start of the output. Before the compare_detached_head() function added in a preceding commit we'd emit this output as an emergent effect. It doesn't make any sense to consider the objectsize, type or other non-attribute of the "(HEAD detached at <hash>)" message for the purposes of sorting. Let's always emit it at the top instead. The only reason it was sorted in the first place is because we're injecting it into the ref-filter machinery so builtin/branch.c doesn't need to do its own "am I detached?" detection. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-07branch: sort detached HEAD based on a flagLibravatar Ævar Arnfjörð Bjarmason1-2/+2
Change the ref-filter sorting of detached HEAD to check the FILTER_REFS_DETACHED_HEAD flag, instead of relying on the ref description filled-in by get_head_description() to start with "(", which in turn we expect to ASCII-sort before any other reference. For context, we'd like the detached line to appear first at the start of "git branch -l", e.g.: $ git branch -l * (HEAD detached at <hash>) master This doesn't change that, but improves on a fix made in 28438e84e04 (ref-filter: sort detached HEAD lines firstly, 2019-06-18) and gives the Chinese translation the ability to use its preferred punctuation marks again. In Chinese the fullwidth versions of punctuation like "()" are typically written as (U+FF08 fullwidth left parenthesis), (U+FF09 fullwidth right parenthesis) instead[1]. This form is used in both po/zh_{CN,TW}.po in most cases where "()" is translated in a string. Aside from that improvement to the Chinese translation, it also just makes for cleaner code that we mark any special cases in the ref_array we're sorting with flags and make the sort function aware of them, instead of piggy-backing on the general-case of strcmp() doing the right thing. As seen in the amended tests this made reverse sorting a bit more consistent. Before this we'd sometimes sort this message in the middle, now it's consistently at the beginning or end, depending on whether we're doing a normal or reverse sort. Having it at the end doesn't make much sense either, but at least it behaves consistently now. A follow-up commit will make this behavior under reverse sorting even better. I'm removing the "TRANSLATORS" comments that were in the old code while I'm at it. Those were added in d4919bb288e (ref-filter: move get_head_description() from branch.c, 2017-01-10). I think it's obvious from context, string and translation memory in typical translation tools that these are the same or similar string. 1. https://en.wikipedia.org/wiki/Chinese_punctuation#Marks_similar_to_European_punctuation Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-07fsck: reject .gitmodules git:// urls with newlinesLibravatar Jeff King1-0/+15
The previous commit taught the clone/fetch client side to reject a git:// URL with a newline in it. Let's also catch these when fscking a .gitmodules file, which will give an earlier warning. Note that it would be simpler to just complain about newline in _any_ URL, but an earlier tightening for http/ftp made sure we kept allowing newlines for unknown protocols (and this is covered in the tests). So we'll stick to that precedent. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-07git_connect_git(): forbid newlines in host and pathLibravatar Jeff King1-0/+5
When we connect to a git:// server, we send an initial request that looks something like: 002dgit-upload-pack repo.git\0host=example.com If the repo path contains a newline, then it's included literally, and we get: 002egit-upload-pack repo .git\0host=example.com This works fine if you really do have a newline in your repository name; the server side uses the pktline framing to parse the string, not newlines. However, there are many _other_ protocols in the wild that do parse on newlines, such as HTTP. So a carefully constructed git:// URL can actually turn into a valid HTTP request. For example: git://localhost:1234/%0d%0a%0d%0aGET%20/%20HTTP/1.1 %0d%0aHost:localhost%0d%0a%0d%0a becomes: 0050git-upload-pack / GET / HTTP/1.1 Host:localhost host=localhost:1234 on the wire. Again, this isn't a problem for a real Git server, but it does mean that feeding a malicious URL to Git (e.g., through a submodule) can cause it to make unexpected cross-protocol requests. Since repository names with newlines are presumably quite rare (and indeed, we already disallow them in git-over-http), let's just disallow them over this protocol. Hostnames could likewise inject a newline, but this is unlikely a problem in practice; we'd try resolving the hostname with a newline in it, which wouldn't work. Still, it doesn't hurt to err on the side of caution there, since we would not expect them to work in the first place. The ssh and local code paths are unaffected by this patch. In both cases we're trying to run upload-pack via a shell, and will quote the newline so that it makes it intact. An attacker can point an ssh url at an arbitrary port, of course, but unless there's an actual ssh server there, we'd never get as far as sending our shell command anyway. We _could_ similarly restrict newlines in those protocols out of caution, but there seems little benefit to doing so. The new test here is run alongside the git-daemon tests, which cover the same protocol, but it shouldn't actually contact the daemon at all. In theory we could make the test more robust by setting up an actual repository with a newline in it (so that our clone would succeed if our new check didn't kick in). But a repo directory with newline in it is likely not portable across all filesystems. Likewise, we could check git-daemon's log that it was not contacted at all, but we do not currently record the log (and anyway, it would make the test racy with the daemon's log write). We'll just check the client-side stderr to make sure we hit the expected code path. Reported-by: Harold Kim <h.kim@flatt.tech> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-06mergetool--lib: fix '--tool-help' to correctly show available toolsLibravatar Philippe Blain1-0/+11
Commit 83bbf9b92e (mergetool--lib: improve support for vimdiff-style tool variants, 2020-07-29) introduced a regression in the output of `git mergetool --tool-help` and `git difftool --tool-help` [1]. In function 'show_tool_names' in git-mergetool--lib.sh, we loop over the supported mergetools and their variants and accumulate them in the variable 'variants', separating them with a literal '\n'. The code then uses 'echo $variants' to turn these '\n' into newlines, but this behaviour is not portable, it just happens to work in some shells, like dash(1)'s 'echo' builtin. For shells in which 'echo' does not turn '\n' into newlines, the end result is that the only tools that are shown are the existing variants (except the last variant alphabetically), since the variants are separated by actual newlines in '$variants' because of the several 'echo' calls in mergetools/{bc,vimdiff}::list_tool_variants. Fix this bug by embedding an actual line feed into `variants` in show_tool_names(). While at it, replace `sort | uniq` by `sort -u`. To prevent future regressions, add a simple test that checks that a few known tools are correctly shown (let's avoid counting the total number of tools to lessen the maintenance burden when new tools are added or if '--tool-help' learns additional logic, like hiding tools depending on the current platform). [1] https://lore.kernel.org/git/CADtb9DyozjgAsdFYL8fFBEWmq7iz4=prZYVUdH9W-J5CKVS4OA@mail.gmail.com/ Reported-by: Philippe Blain <levraiphilippeblain@gmail.com> Based-on-patch-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-06t4129: don't fail if setgid is set in the test directoryLibravatar Matheus Tavares1-2/+7
The last test of t4129 creates a directory and expects its setgid bit (g+s) to be off. But this makes the test fail when the parent directory has the bit set, as setgid's state is inherited by newly created subdirectories. One way to solve this problem is to allow the presence of this bit when comparing the return of `test_modebits` with the expected value. But then we may have the same problem in the future when other tests start using `test_modebits` on directories (currently t4129 is the only one) and forget about setgid. Instead, let's make the helper function more robust with respect to the state of the setgid bit in the test directory by removing this bit from the returning value. There should be no problem with existing callers as no one currently expects this bit to be on. Note that the sticky bit (+t) and the setuid bit (u+s) are not inherited, so we don't have to worry about those. Reported-by: Kevin Daudt <me@ikke.info> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-06branch tests: add to --sort testsLibravatar Ævar Arnfjörð Bjarmason1-1/+50
Further stress the --sort callback in ref-filter.c. The implementation uses certain short-circuiting logic, let's make sure it behaves the same way on e.g. name & version sort. Improves a test added in aedcb7dc75e (branch.c: use 'ref-filter' APIs, 2015-09-23). I don't think all of this output makes sense, but let's test for the behavior as-is, we can fix bugs in it in a later commit. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-06p7519: allow running without watchman prereqLibravatar Taylor Blau1-1/+6
p7519 measures the performance of the fsmonitor code. To do this, it uses the installed copy of Watchman. If Watchman isn't installed, a noop integration script is installed in its place. When in the latter mode, it is expected that the script should not write a "last update token": in fact, it doesn't write anything at all since the script is blank. Commit 33226af42b (t/perf/fsmonitor: improve error message if typoing hook name, 2020-10-26) made sure that running 'git update-index --fsmonitor' did not write anything to stderr, but this is not the case when using the empty Watchman script, since Git will complain that: $ which watchman watchman not found $ cat .git/hooks/fsmonitor-empty $ git -c core.fsmonitor=.git/hooks/fsmonitor-empty update-index --fsmonitor warning: Empty last update token. Prior to 33226af42b, the output wasn't checked at all, which allowed this noop mode to work. But, 33226af42b breaks p7519 when running it without a 'watchman(1)' on your system. Handle this by only checking that the stderr is empty only when running with a real watchman executable. Otherwise, assert that the error message is the expected one when running in the noop mode. Signed-off-by: Taylor Blau <me@ttaylorr.com> Acked-by: Nipunn Koorapati <nipunn@dropbox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-04t6016: move to lib-log-graph.sh frameworkLibravatar Antonio Russo1-187/+167
t6016 manually reconstructs git log --graph output by using the reported commit hashes from `git rev-parse`. Each tag is converted into an environment variable manually, and then `echo`-ed to an expected output file, which is in turn compared to the actual output. The expected output is difficult to read and write, because, e.g., each line of output must be prefaced with echo, quoted, and properly escaped. Additionally, the test is sensitive to trailing whitespace, which may potentially be removed from graph log output in the future. In order to reduce duplication, ease troubleshooting of failed tests by improving readability, and ease the addition of more tests to this file, port the operations to `lib-log-graph.sh`, which is already used in several other tests, e.g., t4215. Give all merges a simple commit message, and use a common `check_graph` macro taking a heredoc of the expected output which does not required extensive escaping. Signed-off-by: Antonio Russo <aerusso@aerusso.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-04t1300: don't needlessly work with `core.foo` configsLibravatar Martin Ågren1-30/+30
We use various made-up config keys in the "core" section for no real reason. Change them to work in the "section" section instead and be careful to also change "cores" to "sections". Make sure to also catch "Core", "CoReS" and similar. There are a few instances that actually want to work with a real "core" config such as `core.bare` or `core.editor`. After this, it's clearer that they work with "core" for a reason. Reported-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-04t1300: remove duplicate test for `--file no-such-file`Libravatar Martin Ågren1-6/+2
We test that we can handle `git config --file symlink` and the error case of `git config --file symlink-to-missing-file`. For good measure, we also throw in a test to check that we correctly handle referencing a missing regular file. But we have such a test earlier in this script. They both check that we fail to use `--file no-such-file --list`. Drop the latter of these and keep the one that is in the general area where we test `--file` and `GIT_CONFIG`. The one we're dropping also checks that we can't even get a specific key from the missing file -- let's make sure we check that in the test we keep. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-04t1300: remove duplicate test for `--file ../foo`Libravatar Martin Ågren1-4/+0
We have two tests for checking that we can handle `git config --file ../other-config ...`. One, using `--file`, was introduced in 65807ee697 ("builtin-config: Fix crash when using "-f <relative path>" from non-root dir", 2010-01-26), then another, using `GIT_CONFIG`, came about in 270a34438b ("config: stop using config_exclusive_filename", 2012-02-16). The latter of these was then converted to use `--file` in f7e8714101 ("t: prefer "git config --file" to GIT_CONFIG", 2014-03-20). Both where then simplified in a5db0b77b9 ("t1300: extract and use test_cmp_config()", 2018-10-21). These two tests differ slightly in the order of the options used, but other than that, they are identical. Let's drop one. As noted in f7e8714101, we do still have a test for `GIT_CONFIG` and it shares the implementation with `--file`. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-12-23Merge branch 'nk/refspecs-negative-fix'Libravatar Junio C Hamano1-0/+51
Hotfix for recent regression. * nk/refspecs-negative-fix: negative-refspec: improve comment on query_matches_negative_refspec negative-refspec: fix segfault on : refspec