Commit Graph

12978 Commits

Author SHA1 Message Date
Nguyễn Thái Ngọc Duy
e34de73c56 upload-pack: reject shallow requests that would return nothing
Shallow clones with --shallow-since or --shalow-exclude work by
running rev-list to get all reachable commits, then draw a boundary
between reachable and unreachable and send "shallow" requests based on
that.

The code does miss one corner case: if rev-list returns nothing, we'll
have no border and we'll send no shallow requests back to the client
(i.e. no history cuts). This essentially means a full clone (or a full
branch if the client requests just one branch). One example is the
oldest commit is older than what is specified by --shallow-since.

To avoid this, if rev-list returns nothing, we abort the clone/fetch.
The user could adjust their request (e.g. --shallow-since further back
in the past) and retry.

Another possible option for this case is to fall back to a default
depth (like depth 1). But I don't like too much magic that way because
we may return something unexpected to the user. If they request
"history since 2008" and we return a single depth at 2000, that might
break stuff for them. It is better to tell them that something is
wrong and let them take the best course of action.

Note that we need to die() in get_shallow_commits_by_rev_list()
instead of just checking for empty result from its caller
deepen_by_rev_list() and handling the error there. The reason is,
empty result could be a valid case: if you have commits in year 2013
and you request --shallow-since=year.2000 then you should get a full
clone (i.e. empty result).

Reported-by: Andreas Krey <a.krey@gmx.de>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-04 11:03:10 +09:00
Junio C Hamano
023020401d Sync with Git 2.15.2
* 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-22 14:18:06 +09:00
Junio C Hamano
9e0f06d55d Sync with Git 2.14.4
* 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-22 14:15:14 +09:00
Junio C Hamano
7b01c71b64 Sync with Git 2.13.7
* 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-22 14:10:49 +09:00
Johannes Schindelin
dc2d9ba318 is_{hfs,ntfs}_dotgitmodules: add tests
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-21 23:50:11 -04:00
Jeff King
0383bbb901 submodule-config: verify submodule names as paths
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-05-21 23:50:11 -04:00
Junio C Hamano
393eee1cad Merge branch 'jk/cached-commit-buffer' into maint
Code clean-up.

* jk/cached-commit-buffer:
  revision: drop --show-all option
  commit: drop uses of get_cached_commit_buffer()
2018-03-22 14:24:25 -07:00
Junio C Hamano
c9bc2c5d4d Merge branch 'sm/mv-dry-run-update' into maint
Code clean-up.

* sm/mv-dry-run-update:
  mv: remove unneeded 'if (!show_only)'
  t7001: add test case for --dry-run
2018-03-22 14:24:25 -07:00
Junio C Hamano
8bfeb0e42c Merge branch 'gs/test-unset-xdg-cache-home' into maint
Test update.

* gs/test-unset-xdg-cache-home:
  test-lib.sh: unset XDG_CACHE_HOME
2018-03-22 14:24:24 -07:00
Junio C Hamano
9ea8e0ca81 Merge branch 'rd/typofix' into maint
Typofix.

* rd/typofix:
  Correct mispellings of ".gitmodule" to ".gitmodules"
  t/: correct obvious typo "detahced"
2018-03-22 14:24:22 -07:00
Junio C Hamano
68559c464a Merge branch 'sg/doc-test-must-fail-args' into maint
Devdoc update.

* sg/doc-test-must-fail-args:
  t: document 'test_must_fail ok=<signal-name>'
2018-03-22 14:24:20 -07:00
Junio C Hamano
2e1062d30f Merge branch 'jk/gettext-poison' into maint
Test updates.

* jk/gettext-poison:
  git-sh-i18n: check GETTEXT_POISON before USE_GETTEXT_SCHEME
  t0205: drop redundant test
2018-03-22 14:24:19 -07:00
Junio C Hamano
3112c3fa7f Merge branch 'nd/shared-index-fix' into maint
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-22 14:24:16 -07:00
Junio C Hamano
b1bdf46bb8 Merge branch 'cl/t9001-cleanup' into maint
Test clean-up.

* cl/t9001-cleanup:
  t9001: use existing helper in send-email test
2018-03-22 14:24:13 -07:00
Junio C Hamano
dab684ff43 Merge branch 'sg/test-i18ngrep' into maint
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-22 14:24:12 -07:00
Junio C Hamano
f936c9b393 Merge branch 'jk/daemon-fixes' into maint
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-22 14:24:11 -07:00
Junio C Hamano
b0e0fc267b Merge branch 'tg/split-index-fixes' into maint
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-22 14:24:10 -07:00
Junio C Hamano
721dce003f Merge branch 'jt/http-redact-cookies' into maint
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-03-22 14:24:09 -07:00
Junio C Hamano
14890e916f Merge branch 'sb/submodule-update-reset-fix' into maint
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-27 10:43:54 -08:00
Junio C Hamano
c1ab3b8a44 Merge branch 'ab/commit-m-with-fixup' into maint
"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-27 10:43:54 -08:00
Junio C Hamano
12accdc023 Merge branch 'nd/ita-wt-renames-in-status' into maint
"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-27 10:39:35 -08:00
Jeff King
f74bbc8dd2 revision: drop --show-all option
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-22 12:15:25 -08:00
Genki Sky
7976e901c8 test-lib.sh: unset XDG_CACHE_HOME
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-16 11:23:10 -08:00
Junio C Hamano
2409e1035c Merge branch 'nd/add-i-ignore-submodules' into maint
"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-15 15:18:13 -08:00
Junio C Hamano
984c8337de Merge branch 'tg/stash-with-pathspec-fix' into maint
"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-15 15:18:13 -08:00
Junio C Hamano
1363914a6a Merge branch 'jk/abort-clone-with-existing-dest' into maint
"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-15 15:18:13 -08:00
Junio C Hamano
ff19620f81 Merge branch 'jc/merge-symlink-ours-theirs' into maint
"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-15 15:18:12 -08:00
Junio C Hamano
468dc22e00 Merge branch 'dk/describe-all-output-fix' into maint
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-15 15:18:10 -08:00
Junio C Hamano
af38deeb47 Merge branch 'ab/perf-grep-threads' into maint
More perf tests for threaded grep

* ab/perf-grep-threads:
  perf: amend the grep tests to test grep.threads
2018-02-15 15:18:09 -08:00
Robert P. J. Day
5aea9fe6cc Correct mispellings of ".gitmodule" to ".gitmodules"
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-14 11:34:34 -08:00
Robert P. J. Day
c9a800a66d t/: correct obvious typo "detahced"
Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-14 11:34:25 -08:00
SZEDER Gábor
12e31a6b12 t: document 'test_must_fail ok=<signal-name>'
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-12 11:00:38 -08:00
SZEDER Gábor
63b1a175ee t: make 'test_i18ngrep' more informative on failure
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-08 10:54:27 -08:00
SZEDER Gábor
fd29d7b9d7 t: validate 'test_i18ngrep's parameters
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-08 10:54:27 -08:00
SZEDER Gábor
0f59128f7b t: move 'test_i18ncmp' and 'test_i18ngrep' to 'test-lib-functions.sh'
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-08 10:54:27 -08:00
SZEDER Gábor
93b4b0313c t5536: let 'test_i18ngrep' read the file without redirection
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-08 10:54:27 -08:00
SZEDER Gábor
927c1a643a t5510: consolidate 'grep' and 'test_i18ngrep' patterns
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-08 10:54:27 -08:00
SZEDER Gábor
3b85ec34b8 t4001: don't run 'git status' upstream of a pipe
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-08 10:54:27 -08:00
SZEDER Gábor
cc04adc2d0 t6022: don't run 'git merge' upstream of a pipe
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-08 10:54:27 -08:00
SZEDER Gábor
a4ca4553e0 t5812: add 'test_i18ngrep's missing filename parameter
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-08 10:54:27 -08:00
SZEDER Gábor
8cdef01c42 t5541: add 'test_i18ngrep's missing filename parameter
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-08 10:54:27 -08:00
Jeff King
1cdc62f6f1 t0205: drop redundant test
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-08 10:07:51 -08:00
Stefan Moch
36b78cd9db t7001: add test case for --dry-run
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-02-07 11:43:34 -08:00
Jeff King
ed15e58efe daemon: fix length computation in newline stripping
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-25 13:50:17 -08:00
Jeff King
4414a15002 t/lib-git-daemon: add network-protocol helpers
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-25 13:50:17 -08:00
Jeff King
550fbcad1c daemon: handle NULs in extended attribute string
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-25 13:50:17 -08:00
Jeff King
19136be3f8 daemon: fix off-by-one in logging extended attributes
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-25 13:50:17 -08:00
Jeff King
314a73d658 t/lib-git-daemon: record daemon log
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-25 13:50:03 -08:00
Jeff King
02adf84ab8 t5570: use ls-remote instead of clone for interp tests
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-25 10:44:51 -08:00
Nguyễn Thái Ngọc Duy
ef5b3a6c5e read-cache: don't write index twice if we can't write shared index
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>
2018-01-24 10:09:18 -08:00