summaryrefslogtreecommitdiff
path: root/t
AgeCommit message (Collapse)AuthorFilesLines
2018-07-11sequencer: handle empty-set cases consistentlyLibravatar Jeff King1-1/+6
If the user gives us a set that prepare_revision_walk() takes to be empty, like: git cherry-pick base..base then we report an error. It's nonsense, and there's nothing to pick. But if they use revision options that later cull the list, like: git cherry-pick --author=nobody base~2..base then we quietly create an empty todo list and return success. Arguably either behavior is acceptable, but we should definitely be consistent about it. Reporting an error seems to match the original intent, which dates all the way back to 7e2bfd3f99 (revert: allow cherry-picking more than one commit, 2010-06-02). That in turn was trying to match the single-commit case that existed before then (and which continues to issue an error). Signed-off-by: Jeff King <peff@peff.net> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-22Sync with Git 2.15.2Libravatar Junio C Hamano3-0/+182
* maint-2.15: Git 2.15.2 Git 2.14.4 Git 2.13.7 verify_path: disallow symlinks in .gitmodules update-index: stat updated files earlier verify_dotfile: mention case-insensitivity in comment verify_path: drop clever fallthrough skip_prefix: add case-insensitive variant is_{hfs,ntfs}_dotgitmodules: add tests is_ntfs_dotgit: match other .git files is_hfs_dotgit: match other .git files is_ntfs_dotgit: use a size_t for traversing string submodule-config: verify submodule names as paths
2018-05-22Sync with Git 2.14.4Libravatar Junio C Hamano3-0/+182
* maint-2.14: Git 2.14.4 Git 2.13.7 verify_path: disallow symlinks in .gitmodules update-index: stat updated files earlier verify_dotfile: mention case-insensitivity in comment verify_path: drop clever fallthrough skip_prefix: add case-insensitive variant is_{hfs,ntfs}_dotgitmodules: add tests is_ntfs_dotgit: match other .git files is_hfs_dotgit: match other .git files is_ntfs_dotgit: use a size_t for traversing string submodule-config: verify submodule names as paths
2018-05-22Sync with Git 2.13.7Libravatar Junio C Hamano3-0/+182
* maint-2.13: Git 2.13.7 verify_path: disallow symlinks in .gitmodules update-index: stat updated files earlier verify_dotfile: mention case-insensitivity in comment verify_path: drop clever fallthrough skip_prefix: add case-insensitive variant is_{hfs,ntfs}_dotgitmodules: add tests is_ntfs_dotgit: match other .git files is_hfs_dotgit: match other .git files is_ntfs_dotgit: use a size_t for traversing string submodule-config: verify submodule names as paths
2018-05-21is_{hfs,ntfs}_dotgitmodules: add testsLibravatar Johannes Schindelin2-0/+106
This tests primarily for NTFS issues, but also adds one example of an HFS+ issue. Thanks go to Congyi Wu for coming up with the list of examples where NTFS would possibly equate the filename with `.gitmodules`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff King <peff@peff.net>
2018-05-21submodule-config: verify submodule names as pathsLibravatar Jeff King1-0/+76
Submodule "names" come from the untrusted .gitmodules file, but we blindly append them to $GIT_DIR/modules to create our on-disk repo paths. This means you can do bad things by putting "../" into the name (among other things). Let's sanity-check these names to avoid building a path that can be exploited. There are two main decisions: 1. What should the allowed syntax be? It's tempting to reuse verify_path(), since submodule names typically come from in-repo paths. But there are two reasons not to: a. It's technically more strict than what we need, as we really care only about breaking out of the $GIT_DIR/modules/ hierarchy. E.g., having a submodule named "foo/.git" isn't actually dangerous, and it's possible that somebody has manually given such a funny name. b. Since we'll eventually use this checking logic in fsck to prevent downstream repositories, it should be consistent across platforms. Because verify_path() relies on is_dir_sep(), it wouldn't block "foo\..\bar" on a non-Windows machine. 2. Where should we enforce it? These days most of the .gitmodules reads go through submodule-config.c, so I've put it there in the reading step. That should cover all of the C code. We also construct the name for "git submodule add" inside the git-submodule.sh script. This is probably not a big deal for security since the name is coming from the user anyway, but it would be polite to remind them if the name they pick is invalid (and we need to expose the name-checker to the shell anyway for our test scripts). This patch issues a warning when reading .gitmodules and just ignores the related config entry completely. This will generally end up producing a sensible error, as it works the same as a .gitmodules file which is missing a submodule entry (so "submodule update" will barf, but "git clone --recurse-submodules" will print an error but not abort the clone. There is one minor oddity, which is that we print the warning once per malformed config key (since that's how the config subsystem gives us the entries). So in the new test, for example, the user would see three warnings. That's OK, since the intent is that this case should never come up outside of malicious repositories (and then it might even benefit the user to see the message multiple times). Credit for finding this vulnerability and the proof of concept from which the test script was adapted goes to Etienne Stalmans. Signed-off-by: Jeff King <peff@peff.net>
2018-03-22Merge branch 'jk/cached-commit-buffer' into maintLibravatar Junio C Hamano1-31/+0
Code clean-up. * jk/cached-commit-buffer: revision: drop --show-all option commit: drop uses of get_cached_commit_buffer()
2018-03-22Merge branch 'sm/mv-dry-run-update' into maintLibravatar Junio C Hamano1-0/+6
Code clean-up. * sm/mv-dry-run-update: mv: remove unneeded 'if (!show_only)' t7001: add test case for --dry-run
2018-03-22Merge branch 'gs/test-unset-xdg-cache-home' into maintLibravatar Junio C Hamano1-0/+1
Test update. * gs/test-unset-xdg-cache-home: test-lib.sh: unset XDG_CACHE_HOME
2018-03-22Merge branch 'rd/typofix' into maintLibravatar Junio C Hamano2-2/+2
Typofix. * rd/typofix: Correct mispellings of ".gitmodule" to ".gitmodules" t/: correct obvious typo "detahced"
2018-03-22Merge branch 'sg/doc-test-must-fail-args' into maintLibravatar Junio C Hamano2-2/+22
Devdoc update. * sg/doc-test-must-fail-args: t: document 'test_must_fail ok=<signal-name>'
2018-03-22Merge branch 'jk/gettext-poison' into maintLibravatar Junio C Hamano1-4/+0
Test updates. * jk/gettext-poison: git-sh-i18n: check GETTEXT_POISON before USE_GETTEXT_SCHEME t0205: drop redundant test
2018-03-22Merge branch 'nd/shared-index-fix' into maintLibravatar Junio C Hamano1-0/+19
Code clean-up. * nd/shared-index-fix: read-cache: don't write index twice if we can't write shared index read-cache.c: move tempfile creation/cleanup out of write_shared_index read-cache.c: change type of "temp" in write_shared_index()
2018-03-22Merge branch 'cl/t9001-cleanup' into maintLibravatar Junio C Hamano1-10/+7
Test clean-up. * cl/t9001-cleanup: t9001: use existing helper in send-email test
2018-03-22Merge branch 'sg/test-i18ngrep' into maintLibravatar Junio C Hamano8-43/+72
Test fixes. * sg/test-i18ngrep: t: make 'test_i18ngrep' more informative on failure t: validate 'test_i18ngrep's parameters t: move 'test_i18ncmp' and 'test_i18ngrep' to 'test-lib-functions.sh' t5536: let 'test_i18ngrep' read the file without redirection t5510: consolidate 'grep' and 'test_i18ngrep' patterns t4001: don't run 'git status' upstream of a pipe t6022: don't run 'git merge' upstream of a pipe t5812: add 'test_i18ngrep's missing filename parameter t5541: add 'test_i18ngrep's missing filename parameter
2018-03-22Merge branch 'jk/daemon-fixes' into maintLibravatar Junio C Hamano3-11/+101
Assorted fixes to "git daemon". * jk/daemon-fixes: daemon: fix length computation in newline stripping t/lib-git-daemon: add network-protocol helpers daemon: handle NULs in extended attribute string daemon: fix off-by-one in logging extended attributes t/lib-git-daemon: record daemon log t5570: use ls-remote instead of clone for interp tests
2018-03-22Merge branch 'tg/split-index-fixes' into maintLibravatar Junio C Hamano1-0/+19
The split-index mode had a few corner case bugs fixed. * tg/split-index-fixes: travis: run tests with GIT_TEST_SPLIT_INDEX split-index: don't write cache tree with null oid entries read-cache: fix reading the shared index for other repos
2018-03-22Merge branch 'jt/http-redact-cookies' into maintLibravatar Junio C Hamano1-0/+33
The http tracing code, often used to debug connection issues, learned to redact potentially sensitive information from its output so that it can be more safely sharable. * jt/http-redact-cookies: http: support omitting data from traces http: support cookie redaction when tracing
2018-02-27Merge branch 'sb/submodule-update-reset-fix' into maintLibravatar Junio C Hamano1-2/+17
When resetting the working tree files recursively, the working tree of submodules are now also reset to match. * sb/submodule-update-reset-fix: submodule: submodule_move_head omits old argument in forced case unpack-trees: oneway_merge to update submodules t/lib-submodule-update.sh: fix test ignoring ignored files in submodules t/lib-submodule-update.sh: clarify test
2018-02-27Merge branch 'ab/commit-m-with-fixup' into maintLibravatar Junio C Hamano1-1/+8
"git commit --fixup" did not allow "-m<message>" option to be used at the same time; allow it to annotate resulting commit with more text. * ab/commit-m-with-fixup: commit: add support for --fixup <commit> -m"<extra message>" commit doc: document that -c, -C, -F and --fixup with -m error
2018-02-27Merge branch 'nd/ita-wt-renames-in-status' into maintLibravatar Junio C Hamano1-0/+72
"git status" after moving a path in the working tree (hence making it appear "removed") and then adding with the -N option (hence making that appear "added") detected it as a rename, but did not report the old and new pathnames correctly. * nd/ita-wt-renames-in-status: wt-status.c: handle worktree renames wt-status.c: rename rename-related fields in wt_status_change_data wt-status.c: catch unhandled diff status codes wt-status.c: coding style fix Use DIFF_DETECT_RENAME for detect_rename assignments t2203: test status output with porcelain v2 format
2018-02-22revision: drop --show-all optionLibravatar Jeff King1-31/+0
This was an undocumented debugging aid that does not seem to have come in handy in the past decade, judging from its lack of mentions on the mailing list. Let's drop it in the name of simplicity. This is morally a revert of 3131b71301 (Add "--show-all" revision walker flag for debugging, 2008-02-09), but note that I did leave in the mapping of UNINTERESTING to "^" in get_revision_mark(). I don't think this would be possible to trigger with the current code, but it's the only sensible marker. We'll skip the usual deprecation period because this was explicitly a debugging aid that was never documented. Signed-off-by: Jeff King <peff@peff.net> Acked-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-16test-lib.sh: unset XDG_CACHE_HOMELibravatar Genki Sky1-0/+1
git respects XDG_CACHE_HOME for the credential cache. So, we should unset XDG_CACHE_HOME for the test environment, lest a user's custom one cause failure in the test. For example, t/t0301-credential-cache.sh expects a default directory to be used if it hasn't explicitly set XDG_CACHE_HOME. Signed-off-by: Genki Sky <sky@genki.is> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-15Merge branch 'nd/add-i-ignore-submodules' into maintLibravatar Junio C Hamano1-0/+48
"git add -p" was taught to ignore local changes to submodules as they do not interfere with the partial addition of regular changes anyway. * nd/add-i-ignore-submodules: add--interactive: ignore submodule changes except HEAD
2018-02-15Merge branch 'tg/stash-with-pathspec-fix' into maintLibravatar Junio C Hamano1-0/+32
"git stash -- <pathspec>" incorrectly blew away untracked files in the directory that matched the pathspec, which has been corrected. * tg/stash-with-pathspec-fix: stash: don't delete untracked files that match pathspec
2018-02-15Merge branch 'jk/abort-clone-with-existing-dest' into maintLibravatar Junio C Hamano1-26/+74
"git clone $there $here" is allowed even when here directory exists as long as it is an empty directory, but the command incorrectly removed it upon a failure of the operation. * jk/abort-clone-with-existing-dest: clone: do not clean up directories we didn't create clone: factor out dir_exists() helper t5600: modernize style t5600: fix outdated comment about unborn HEAD
2018-02-15Merge branch 'jc/merge-symlink-ours-theirs' into maintLibravatar Junio C Hamano1-0/+32
"git merge -Xours/-Xtheirs" learned to use our/their version when resolving a conflicting updates to a symbolic link. * jc/merge-symlink-ours-theirs: merge: teach -Xours/-Xtheirs to symbolic link merge
2018-02-15Merge branch 'dk/describe-all-output-fix' into maintLibravatar Junio C Hamano1-1/+5
An old regression in "git describe --all $annotated_tag^0" has been fixed. * dk/describe-all-output-fix: describe: prepend "tags/" when describing tags with embedded name
2018-02-15Merge branch 'ab/perf-grep-threads' into maintLibravatar Junio C Hamano2-21/+86
More perf tests for threaded grep * ab/perf-grep-threads: perf: amend the grep tests to test grep.threads
2018-02-14Correct mispellings of ".gitmodule" to ".gitmodules"Libravatar Robert P. J. Day1-1/+1
There are a small number of misspellings, ".gitmodule", scattered throughout the code base, correct them ... no apparent functional changes. Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-14t/: correct obvious typo "detahced"Libravatar Robert P. J. Day1-1/+1
Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-12t: document 'test_must_fail ok=<signal-name>'Libravatar SZEDER Gábor2-2/+22
Since 'test_might_fail' is implemented as a thin wrapper around 'test_must_fail', it also accepts the same options. Mention this in the docs as well. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-08t: make 'test_i18ngrep' more informative on failureLibravatar SZEDER Gábor1-4/+20
When 'test_i18ngrep' can't find the expected pattern, it exits completely silently; when its negated form does find the pattern that shouldn't be there, it prints the matching line(s) but otherwise exits without any error message. This leaves the developer puzzled about what could have gone wrong. Make 'test_i18ngrep' more informative on failure by printing an error message including the invoked 'grep' command and the contents of the file it had to scan through. Note that this "dump the scanned file" part is not quite perfect, as it dumps only the file specified as the function's last positional parameter, thus assuming that there is only a single file parameter. I think that's a reasonable assumption to make, one that holds true in the current code base. And even if someone were to scan multiple files at once in the future, the worst thing that could happen is that the verbose error message won't include the contents of all those files, only the last one. Alas, we can't really do any better than this, because checking whether the other positional parameters match a filename can result in false positives: 't3400-rebase.sh' and 't3404-rebase-interactive.sh' contain one test each, where the 'test_i18ngrep's pattern verbatimly matches a file in the trash directory. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-08t: validate 'test_i18ngrep's parametersLibravatar SZEDER Gábor1-0/+12
Some of the previous patches in this series fixed bogus 'test_i18ngrep' invocations: - Two invocations where the tested git command's standard output is directly piped into 'test_i18ngrep'. While convenient, this is an antipattern, because the pipe hides the git command's exit code, and the test could continue even if the command exited with error. - Two invocations that had neither a filename parameter nor anything piped into their standard input, yet both managed to remain unnoticed for years. A third similarly bogus invocation is currently lurking in 'pu' for a couple of weeks now. Prevent similar mistakes in the future by validating 'test_i18ngrep's parameters requiring that - The last parameter names an existing file to be read, effectively forbidding piping into 'test_i18ngrep'. Note that this change will also forbid cases where 'test_i18ngrep' would legitimately read its standard input, e.g. when its standard input is redirected from a file, or when a git command's standard output is first written to an intermediate file, which is then preprocessed by a non-git command before the results are piped into 'test_i18ngrep'. See two of the previous patches for the only such cases we had in our test suite. However, reliably preventing the piping antipattern is arguably more important than supporting these cases, which can be easily worked around by opening the file directly or using an intermediate file anyway. - There are at least two parameters, not including the optional '!' to negate the pattern. This ought to catch corner cases when 'test_i18ngrep' looks for the name of an existing file on its standard input; the above check would miss this case becase the filename as pattern would be the last parameter. Note that this is not quite perfect, as it doesn't account for any 'grep --options' given as parameters. However, doing so would be far too complicated, considering that patterns can start with dashes as well, and in the majority of the cases we don't use any such options anyway. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-08t: move 'test_i18ncmp' and 'test_i18ngrep' to 'test-lib-functions.sh'Libravatar SZEDER Gábor2-26/+26
Both 'test_i18ncmp' and 'test_i18ngrep' helper functions are supposed to be called from our test scripts, so they should be in 'test-lib-functions.sh'. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-08t5536: let 'test_i18ngrep' read the file without redirectionLibravatar SZEDER Gábor1-1/+1
Redirecting 'test_i18ngrep's standard input from a file will interfere with the linting that will be added in a later patch. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-08t5510: consolidate 'grep' and 'test_i18ngrep' patternsLibravatar SZEDER Gábor1-6/+3
One of the tests in 't5510-fetch.sh' checks the output of 'git fetch' using 'test_i18ngrep', and while doing so it prefilters the output with 'grep' before piping the result into 'test_i18ngrep'. This prefiltering is unnecessary, with the appropriate pattern 'test_i18ngrep' can do it all by itself. Furthermore, piping data into 'test_i18ngrep' will interfere with the linting that will be added in a later patch. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-08t4001: don't run 'git status' upstream of a pipeLibravatar SZEDER Gábor1-3/+8
The primary purpose of three tests in 't4001-diff-rename.sh' is to check rename detection in 'git status', but all three do so by running 'git status' upstream of a pipe, hiding its exit code. Consequently, the test could continue even if 'git status' exited with error. Use an intermediate file between 'git status' and 'test_i18ngrep' to catch a potential failure of the former. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-08t6022: don't run 'git merge' upstream of a pipeLibravatar SZEDER Gábor1-2/+4
The primary purpose of 't6022-merge-rename.sh' is to test 'git merge', but one of the tests runs it upstream of a pipe, hiding its exit code. Consequently, the test could continue even if 'git merge' exited with error. Use an intermediate file between 'git merge' and 'test_i18ngrep' to catch a potential failure of the former. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-08t5812: add 'test_i18ngrep's missing filename parameterLibravatar SZEDER Gábor1-4/+1
The second 'test_i18ngrep' invocation in the test 'curl redirects respect whitelist' is missing its filename parameter. This has remained unnoticed since its introduction in f4113cac0 (http: limit redirection to protocol-whitelist, 2015-09-22), because it would only cause the test to fail if Git was built with a sufficiently old libcurl version. The test's two ||-chained 'test_i18ngrep' invocations are supposed to check that either one of the two patterns is present in 'git clone's error message. As it happens, the first invocation covers the error message from any reasonably up-to-date libcurl, thus the second invocation, the one without the filename parameter, isn't executed at all. Apparently no one has run the test suite's httpd tests with such an old libcurl in the last 2+ years, or at least they haven't bothered to notify us about the failed test. Fix this by consolidating the two patterns into a single extended regexp, eliminating the need for an ||-chained second 'test_i18ngrep' invocation. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-08t5541: add 'test_i18ngrep's missing filename parameterLibravatar SZEDER Gábor1-1/+1
The test 'push --no-progress silences progress but not status' runs 'test_i18ngrep' without specifying a filename parameter. This has remained unnoticed since its introduction in e304aeba2 (t5541: test more combinations of --progress, 2012-05-01), because that 'test_i18ngrep' is supposed to check that the given pattern is not present in its input, and of course it won't find that pattern if its input is empty (as it comes from /dev/null). This also means that this test could miss a potential breakage of 'git push --no-progress'. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-08t0205: drop redundant testLibravatar Jeff King1-4/+0
We check that a shell variable is non-empty, and then we check that it's equal to a particular value. Just checking the latter covers both cases. I suspect the original was trying to give better output when the test fails, but using "-x" covers that these days. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-07t7001: add test case for --dry-runLibravatar Stefan Moch1-0/+6
Make sure that "git mv --dry-run" does not move file. Signed-off-by: Stefan Moch <stefanmoch@mail.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-25daemon: fix length computation in newline strippingLibravatar Jeff King1-0/+15
When git-daemon gets a pktline request, we strip off any trailing newline, replacing it with a NUL. Clients prior to 5ad312bede (in git v1.4.0) would send: git-upload-pack repo.git\n and we need to strip it off to understand their request. After 5ad312bede, we send the host attribute but no newline, like: git-upload-pack repo.git\0host=example.com\0 Both of these are parsed correctly by git-daemon. But if some client were to combine the two: git-upload-pack repo.git\n\0host=example.com\0 we don't parse it correctly. The problem is that we use the "len" variable to record the position of the NUL separator, but then decrement it when we strip the newline. So we start with: git-upload-pack repo.git\n\0host=example.com\0 ^-- len and end up with: git-upload-pack repo.git\0\0host=example.com\0 ^-- len This is arguably correct, since "len" tells us the length of the initial string, but we don't actually use it for that. What we do use it for is finding the offset of the extended attributes; they used to be at len+1, but are now at len+2. We can solve that by just leaving "len" where it is. We don't have to care about the length of the shortened string, since we just treat it like a C string. No version of Git ever produced such a string, but it seems like the daemon code meant to handle this case (and it seems like a reasonable thing for somebody to do in a 3rd-party implementation). Reported-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-25t/lib-git-daemon: add network-protocol helpersLibravatar Jeff King2-1/+58
All of our git-protocol tests rely on invoking the client and having it make a request of a server. That gives a nice real-world test of how the two behave together, but it doesn't leave any room for testing how a server might react to _other_ clients. Let's add a few test helper functions which can be used to manually conduct a git-protocol conversation with a remote git-daemon: 1. To connect to a remote git-daemon, we need something like "netcat". But not everybody will have netcat. And even if they do, the behavior with respect to half-duplex shutdowns is not portable (openbsd netcat has "-N", with others you must rely on "-q 1", which is racy). Here we provide a "fake_nc" that is capable of doing a client-side netcat, with sane half-duplex semantics. It relies on perl's IO::Socket::INET. That's been in the base distribution since 5.6.0, so it's probably available everywhere. But just to be on the safe side, we'll add a prereq. 2. To help tests speak and read pktline, this patch adds packetize() and depacketize() functions. I've put fake_nc() into lib-git-daemon.sh, since that's really the only server where we'd need to use a network socket. Whereas the pktline helpers may be of more general use, so I've added them to test-lib-functions.sh. Programs like upload-pack speak pktline, but can talk directly over stdio without a network socket. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-25daemon: handle NULs in extended attribute stringLibravatar Jeff King1-3/+5
If we receive a request with extended attributes after the NUL, we try to write those attributes to the log. We do so with a "%s" format specifier, which will only show characters up to the first NUL. That's enough for printing a "host=" specifier. But since dfe422d04d (daemon: recognize hidden request arguments, 2017-10-16) we may have another NUL, followed by protocol parameters, and those are not logged at all. Let's cut out the attempt to show the whole string, and instead log when we parse individual attributes. We could leave the "extended attributes (%d bytes) exist" part of the log, which in theory could alert us to attributes that fail to parse. But anything we don't parse as a "host=" parameter gets blindly added to the "protocol" attribute, so we'd see it in that part of the log. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-25daemon: fix off-by-one in logging extended attributesLibravatar Jeff King1-0/+11
If receive a request like: git-upload-pack /foo.git\0host=localhost we mark the offset of the NUL byte as "len", and then log the bytes after the NUL with a "%.*s" placeholder, using "pktlen - len" as the length, and "line + len + 1" as the start of the string. This is off-by-one, since the start of the string skips past the separating NUL byte, but the adjusted length includes it. Fortunately this doesn't actually read past the end of the buffer, since "%.*s" will stop when it hits a NUL. And regardless of what is in the buffer, packet_read() will always add an extra NUL terminator for safety. As an aside, the git.git client sends an extra NUL after a "host" field, too, so we'd generally hit that one first, not the one added by packet_read(). You can see this in the test output which reports 15 bytes, even though the string has only 14 bytes of visible data. But the point is that even a client sending unusual data could not get us to read past the end of the buffer, so this is purely a cosmetic fix. Reported-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-25t/lib-git-daemon: record daemon logLibravatar Jeff King1-4/+12
When we start git-daemon for our tests, we send its stderr log stream to a named pipe. We synchronously read the first line to make sure that the daemon started, and then dump the rest to descriptor 4. This is handy for debugging test output with "--verbose", but the tests themselves can't access the log data. Let's dump the log into a file, as well, so that future tests can check the log. There are a few subtleties worth calling out here: - we'll continue to send output to descriptor 4 for viewing/debugging, which would imply swapping out "cat" for "tee". But we want to ensure that there's no buffering, and "tee" doesn't have a standard way to ask for that. So we'll use a shell loop around "read" and "printf" instead. That ensures that after a request has been served, the matching log entries will have made it to the file. - the existing first-line shell loop used read/echo. We'll switch to consistently using "read -r" and "printf" to relay data as faithfully as possible. - we open the logfile for append, rather than just output. That makes it OK for tests to truncate the logfile without restarting the daemon (the OS will atomically seek to the end of the file when outputting each line). That allows tests to look at the log without worrying about pollution from earlier tests. Helped-by: Lucas Werkmeister <mail@lucaswerkmeister.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-25t5570: use ls-remote instead of clone for interp testsLibravatar Jeff King1-6/+3
We don't actually care about the clone operation here; we just want to know if we were able to actually contact the remote repository. Using ls-remote does that more efficiently, and without us having to worry about managing the tmp.git directory. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-24read-cache: don't write index twice if we can't write shared indexLibravatar Nguyễn Thái Ngọc Duy1-0/+19
In a0a967568e ("update-index --split-index: do not split if $GIT_DIR is read only", 2014-06-13), we tried to make sure we can still write an index, even if the shared index can not be written. We did so by just calling 'do_write_locked_index()' just before 'write_shared_index()'. 'do_write_locked_index()' always at least closes the tempfile nowadays, and used to close or commit the lockfile if COMMIT_LOCK or CLOSE_LOCK were given at the time this feature was introduced. COMMIT_LOCK or CLOSE_LOCK is passed in by most callers of 'write_locked_index()'. After calling 'write_shared_index()', we call 'write_split_index()', which calls 'do_write_locked_index()' again, which then tries to use the closed lockfile again, but in fact fails to do so as it's already closed. This eventually leads to a segfault. Make sure to write the main index only once. [nd: most of the commit message and investigation done by Thomas, I only tweaked the solution a bit] Helped-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>