Commit Graph

18486 Commits

Author SHA1 Message Date
Ævar Arnfjörð Bjarmason
465028e0e2 merge: add missing strbuf_release()
We strbuf_reset() this "struct strbuf" in a loop earlier, but never
freed it. Plugs a memory leak that's been here ever since this code
got introduced in 1c7b76be7d (Build in merge, 2008-07-07).

This takes us from 68 failed tests in "t7600-merge.sh" to 59 under
SANITIZE=leak, and makes "t7604-merge-custom-message.sh" pass!

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-07 15:40:16 -07:00
Ævar Arnfjörð Bjarmason
272f0a574d ls-files: add missing string_list_clear()
Fix a memory leak that's been here ever since 72aeb18772 (clean.c,
ls-files.c: respect encapsulation of exclude_list_groups, 2013-01-16),
we dup'd the argument in option_parse_exclude(), but never freed the
string_list.

This makes almost all of t3001-ls-files-others-exclude.sh pass (it had
a lot of failures before). Let's mark it as passing with
TEST_PASSES_SANITIZE_LEAK=true, and then exclude the tests that still
failed with a !SANITIZE_LEAK prerequisite check until we fix those
leaks. We can still see the failed tests under
GIT_TEST_FAIL_PREREQS=true.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-07 15:40:15 -07:00
Ævar Arnfjörð Bjarmason
eab4ac6a23 ls-files: fix a trivial dir_clear() leak
Fix an edge case that was missed when the dir_clear() call was added
in eceba53214 (dir: fix problematic API to avoid memory leaks,
2020-08-18), we need to also clean up when we're about to exit with
non-zero.

That commit says, on the topic of the dir_clear() API and UNLEAK():

    [...]two of them clearly thought about leaks since they had an
    UNLEAK(dir) directive, which to me suggests that the method to
    free the data was too unclear.

I think that 0e5bba53af (add UNLEAK annotation for reducing leak
false positives, 2017-09-08) which added the UNLEAK() makes it clear
that that wasn't the case, rather it was the desire to avoid the
complexity of freeing the memory at the end of the program.

This does add a bit of complexity, but I think it's worth it to just
fix these leaks when it's easy in built-ins. It allows them to serve
as canaries for underlying APIs that shouldn't be leaking, it
encourages us to make those freeing APIs nicer for all their users,
and it prevents other leaking regressions by being able to mark the
entire test as TEST_PASSES_SANITIZE_LEAK=true.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-07 15:40:15 -07:00
Ævar Arnfjörð Bjarmason
6ad66ab45e tests: fix test-oid-array leak, test in SANITIZE=leak
Fix a trivial memory leak present ever since 38d905bf58 (sha1-array:
add test-sha1-array and basic tests, 2014-10-01), now that that's
fixed we can test this under GIT_TEST_PASSING_SANITIZE_LEAK=true.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-07 15:40:15 -07:00
Ævar Arnfjörð Bjarmason
926d233035 tests: fix a memory leak in test-oidtree.c
Fix a memory leak in t/helper/test-oidtree.c, we were not freeing the
"struct strbuf" we used for the stdin input we parsed. This leak has
been here ever since 92d8ed8ac1 (oidtree: a crit-bit tree for
odb_loose_cache, 2021-07-07).

Now that it's fixed we can declare that t0069-oidtree.sh will pass
under GIT_TEST_PASSING_SANITIZE_LEAK=true.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-07 15:40:15 -07:00
Ævar Arnfjörð Bjarmason
c0b80e05f7 tests: fix a memory leak in test-parse-options.c
Fix a memory leak in t/helper/test-parse-options.c, we were not
freeing the allocated "struct string_list" or its items. Let's move
the declaration of the "list" variable into the cmd__parse_options()
and release it at the end.

In c8ba163916 (parse-options: add OPT_STRING_LIST helper, 2011-06-09)
the "list" variable was added, and later on in
c8ba163916 (parse-options: add OPT_STRING_LIST helper, 2011-06-09)
the "expect" was added.

The "list" variable was last touched in 2721ce21e4 (use string_list
initializer consistently, 2016-06-13), but it was still left at the
static scope, it's better to move it to the function for consistency.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-07 15:40:15 -07:00
Ævar Arnfjörð Bjarmason
6a75658c0a tests: fix a memory leak in test-prio-queue.c
Fix a memory leak in t/helper/test-prio-queue.c, the lack of freeing
the memory with clear_prio_queue() has been there ever since this code
was originally added in b4b594a315 (prio-queue: priority queue of
pointers to structs, 2013-06-06).

By fixing this leak we can cleanly run t0009-prio-queue.sh under
SANITIZE=leak, so annotate it as such with
TEST_PASSES_SANITIZE_LEAK=true.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-07 15:40:15 -07:00
Junio C Hamano
6cb3deb451 Merge branch 'ab/sanitize-leak-ci' into ab/mark-leak-free-tests-more
* ab/sanitize-leak-ci:
  tests: add a test mode for SANITIZE=leak, run it in CI
  Makefile: add SANITIZE=leak flag to GIT-BUILD-OPTIONS
2021-10-07 15:39:59 -07:00
Junio C Hamano
16119bac40 Merge branch 'lh/systemd-timers'
Testfix.

* lh/systemd-timers:
  maintenance: fix test t7900-maintenance.sh
2021-10-06 13:40:13 -07:00
Junio C Hamano
d8d33378ed Merge branch 'ab/repo-settings-cleanup'
Code cleanup.

* ab/repo-settings-cleanup:
  repository.h: don't use a mix of int and bitfields
  repo-settings.c: simplify the setup
  read-cache & fetch-negotiator: check "enum" values in switch()
  environment.c: remove test-specific "ignore_untracked..." variable
  wrapper.c: add x{un,}setenv(), and use xsetenv() in environment.c
2021-10-06 13:40:11 -07:00
Junio C Hamano
844cc43377 Merge branch 'tb/commit-graph-usage-fix'
Regression in "git commit-graph" command line parsing has been
corrected.

* tb/commit-graph-usage-fix:
  builtin/multi-pack-index.c: disable top-level --[no-]progress
  builtin/commit-graph.c: don't accept common --[no-]progress
2021-10-06 13:40:11 -07:00
Junio C Hamano
7cebe73dbd Merge branch 'pw/rebase-of-a-tag-fix'
"git rebase <upstream> <tag>" failed when aborted in the middle, as
it mistakenly tried to write the tag object instead of peeling it
to HEAD.

* pw/rebase-of-a-tag-fix:
  rebase: dereference tags
  rebase: use lookup_commit_reference_by_name()
  rebase: use our standard error return value
  t3407: rework rebase --quit tests
  t3407: strengthen rebase --abort tests
  t3407: use test_path_is_missing
  t3407: rename a variable
  t3407: use test_cmp_rev
  t3407: use test_commit
  t3407: run tests in $TEST_DIRECTORY
2021-10-06 13:40:11 -07:00
Junio C Hamano
921c795c25 Merge branch 'jt/add-submodule-odb-clean-up'
More code paths that use the hack to add submodule's object
database to the set of alternate object store have been cleaned up.

* jt/add-submodule-odb-clean-up:
  revision: remove "submodule" from opt struct
  repository: support unabsorbed in repo_submodule_init
  submodule: remove unnecessary unabsorbed fallback
2021-10-06 13:40:11 -07:00
Junio C Hamano
3a757d0369 Merge branch 'ah/connect-parse-feature-v0-fix'
Protocol v0 clients can get stuck parsing a malformed feature line.

* ah/connect-parse-feature-v0-fix:
  connect: also update offset for features without values
2021-10-03 21:49:21 -07:00
Junio C Hamano
cbb1ae05d5 Merge branch 'ds/perf-test-built-path-fix'
Perf test fix.

* ds/perf-test-built-path-fix:
  t/perf/run: fix bin-wrappers computation
2021-10-03 21:49:19 -07:00
Junio C Hamano
58e2bc452b Merge branch 'jk/http-redact-fix'
Sensitive data in the HTTP trace were supposed to be redacted, but
we failed to do so in HTTP/2 requests.

* jk/http-redact-fix:
  http: match headers case-insensitively when redacting
2021-10-03 21:49:19 -07:00
Junio C Hamano
6a4f5dadd3 Merge branch 'da/difftool-dir-diff-symlink-fix'
"git difftool --dir-diff" mishandled symbolic links.

* da/difftool-dir-diff-symlink-fix:
  difftool: fix symlink-file writing in dir-diff mode
2021-10-03 21:49:19 -07:00
Junio C Hamano
1030daecda Merge branch 'cb/cvsserver'
"git cvsserver" had a long-standing bug in its authentication code,
which has finally been corrected (it is unclear and is a separate
question if anybody is seriously using it, though).

* cb/cvsserver:
  Documentation: cleanup git-cvsserver
  git-cvsserver: protect against NULL in crypt(3)
  git-cvsserver: use crypt correctly to compare password hashes
2021-10-03 21:49:17 -07:00
Junio C Hamano
ac162a606b Merge branch 'jk/clone-unborn-head-in-bare'
"git clone" from a repository whose HEAD is unborn into a bare
repository didn't follow the branch name the other side used, which
is corrected.

* jk/clone-unborn-head-in-bare:
  clone: handle unborn branch in bare repos
2021-10-03 21:49:17 -07:00
Junio C Hamano
4a6fd7d3c7 Merge branch 'en/stash-df-fix'
"git stash", where the tentative change involves changing a
directory to a file (or vice versa), was confused, which has been
corrected.

* en/stash-df-fix:
  stash: restore untracked files AFTER restoring tracked files
  stash: avoid feeding directories to update-index
  t3903: document a pair of directory/file bugs
2021-10-03 21:49:16 -07:00
Junio C Hamano
bb1677fc29 Merge branch 'jk/reduce-malloc-in-v2-servers'
Code cleanup to limit memory consumption and tighten protocol
message parsing.

* jk/reduce-malloc-in-v2-servers:
  ls-refs: reject unknown arguments
  serve: reject commands used as capabilities
  serve: reject bogus v2 "command=ls-refs=foo"
  docs/protocol-v2: clarify some ls-refs ref-prefix details
  ls-refs: ignore very long ref-prefix counts
  serve: drop "keys" strvec
  serve: provide "receive" function for session-id capability
  serve: provide "receive" function for object-format capability
  serve: add "receive" method for v2 capabilities table
  serve: return capability "value" from get_capability()
  serve: rename is_command() to parse_command()
2021-09-28 13:06:53 -07:00
Lénaïc Huard
670e597399 maintenance: fix test t7900-maintenance.sh
Commit b681b191 introduced the support of systemd timers for git
maintenance.
A test is leveraging the `systemd-analyze verify` utility to verify the
correctness of the systemd unit files generated by git.

But on some systems, although the `systemd-analyze` tool is installed
and supports the `verify` subcommand, it fails with some permission
errors.

So, instead of only checking if the `verify` subcommand exists, a more
reliable way of detecting whether `systemd-analyze verify` can be used
is to try to use it.

The SYSTEMD_ANALYZE prerequisite is now trying to run `systemd-analyze
verify` on a systemd unit file which is shipped by systemd itself.
We can reasonably think that, on systemd hosts, this file is present and
valid.

Signed-off-by: Lénaïc Huard <lenaic@lhuard.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-27 16:06:59 -07:00
Andrzej Hunt
44d2aec6e8 connect: also update offset for features without values
parse_feature_value() takes an offset, and uses it to seek past the
point in features_list that we've already seen. However if the feature
being searched for does not specify a value, the offset is not
updated. Therefore if we call parse_feature_value() in a loop on a
value-less feature, we'll keep on parsing the same feature over and over
again. This usually isn't an issue: there's no point in using
next_server_feature_value() to search for repeated instances of the same
capability unless that capability typically specifies a value - but a
broken server could send a response that omits the value for a feature
even when we are expecting a value.

Therefore we add an offset update calculation for the no-value case,
which helps ensure that loops using next_server_feature_value() will
always terminate.

next_server_feature_value(), and the offset calculation, were first
added in 2.28 in 2c6a403d96 (connect: add function to parse multiple
v1 capability values, 2020-05-25).

Thanks to Peff for authoring the test.

Co-authored-by: Jeff King <peff@peff.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-27 10:34:41 -07:00
Junio C Hamano
50eb005eb3 Merge branch 'cb/plug-leaks-in-alloca-emu-users'
Leakfix.

* cb/plug-leaks-in-alloca-emu-users:
  t0000: avoid masking git exit value through pipes
  tree-diff: fix leak when not HAVE_ALLOCA_H
2021-09-23 13:44:49 -07:00
Junio C Hamano
bd42622e5f Merge branch 'ma/help-w-check-for-requested-page'
The error in "git help no-such-git-command" is handled better.

* ma/help-w-check-for-requested-page:
  help: make sure local html page exists before calling external processes
2021-09-23 13:44:48 -07:00
Junio C Hamano
c2e799012b Merge branch 'cb/unix-sockets-with-windows'
Adjust credential-cache helper to Windows.

* cb/unix-sockets-with-windows:
  git-compat-util: include declaration for unix sockets in windows
  credential-cache: check for windows specific errors
  t0301: fixes for windows compatibility
2021-09-23 13:44:48 -07:00
Junio C Hamano
0e35107e7d Merge branch 'ab/retire-option-argument'
An oddball OPTION_ARGUMENT feature has been removed from the
parse-options API.

* ab/retire-option-argument:
  parse-options API: remove OPTION_ARGUMENT feature
  difftool: use run_command() API in run_file_diff()
  difftool: prepare "diff" cmdline in cmd_difftool()
  difftool: prepare "struct child_process" in cmd_difftool()
2021-09-23 13:44:48 -07:00
Junio C Hamano
0a4cb1f1f2 Merge branch 'mr/bisect-in-c-4'
Rewrite of "git bisect" in C continues.

* mr/bisect-in-c-4:
  bisect--helper: retire `--bisect-next-check` subcommand
  bisect--helper: reimplement `bisect_run` shell function in C
  bisect--helper: reimplement `bisect_visualize()` shell function in C
  run-command: make `exists_in_PATH()` non-static
  t6030-bisect-porcelain: add test for bisect visualize
  t6030-bisect-porcelain: add tests to control bisect run exit cases
2021-09-23 13:44:48 -07:00
Junio C Hamano
57e4a7b633 Merge branch 'ab/unused-script-helpers'
Code clean-up.

* ab/unused-script-helpers:
  test-lib: remove unused $_x40 and $_z40 variables
  git-bisect: remove unused SHA-1 $x40 shell variable
  git-sh-setup: remove unused "pull with rebase" message
  git-submodule: remove unused is_zero_oid() function
2021-09-23 13:44:47 -07:00
Junio C Hamano
cabb41d0f6 Merge branch 'jk/http-server-protocol-versions'
Taking advantage of the CGI interface, http-backend has been
updated to enable protocol v2 automatically when the other side
asks for it.

* jk/http-server-protocol-versions:
  docs/protocol-v2: point readers transport config discussion
  docs/git: discuss server-side config for GIT_PROTOCOL
  docs/http-backend: mention v2 protocol
  http-backend: handle HTTP_GIT_PROTOCOL CGI variable
  t5551: test v2-to-v0 http protocol fallback
2021-09-23 13:44:47 -07:00
Junio C Hamano
ffb0387608 Merge branch 'ab/test-tool-run-command-cleanup'
Code clean-up.

* ab/test-tool-run-command-cleanup:
  test-tool run-command: fix flip-flop init pattern
2021-09-23 13:44:46 -07:00
Junio C Hamano
b83e131029 Merge branch 'en/tests-cleanup-leftover-untracked'
Test clean-up.

* en/tests-cleanup-leftover-untracked:
  tests: remove leftover untracked files
2021-09-23 13:44:46 -07:00
Junio C Hamano
6c84b007c4 Merge branch 'en/am-abort-fix'
When "git am --abort" fails to abort correctly, it still exited
with exit status of 0, which has been corrected.

* en/am-abort-fix:
  am: fix incorrect exit status on am fail to abort
  t4151: add a few am --abort tests
  git-am.txt: clarify --abort behavior
2021-09-23 13:44:45 -07:00
Junio C Hamano
06a0eeaa25 Merge branch 'ps/update-ref-batch-flush'
"git update-ref --stdin" failed to flush its output as needed,
which potentially led the conversation to a deadlock.

* ps/update-ref-batch-flush:
  t1400: avoid SIGPIPE race condition on fifo
  update-ref: fix streaming of status updates
2021-09-23 13:44:45 -07:00
Ævar Arnfjörð Bjarmason
956d2e4639 tests: add a test mode for SANITIZE=leak, run it in CI
While git can be compiled with SANITIZE=leak, we have not run
regression tests under that mode. Memory leaks have only been fixed as
one-offs without structured regression testing.

This change adds CI testing for it. We'll now build and small set of
whitelisted t00*.sh tests under Linux with a new job called
"linux-leaks".

The CI target uses a new GIT_TEST_PASSING_SANITIZE_LEAK=true test
mode. When running in that mode, we'll assert that we were compiled
with SANITIZE=leak. We'll then skip all tests, except those that we've
opted-in by setting "TEST_PASSES_SANITIZE_LEAK=true".

A test setting "TEST_PASSES_SANITIZE_LEAK=true" setting can in turn
make use of the "SANITIZE_LEAK" prerequisite, should they wish to
selectively skip tests even under
"GIT_TEST_PASSING_SANITIZE_LEAK=true". In the preceding commit we
started doing this in "t0004-unwritable.sh" under SANITIZE=leak, now
it'll combine nicely with "GIT_TEST_PASSING_SANITIZE_LEAK=true".

This is how tests that don't set "TEST_PASSES_SANITIZE_LEAK=true" will
be skipped under GIT_TEST_PASSING_SANITIZE_LEAK=true:

    $ GIT_TEST_PASSING_SANITIZE_LEAK=true ./t0001-init.sh
    1..0 # SKIP skip all tests in t0001 under SANITIZE=leak, TEST_PASSES_SANITIZE_LEAK not set

The intent is to add more TEST_PASSES_SANITIZE_LEAK=true annotations
as follow-up change, but let's start small to begin with.

In ci/run-build-and-tests.sh we make use of the default "*" case to
run "make test" without any GIT_TEST_* modes. SANITIZE=leak is known
to fail in combination with GIT_TEST_SPLIT_INDEX=true in
t0016-oidmap.sh, and we're likely to have other such failures in
various GIT_TEST_* modes. Let's focus on getting the base tests
passing, we can expand coverage to GIT_TEST_* modes later.

It would also be possible to implement a more lightweight version of
this by only relying on setting "LSAN_OPTIONS". See
<YS9OT/pn5rRK9cGB@coredump.intra.peff.net>[1] and
<YS9ZIDpANfsh7N+S@coredump.intra.peff.net>[2] for a discussion of
that. I've opted for this approach of adding a GIT_TEST_* mode instead
because it's consistent with how we handle other special test modes.

Being able to add a "!SANITIZE_LEAK" prerequisite and calling
"test_done" early if it isn't satisfied also means that we can more
incrementally add regression tests without being forced to fix
widespread and hard-to-fix leaks at the same time.

We have tests that do simple checking of some tool we're interested
in, but later on in the script might be stressing trace2, or common
sources of leaks like "git log" in combination with the tool (e.g. the
commit-graph tests). To be clear having a prerequisite could also be
accomplished by using "LSAN_OPTIONS" directly.

On the topic of "LSAN_OPTIONS": It would be nice to have a mode to
aggregate all failures in our various scripts, see [2] for a start at
doing that which sets "log_path" in "LSAN_OPTIONS". I've punted on
that for now, it can be added later.

As of writing this we've got major regressions between master..seen,
i.e. the t000*.sh tests and more fixed since 31f9acf9ce (Merge branch
'ah/plugleaks', 2021-08-04) have regressed recently.

See the discussion at <87czsv2idy.fsf@evledraar.gmail.com>[3] about
the lack of this sort of test mode, and 0e5bba53af (add UNLEAK
annotation for reducing leak false positives, 2017-09-08) for the
initial addition of SANITIZE=leak.

See also 09595ab381 (Merge branch 'jk/leak-checkers', 2017-09-19),
7782066f67 (Merge branch 'jk/apache-lsan', 2019-05-19) and the recent
936e58851a (Merge branch 'ah/plugleaks', 2021-05-07) for some of the
past history of "one-off" SANITIZE=leak (and more) fixes.

As noted in [5] we can't support this on OSX yet until Clang 14 is
released, at that point we'll probably want to resurrect that
"osx-leaks" job.

1. https://github.com/google/sanitizers/wiki/AddressSanitizerLeakSanitizer
2. https://lore.kernel.org/git/YS9OT%2Fpn5rRK9cGB@coredump.intra.peff.net/
3. https://lore.kernel.org/git/87czsv2idy.fsf@evledraar.gmail.com/
4. https://lore.kernel.org/git/YS9ZIDpANfsh7N+S@coredump.intra.peff.net/
5. https://lore.kernel.org/git/20210916035603.76369-1-carenas@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-23 11:29:45 -07:00
Ævar Arnfjörð Bjarmason
2cdc292b31 Makefile: add SANITIZE=leak flag to GIT-BUILD-OPTIONS
When SANITIZE=leak is specified we'll now add a SANITIZE_LEAK flag to
GIT-BUILD-OPTIONS, this can then be picked up by the test-lib.sh,
which sets a SANITIZE_LEAK prerequisite.

We can then skip specific tests that are known to fail under
SANITIZE=leak, add one such annotation to t0004-unwritable.sh, which
now passes under SANITIZE=leak.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-23 11:29:45 -07:00
David Aguilar
5bafb3576a difftool: fix symlink-file writing in dir-diff mode
The difftool dir-diff mode handles symlinks by replacing them with their
readlink(2) values. This allows diff tools to see changes to symlinks
as if they were regular text diffs with the old and new path values.
This is analogous to what "git diff" displays when symlinks change.

The temporary diff directories that are created initially contain
symlinks because they get checked-out using a temporary index that
retains the original symlinks as checked-in to the repository.

A bug was introduced when difftool was rewritten in C that made
difftool write the readlink(2) contents into the pointed-to file rather
than the symlink itself. The write was going through the symlink and
writing to its target rather than writing to the symlink path itself.

Replace symlinks with raw text files by unlinking the symlink path
before writing the readlink(2) content into them.

When 18ec800512 (difftool: handle modified symlinks in dir-diff mode,
2017-03-15) added handling for modified symlinks this bug got recorded
in the test suite. The tests included the pointed-to symlink target
paths. These paths were being reported because difftool was erroneously
writing to them, but they should have never been reported nor written.

Correct the modified-symlinks test cases by removing the target files
from the expected output.

Add a test to ensure that symlinks are written with the readlink(2)
values and that the target files contain their original content.

Reported-by: Alan Blotz <work@blotz.org>
Helped-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-23 11:24:41 -07:00
Jeff King
b66c77a64e http: match headers case-insensitively when redacting
When HTTP/2 is in use, we fail to correctly redact "Authorization" (and
other) headers in our GIT_TRACE_CURL output.

We get the headers in our CURLOPT_DEBUGFUNCTION callback, curl_trace().
It passes them along to curl_dump_header(), which in turn checks
redact_sensitive_header(). We see the headers as a text buffer like:

  Host: ...
  Authorization: Basic ...

After breaking it into lines, we match each header using skip_prefix().
This is case-sensitive, even though HTTP headers are case-insensitive.
This has worked reliably in the past because these headers are generated
by curl itself, which is predictable in what it sends.

But when HTTP/2 is in use, instead we get a lower-case "authorization:"
header, and we fail to match it. The fix is simple: we should match with
skip_iprefix().

Testing is more complicated, though. We do have a test for the redacting
feature, but we don't hit the problem case because our test Apache setup
does not understand HTTP/2. You can reproduce the issue by applying this
on top of the test change in this patch:

	diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
	index afa91e38b0..19267c7107 100644
	--- a/t/lib-httpd/apache.conf
	+++ b/t/lib-httpd/apache.conf
	@@ -29,6 +29,9 @@ ErrorLog error.log
	 	LoadModule setenvif_module modules/mod_setenvif.so
	 </IfModule>

	+LoadModule http2_module modules/mod_http2.so
	+Protocols h2c
	+
	 <IfVersion < 2.4>
	 LockFile accept.lock
	 </IfVersion>
	@@ -64,8 +67,8 @@ LockFile accept.lock
	 <IfModule !mod_access_compat.c>
	 	LoadModule access_compat_module modules/mod_access_compat.so
	 </IfModule>
	-<IfModule !mod_mpm_prefork.c>
	-	LoadModule mpm_prefork_module modules/mod_mpm_prefork.so
	+<IfModule !mod_mpm_event.c>
	+	LoadModule mpm_event_module modules/mod_mpm_event.so
	 </IfModule>
	 <IfModule !mod_unixd.c>
	 	LoadModule unixd_module modules/mod_unixd.so
	diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
	index 1c2a444ae7..ff74f0ae8a 100755
	--- a/t/t5551-http-fetch-smart.sh
	+++ b/t/t5551-http-fetch-smart.sh
	@@ -24,6 +24,10 @@ test_expect_success 'create http-accessible bare repository' '
	 	git push public main:main
	 '

	+test_expect_success 'prefer http/2' '
	+	git config --global http.version HTTP/2
	+'
	+
	 setup_askpass_helper

	 test_expect_success 'clone http repository' '

but this has a few issues:

  - it's not necessarily portable. The http2 apache module might not be
    available on all systems. Further, the http2 module isn't compatible
    with the prefork mpm, so we have to switch to something else. But we
    don't necessarily know what's available. It would be nice if we
    could have conditional config, but IfModule only tells us if a
    module is already loaded, not whether it is available at all.

    This might be a non-issue. The http tests are already optional, and
    modern-enough systems may just have both of these. But...

  - if we do this, then we'd no longer be testing HTTP/1.1 at all. I'm
    not sure how much that matters since it's all handled by curl under
    the hood, but I'd worry that some detail leaks through. We'd
    probably want two scripts running similar tests, one with HTTP/2 and
    one with HTTP/1.1.

  - speaking of which, a later test fails with the patch above! The
    problem is that it is making sure we used a chunked
    transfer-encoding by looking for that header in the trace. But
    HTTP/2 doesn't support that, as it has its own streaming mechanisms
    (the overall operation works fine; we just don't see the header in
    the trace).

Furthermore, even with the changes above, this test still does not
detect the current failure, because we see _both_ HTTP/1.1 and HTTP/2
requests, which confuse it. Quoting only the interesting bits from the
resulting trace file, we first see:

  => Send header: GET /auth/smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1
  => Send header: Connection: Upgrade, HTTP2-Settings
  => Send header: Upgrade: h2c
  => Send header: HTTP2-Settings: AAMAAABkAAQCAAAAAAIAAAAA

  <= Recv header: HTTP/1.1 401 Unauthorized
  <= Recv header: Date: Wed, 22 Sep 2021 20:03:32 GMT
  <= Recv header: Server: Apache/2.4.49 (Debian)
  <= Recv header: WWW-Authenticate: Basic realm="git-auth"

So the client asks for HTTP/2, but Apache does not do the upgrade for
the 401 response. Then the client repeats with credentials:

  => Send header: GET /auth/smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1
  => Send header: Authorization: Basic <redacted>
  => Send header: Connection: Upgrade, HTTP2-Settings
  => Send header: Upgrade: h2c
  => Send header: HTTP2-Settings: AAMAAABkAAQCAAAAAAIAAAAA

  <= Recv header: HTTP/1.1 101 Switching Protocols
  <= Recv header: Upgrade: h2c
  <= Recv header: Connection: Upgrade
  <= Recv header: HTTP/2 200
  <= Recv header: content-type: application/x-git-upload-pack-advertisement

So the client does properly redact there, because we're speaking
HTTP/1.1, and the server indicates it can do the upgrade. And then the
client will make further requests using HTTP/2:

  => Send header: POST /auth/smart/repo.git/git-upload-pack HTTP/2
  => Send header: authorization: Basic dXNlckBob3N0OnBhc3NAaG9zdA==
  => Send header: content-type: application/x-git-upload-pack-request

And there we can see that the credential is _not_ redacted. This part of
the test is what gets confused:

	# Ensure that there is no "Basic" followed by a base64 string, but that
	# the auth details are redacted
	! grep "Authorization: Basic [0-9a-zA-Z+/]" trace &&
	grep "Authorization: Basic <redacted>" trace

The first grep does not match the un-redacted HTTP/2 header, because
it insists on an uppercase "A". And the second one does find the
HTTP/1.1 header. So as far as the test is concerned, everything is OK,
but it failed to notice the un-redacted lines.

We can make this test (and the other related ones) more robust by adding
"-i" to grep case-insensitively. This isn't really doing anything for
now, since we're not actually speaking HTTP/2, but it future-proofs the
tests for a day when we do (either we add explicit HTTP/2 test support,
or it's eventually enabled by default by our Apache+curl test setup).
And it doesn't hurt in the meantime for the tests to be more careful.

The change to use "grep -i", coupled with the changes to use HTTP/2
shown above, causes the test to fail with the current code, and pass
after this patch is applied.

And finally, there's one other way to demonstrate the issue (and how I
actually found it originally). Looking at GIT_TRACE_CURL output against
github.com, you'll see the unredacted output, even if you didn't set
http.version. That's because setting it is only necessary for curl to
send the extra headers in its HTTP/1.1 request that say "Hey, I speak
HTTP/2; upgrade if you do, too". But for a production site speaking
https, the server advertises via ALPN, a TLS extension, that it supports
HTTP/2, and the client can immediately start using it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-22 21:24:58 -07:00
Derrick Stolee
f9d65b04cd t/perf/run: fix bin-wrappers computation
The GIT_TEST_INSTALLED was moved from perf-lib.sh to run in df0f5021
(perf-lib.sh: remove GIT_TEST_INSTALLED from perf-lib.sh, 2019-05-07)
and that included a change to how it inspected the existence of a
bin-wrappers directory. However, that included a typo that made the
match of bin-wrappers never work. Specifically, the assignment was

	mydir_abs_wrappers="$mydir_abs_wrappers/bin-wrappers"

which uses the same variable before it is initialized. By changing it to

	mydir_abs_wrappers="$mydir_abs/bin-wrappers"

We can correctly use the bin-wrappers directory.

This is critical to successfully computing performance of commands that
execute subcommands. The bin-wrappers ensure that the --exec-path is set
correctly.

Reported-by: Victoria Dye <vdye@github.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-22 13:26:11 -07:00
Ævar Arnfjörð Bjarmason
c6b4888b3f environment.c: remove test-specific "ignore_untracked..." variable
Instead of the global ignore_untracked_cache_config variable added in
dae6c322fa (test-dump-untracked-cache: don't modify the untracked
cache, 2016-01-27) we can make use of the new facility to set config
via environment variables added in d8d77153ea (config: allow
specifying config entries via envvar pairs, 2021-01-12).

It's arguably a bit hacky to use setenv() and getenv() to pass
messages between the same program, but since the test helpers are not
the main intended audience of repo-settings.c I think it's better than
hardcoding the test-only special-case in prepare_repo_settings().

This uses the xsetenv() wrapper added in the preceding commit, if we
don't set these in the environment we'll fail in
t7063-status-untracked-cache.sh, but let's fail earlier anyway if that
were to happen.

This breaks any parent process that's potentially using the
GIT_CONFIG_* and GIT_CONFIG_PARAMETERS mechanism to pass one-shot
config setting down to a git subprocess, but in this case we don't
care about the general case of such potential parents. This process
neither spawns other "git" processes, nor is it interested in other
configuration. We might want to pick up other test modes here, but
those will be passed via GIT_TEST_* environment variables.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-22 13:15:00 -07:00
Phillip Wood
7740ac691d rebase: dereference tags
A rebase started with 'git rebase <A> <B>' is conceptually to first
checkout <B> and run 'git rebase <A>' starting from that state.  'git
rebase --abort' in the middle of such a rebase should take us back to
the state we checked out <B>.

This used to work, even when <B> is a tag that points at a commit,
until Git 2.20.0 when the command was reimplemented in C.  The command
now complains that the tag object itself cannot be checked out, which
may be technically correct but is not what the user asked to do.

Fix this old regression by using lookup_commit_reference_by_name()
when parsing <B>. The scripted version did not need to peel the tag
because the commands it passed the tag to (e.g 'git reset') peeled the
tag themselves.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-22 12:04:52 -07:00
Taylor Blau
0394f8d002 builtin/multi-pack-index.c: disable top-level --[no-]progress
In a similar spirit as the previous patch, let sub-commands which
support showing or hiding a progress meter handle parsing the
`--progress` or `--no-progress` option, but do not expose it as an
option to the top-level `multi-pack-index` builtin.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-22 09:26:29 -07:00
Junio C Hamano
71e36361bf Merge branch 'jk/t5562-racefix'
Test update.

* jk/t5562-racefix:
  t5562: use alarm() to interrupt timed child-wait
2021-09-20 15:20:46 -07:00
Junio C Hamano
a16dd13740 Merge branch 'ds/mergies-with-sparse-index'
Various mergy operations have been prepared to work efficiently
with the sparse index.

* ds/mergies-with-sparse-index:
  sparse-index: integrate with cherry-pick and rebase
  sequencer: ensure full index if not ORT strategy
  t1092: add cherry-pick, rebase tests
  merge-ort: expand only for out-of-cone conflicts
  merge: make sparse-aware with ORT
  diff: ignore sparse paths in diffstat
2021-09-20 15:20:45 -07:00
Junio C Hamano
dc89c34d9e Merge branch 'ds/sparse-index-ignored-files'
In cone mode, the sparse-index code path learned to remove ignored
files (like build artifacts) outside the sparse cone, allowing the
entire directory outside the sparse cone to be removed, which is
especially useful when the sparse patterns change.

* ds/sparse-index-ignored-files:
  sparse-checkout: clear tracked sparse dirs
  sparse-index: add SPARSE_INDEX_MEMORY_ONLY flag
  attr: be careful about sparse directories
  sparse-checkout: create helper methods
  sparse-index: use WRITE_TREE_MISSING_OK
  sparse-index: silently return when cache tree fails
  unpack-trees: fix nested sparse-dir search
  sparse-index: silently return when not using cone-mode patterns
  t7519: rewrite sparse index test
2021-09-20 15:20:44 -07:00
Junio C Hamano
5331af2352 Merge branch 'ab/serve-cleanup'
Code clean-up around "git serve".

* ab/serve-cleanup:
  upload-pack: document and rename --advertise-refs
  serve.[ch]: remove "serve_options", split up --advertise-refs code
  {upload,receive}-pack tests: add --advertise-refs tests
  serve.c: move version line to advertise_capabilities()
  serve: move transfer.advertiseSID check into session_id_advertise()
  serve.[ch]: don't pass "struct strvec *keys" to commands
  serve: use designated initializers
  transport: use designated initializers
  transport: rename "fetch" in transport_vtable to "fetch_refs"
  serve: mark has_capability() as static
2021-09-20 15:20:43 -07:00
Junio C Hamano
75405e7270 Merge branch 'dt/submodule-diff-fixes'
"git diff --submodule=diff" showed failure from run_command() when
trying to run diff inside a submodule, when the user manually
removes the submodule directory.

* dt/submodule-diff-fixes:
  diff --submodule=diff: don't print failure message twice
  diff --submodule=diff: do not fail on ever-initialied deleted submodules
  t4060: remove unused variable
2021-09-20 15:20:41 -07:00
Junio C Hamano
ed8794ef7a Merge branch 'lh/systemd-timers'
"git maintenance" scheduler learned to use systemd timers as a
possible backend.

* lh/systemd-timers:
  maintenance: add support for systemd timers on Linux
  maintenance: `git maintenance run` learned `--scheduler=<scheduler>`
  cache.h: Introduce a generic "xdg_config_home_for(…)" function
2021-09-20 15:20:40 -07:00
Junio C Hamano
11e5d0a262 Merge branch 'jt/grep-wo-submodule-odb-as-alternate'
The code to make "git grep" recurse into submodules has been
updated to migrate away from the "add submodule's object store as
an alternate object store" mechanism (which is suboptimal).

* jt/grep-wo-submodule-odb-as-alternate:
  t7814: show lack of alternate ODB-adding
  submodule-config: pass repo upon blob config read
  grep: add repository to OID grep sources
  grep: allocate subrepos on heap
  grep: read submodule entry with explicit repo
  grep: typesafe versions of grep_source_init
  grep: use submodule-ODB-as-alternate lazy-addition
  submodule: lazily add submodule ODBs as alternates
2021-09-20 15:20:39 -07:00
Junio C Hamano
0649303820 Merge branch 'tb/multi-pack-bitmaps'
The reachability bitmap file used to be generated only for a single
pack, but now we've learned to generate bitmaps for history that
span across multiple packfiles.

* tb/multi-pack-bitmaps: (29 commits)
  pack-bitmap: drop bitmap_index argument from try_partial_reuse()
  pack-bitmap: drop repository argument from prepare_midx_bitmap_git()
  p5326: perf tests for MIDX bitmaps
  p5310: extract full and partial bitmap tests
  midx: respect 'GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP'
  t7700: update to work with MIDX bitmap test knob
  t5319: don't write MIDX bitmaps in t5319
  t5310: disable GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP
  t0410: disable GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP
  t5326: test multi-pack bitmap behavior
  t/helper/test-read-midx.c: add --checksum mode
  t5310: move some tests to lib-bitmap.sh
  pack-bitmap: write multi-pack bitmaps
  pack-bitmap: read multi-pack bitmaps
  pack-bitmap.c: avoid redundant calls to try_partial_reuse
  pack-bitmap.c: introduce 'bitmap_is_preferred_refname()'
  pack-bitmap.c: introduce 'nth_bitmap_object_oid()'
  pack-bitmap.c: introduce 'bitmap_num_objects()'
  midx: avoid opening multiple MIDXs when writing
  midx: close linked MIDXs, avoid leaking memory
  ...
2021-09-20 15:20:39 -07:00