sort by:
Revision Author Date Message Commit Date
df93e40 init: refactor the template directory discovery into its own function We will need to call this function from `hook.c` to be able to prevent hooks from running that were written as part of a `clone` but did not originate from the template directory. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2024, 20:30:10 UTC
48c171d find_hook(): refactor the `STRIP_EXTENSION` logic When looking for a hook and not finding one, and when `STRIP_EXTENSION` is available (read: if we're on Windows and `.exe` is the required extension for executable programs), we want to look also for a hook with that extension. Previously, we added that handling into the conditional block that was meant to handle when no hook was found (possibly providing some advice for the user's benefit). If the hook with that file extension was found, we'd return early from that function instead of writing out said advice, of course. However, we're about to introduce a safety valve to prevent hooks from being run during a clone, to reduce the attack surface of bugs that allow writing files to be written into arbitrary locations. To prepare for that, refactor the logic to avoid the early return, by separating the `STRIP_EXTENSION` handling from the conditional block handling the case when no hook was found. This commit is best viewed with `--patience`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2024, 20:30:09 UTC
31572dc clone: when symbolic links collide with directories, keep the latter When recursively cloning a repository with submodules, we must ensure that the submodules paths do not suddenly contain symbolic links that would let Git write into unintended locations. We just plugged that vulnerability, but let's add some more defense-in-depth. Since we can only keep one item on disk if multiple index entries' paths collide, we may just as well avoid keeping a symbolic link (because that would allow attack vectors where Git follows those links by mistake). Technically, we handle more situations than cloning submodules into paths that were (partially) replaced by symbolic links. This provides defense-in-depth in case someone finds a case-folding confusion vulnerability in the future that does not even involve submodules. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2024, 20:30:08 UTC
850c3a2 entry: report more colliding paths In b878579ae7 (clone: report duplicate entries on case-insensitive filesystems, 2018-08-17) code was added to warn about index entries that resolve to the same file system entity (usually the cause is a case-insensitive filesystem). In Git for Windows, where inodes are not trusted (because of a performance trade-off, inodes are equal to 0 by default), that check does not compare inode numbers but the verbatim path. This logic works well when index entries' paths differ only in case. However, for file/directory conflicts only the file's path was reported, leaving the user puzzled with what that path collides. Let's try ot catch colliding paths even if one path is the prefix of the other. We do this also in setups where the file system is case-sensitive because the inode check would not be able to catch those collisions. While not a complete solution (for example, on macOS, Unicode normalization could also lead to file/directory conflicts but be missed by this logic), it is at least another defensive layer on top of what the previous commits added. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2024, 20:30:07 UTC
e4930e8 t5510: verify that D/F confusion cannot lead to an RCE The most critical vulnerabilities in Git lead to a Remote Code Execution ("RCE"), i.e. the ability for an attacker to have malicious code being run as part of a Git operation that is not expected to run said code, such has hooks delivered as part of a `git clone`. A couple of parent commits ago, a bug was fixed that let Git be confused by the presence of a path `a-` to mistakenly assume that a directory `a/` can safely be created without removing an existing `a` that is a symbolic link. This bug did not represent an exploitable vulnerability on its own; Let's make sure it stays that way. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2024, 20:30:06 UTC
e8d0608 submodule: require the submodule path to contain directories only Submodules are stored in subdirectories of their superproject. When these subdirectories have been replaced with symlinks by a malicious actor, all kinds of mayhem can be caused. This _should_ not be possible, but many CVEs in the past showed that _when_ possible, it allows attackers to slip in code that gets executed during, say, a `git clone --recursive` operation. Let's add some defense-in-depth to disallow submodule paths to have anything except directories in them. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2024, 20:30:04 UTC
eafffd9 clone_submodule: avoid using `access()` on directories In 0060fd1511b (clone --recurse-submodules: prevent name squatting on Windows, 2019-09-12), I introduced code to verify that a git dir either does not exist, or is at least empty, to fend off attacks where an inadvertently (and likely maliciously) pre-populated git dir would be used while cloning submodules recursively. The logic used `access(<path>, X_OK)` to verify that a directory exists before calling `is_empty_dir()` on it. That is a curious way to check for a directory's existence and might well fail for unwanted reasons. Even the original author (it was I ;-) ) struggles to explain why this function was used rather than `stat()`. This code was _almost_ copypastad in the previous commit, but that `access()` call was caught during review. Let's use `stat()` instead also in the code that was almost copied verbatim. Let's not use `lstat()` because in the unlikely event that somebody snuck a symbolic link in, pointing to a crafted directory, we want to verify that that directory is empty. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2024, 20:30:03 UTC
9706576 submodules: submodule paths must not contain symlinks When creating a submodule path, we must be careful not to follow symbolic links. Otherwise we may follow a symbolic link pointing to a gitdir (which are valid symbolic links!) e.g. while cloning. On case-insensitive filesystems, however, we blindly replace a directory that has been created as part of the `clone` operation with a symlink when the path to the latter differs only in case from the former's path. Let's simply avoid this situation by expecting not ever having to overwrite any existing file/directory/symlink upon cloning. That way, we won't even replace a directory that we just created. This addresses CVE-2024-32002. Reported-by: Filip Hejsek <filip.hejsek@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2024, 20:30:02 UTC
9cf8547 clone: prevent clashing git dirs when cloning submodule in parallel While it is expected to have several git dirs within the `.git/modules/` tree, it is important that they do not interfere with each other. For example, if one submodule was called "captain" and another submodule "captain/hooks", their respective git dirs would clash, as they would be located in `.git/modules/captain/` and `.git/modules/captain/hooks/`, respectively, i.e. the latter's files could clash with the actual Git hooks of the former. To prevent these clashes, and in particular to prevent hooks from being written and then executed as part of a recursive clone, we introduced checks as part of the fix for CVE-2019-1387 in a8dee3ca61 (Disallow dubiously-nested submodule git directories, 2019-10-01). It is currently possible to bypass the check for clashing submodule git dirs in two ways: 1. parallel cloning 2. checkout --recurse-submodules Let's check not only before, but also after parallel cloning (and before checking out the submodule), that the git dir is not clashing with another one, otherwise fail. This addresses the parallel cloning issue. As to the parallel checkout issue: It requires quite a few manual steps to create clashing git dirs because Git itself would refuse to initialize the inner one, as demonstrated by the test case. Nevertheless, let's teach the recursive checkout (namely, the `submodule_move_head()` function that is used by the recursive checkout) to be careful to verify that it does not use a clashing git dir, and if it does, disable it (by deleting the `HEAD` file so that subsequent Git calls won't recognize it as a git dir anymore). Note: The parallel cloning test case contains a `cat err` that proved to be highly useful when analyzing the racy nature of the operation (the operation can fail with three different error messages, depending on timing), and was left on purpose to ease future debugging should the need arise. Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2024, 20:30:01 UTC
b20c10f t7423: add tests for symlinked submodule directories Submodule operations must not follow symlinks in working tree, because otherwise files might be written to unintended places, leading to vulnerabilities. Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2024, 20:30:00 UTC
c30a574 has_dir_name(): do not get confused by characters < '/' There is a bug in directory/file ("D/F") conflict checking optimization: It assumes that such a conflict cannot happen if a newly added entry's path is lexicgraphically "greater than" the last already-existing index entry _and_ contains a directory separator that comes strictly after the common prefix (`len > len_eq_offset`). This assumption is incorrect, though: `a-` sorts _between_ `a` and `a/b`, their common prefix is `a`, the slash comes after the common prefix, and there is still a file/directory conflict. Let's re-design this logic, taking these facts into consideration: - It is impossible for a file to sort after another file with whose directory it conflicts because the trailing NUL byte is always smaller than any other character. - Since there are quite a number of ASCII characters that sort before the slash (e.g. `-`, `.`, the space character), looking at the last already-existing index entry is not enough to determine whether there is a D/F conflict when the first character different from the existing last index entry's path is a slash. If it is not a slash, there cannot be a file/directory conflict. And if the existing index entry's first different character is a slash, it also cannot be a file/directory conflict because the optimization requires the newly-added entry's path to sort _after_ the existing entry's, and the conflicting file's path would not. So let's fall back to the regular binary search whenever the newly-added item's path differs in a slash character. If it does not, and it sorts after the last index entry, there is no D/F conflict and the new index entry can be safely appended. This fix also nicely simplifies the logic and makes it much easier to reason about, while the impact on performance should be negligible: After this fix, the optimization will be skipped only when index entry's paths differ in a slash and a space, `!`, `"`, `#`, `$`, `%`, `&`, `'`, | ( `)`, `*`, `+`, `,`, `-`, or `.`, which should be a rare situation. Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2024, 20:29:58 UTC
e69ac42 docs: document security issues around untrusted .git dirs For a long time our general philosophy has been that it's unsafe to run arbitrary Git commands if you don't trust the hooks or config in .git, but that running upload-pack should be OK. E.g., see 1456b043fc (Remove post-upload-hook, 2009-12-10), or the design of uploadpack.packObjectsHook. But we never really documented this (and even the discussions that led to 1456b043fc were not on the public list!). Let's try to make our approach more clear, but also be realistic that even upload-pack carries some risk. Helped-by: Filip Hejsek <filip.hejsek@gmail.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2024, 20:29:57 UTC
7b70e9e upload-pack: disable lazy-fetching by default The upload-pack command tries to avoid trusting the repository in which it's run (e.g., by not running any hooks and not using any config that contains arbitrary commands). But if the server side of a fetch or a clone is a partial clone, then either upload-pack or its child pack-objects may run a lazy "git fetch" under the hood. And it is very easy to convince fetch to run arbitrary commands. The "server" side can be a local repository owned by someone else, who would be able to configure commands that are run during a clone with the current user's permissions. This issue has been designated CVE-2024-32004. The fix in this commit's parent helps in this scenario, as well as in related scenarios using SSH to clone, where the untrusted .git directory is owned by a different user id. But if you received one as a zip file, on a USB stick, etc, it may be owned by your user but still untrusted. This has been designated CVE-2024-32465. To mitigate the issue more completely, let's disable lazy fetching entirely during `upload-pack`. While fetching from a partial repository should be relatively rare, it is certainly not an unreasonable workflow. And thus we need to provide an escape hatch. This commit works by respecting a GIT_NO_LAZY_FETCH environment variable (to skip the lazy-fetch), and setting it in upload-pack, but only when the user has not already done so (which gives us the escape hatch). The name of the variable is specifically chosen to match what has already been added in 'master' via e6d5479e7a (git: extend --no-lazy-fetch to work across subprocesses, 2024-02-27). Since we're building this fix as a backport for older versions, we could cherry-pick that patch and its earlier steps. However, we don't really need the niceties (like a "--no-lazy-fetch" option) that it offers. By using the same name, everything should just work when the two are eventually merged, but here are a few notes: - the blocking of the fetch in e6d5479e7a is incomplete! It sets fetch_if_missing to 0 when we setup the repository variable, but that isn't enough. pack-objects in particular will call prefetch_to_pack() even if that variable is 0. This patch by contrast checks the environment variable at the lowest level before we call the lazy fetch, where we can be sure to catch all code paths. Possibly the setting of fetch_if_missing from e6d5479e7a can be reverted, but it may be useful to have. For example, some code may want to use that flag to change behavior before it gets to the point of trying to start the fetch. At any rate, that's all outside the scope of this patch. - there's documentation for GIT_NO_LAZY_FETCH in e6d5479e7a. We can live without that here, because for the most part the user shouldn't need to set it themselves. The exception is if they do want to override upload-pack's default, and that requires a separate documentation section (which is added here) - it would be nice to use the NO_LAZY_FETCH_ENVIRONMENT macro added by e6d5479e7a, but those definitions have moved from cache.h to environment.h between 2.39.3 and master. I just used the raw string literals, and we can replace them with the macro once this topic is merged to master. At least with respect to CVE-2024-32004, this does render this commit's parent commit somewhat redundant. However, it is worth retaining that commit as defense in depth, and because it may help other issues (e.g., symlink/hardlink TOCTOU races, where zip files are not really an interesting attack vector). The tests in t0411 still pass, but now we have _two_ mechanisms ensuring that the evil command is not run. Let's beef up the existing ones to check that they failed for the expected reason, that we refused to run upload-pack at all with an alternate user id. And add two new ones for the same-user case that both the restriction and its escape hatch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2024, 20:29:56 UTC
f4aa8c8 fetch/clone: detect dubious ownership of local repositories When cloning from somebody else's repositories, it is possible that, say, the `upload-pack` command is overridden in the repository that is about to be cloned, which would then be run in the user's context who started the clone. To remind the user that this is a potentially unsafe operation, let's extend the ownership checks we have already established for regular gitdir discovery to extend also to local repositories that are about to be cloned. This protection extends also to file:// URLs. The fixes in this commit address CVE-2024-32004. Note: This commit does not touch the `fetch`/`clone` code directly, but instead the function used implicitly by both: `enter_repo()`. This function is also used by `git receive-pack` (i.e. pushes), by `git upload-archive`, by `git daemon` and by `git http-backend`. In setups that want to serve repositories owned by different users than the account running the service, this will require `safe.*` settings to be configured accordingly. Also note: there are tiny time windows where a time-of-check-time-of-use ("TOCTOU") race is possible. The real solution to those would be to work with `fstat()` and `openat()`. However, the latter function is not available on Windows (and would have to be emulated with rather expensive low-level `NtCreateFile()` calls), and the changes would be quite extensive, for my taste too extensive for the little gain given that embargoed releases need to pay extra attention to avoid introducing inadvertent bugs. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2024, 20:29:54 UTC
5c5a4a1 t0411: add tests for cloning from partial repo Cloning from a partial repository must not fetch missing objects into the partial repository, because that can lead to arbitrary code execution. Add a couple of test cases, pretending to the `upload-pack` command (and to that command only) that it is working on a repository owned by someone else. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2024, 20:29:53 UTC
9e06401 Merge branch 'js/github-actions-update' Update remaining GitHub Actions jobs to avoid warnings against using deprecated version of Node.js. * js/github-actions-update: ci(linux32): add a note about Actions that must not be updated ci: bump remaining outdated Actions versions With this backport, `maint-2.39`'s CI builds are finally healthy again. Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> 16 April 2024, 22:01:11 UTC
64f35ba Merge branch 'jc/maint-github-actions-update' * jc/maint-github-actions-update: GitHub Actions: update to github-script@v7 GitHub Actions: update to checkout@v4 Yet another thing to help `maint-2.39`'s CI builds to become healthy again. Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> 16 April 2024, 22:01:00 UTC
213958f ci(linux32): add a note about Actions that must not be updated The Docker container used by the `linux32` job comes without Node.js, and therefore the `actions/checkout` and `actions/upload-artifact` Actions cannot be upgraded to the latest versions (because they use Node.js). One time too many, I accidentally tried to update them, where `actions/checkout` at least fails immediately, but the `actions/upload-artifact` step is only used when any test fails, and therefore the CI run usually passes even though that Action was updated to a version that is incompatible with the Docker container in which this job runs. So let's add a big fat warning, mainly for my own benefit, to avoid running into the very same issue over and over again. Backported-from: 20e0ff8835 (ci(linux32): add a note about Actions that must not be updated, 2024-02-11) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 16 April 2024, 22:00:58 UTC
7e1bcc8 ci: bump remaining outdated Actions versions After activating automatic Dependabot updates in the git-for-windows/git repository, Dependabot noticed a couple of yet-unaddressed updates. They avoid "Node.js 16 Actions" deprecation messages by bumping the following Actions' versions: - actions/upload-artifact from 3 to 4 - actions/download-artifact from 3 to 4 - actions/cache from 3 to 4 Backported-from: 820a340085 (ci: bump remaining outdated Actions versions, 2024-02-11) Helped-by: Matthias Aßhauer <mha1993@live.de> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 16 April 2024, 22:00:57 UTC
f6bed64 GitHub Actions: update to github-script@v7 We seem to be getting "Node.js 16 actions are deprecated." warnings for jobs that use github-script@v6. Update to github-script@v7, which is said to use Node.js 20. Backported-from: c4ddbe043e (GitHub Actions: update to github-script@v7, 2024-02-02) Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 16 April 2024, 22:00:57 UTC
ce47f7c GitHub Actions: update to checkout@v4 We seem to be getting "Node.js 16 actions are deprecated." warnings for jobs that use checkout@v3. Except for the i686 containers job that is kept at checkout@v1 [*], update to checkout@v4, which is said to use Node.js 20. [*] 6cf4d908 (ci(main): upgrade actions/checkout to v3, 2022-12-05) refers to https://github.com/actions/runner/issues/2115 and explains why container jobs are kept at checkout@v1. We may want to check the current status of the issue and move it to the same version as other jobs, but that is outside the scope of this step. Backported-from: e94dec0c1d (GitHub Actions: update to checkout@v4, 2024-02-02) Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 16 April 2024, 22:00:57 UTC
6133e3a Merge branch 'quicker-asan-lsan' This patch speeds up the `asan`/`lsan` jobs that are really slow enough already. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 16 April 2024, 22:00:56 UTC
ea094ee Merge branch 'jk/test-lsan-denoise-output' Tests with LSan from time to time seem to emit harmless message that makes our tests unnecessarily flakey; we work it around by filtering the uninteresting output. * jk/test-lsan-denoise-output: test-lib: ignore uninteresting LSan output Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 16 April 2024, 22:00:54 UTC
b12dcab Merge branch 'js/ci-use-macos-13' Replace macos-12 used at GitHub CI with macos-13. * js/ci-use-macos-13: ci: upgrade to using macos-13 This is another backport to `maint-2.39` to allow less CI jobs to break. Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> 16 April 2024, 21:59:03 UTC
c7db432 Merge branch 'backport/jk/libcurl-8.7-regression-workaround' into maint-2.39 Fix was added to work around a regression in libcURL 8.7.0 (which has already been fixed in their tip of the tree). * jk/libcurl-8.7-regression-workaround: remote-curl: add Transfer-Encoding header only for older curl INSTALL: bump libcurl version to 7.21.3 http: reset POSTFIELDSIZE when clearing curl handle Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> 16 April 2024, 21:58:53 UTC
e1813a3 Merge branch 'jk/redact-h2h3-headers-fix' into maint-2.42 HTTP Header redaction code has been adjusted for a newer version of cURL library that shows its traces differently from earlier versions. * jk/redact-h2h3-headers-fix: http: update curl http/2 info matching for curl 8.3.0 http: factor out matching of curl http/2 trace lines This backport to `maint-2.39` is needed to bring the following test cases back to a working state in conjunction with recent libcurl versions: - t5559.17 GIT_TRACE_CURL redacts auth details - t5559.18 GIT_CURL_VERBOSE redacts auth details - t5559.38 cookies are redacted by default Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> 16 April 2024, 21:58:48 UTC
ef0fc42 Merge branch 'jk/httpd-test-updates' Test update. * jk/httpd-test-updates: t/lib-httpd: increase ssl key size to 2048 bits t/lib-httpd: drop SSLMutex config t/lib-httpd: bump required apache version to 2.4 t/lib-httpd: bump required apache version to 2.2 This is a backport onto the `maint-2.39` branch, to improve the CI health of that branch. Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> 16 April 2024, 21:58:40 UTC
e3cbeb9 Merge branch 'jk/http-test-fixes' Various fix-ups on HTTP tests. * jk/http-test-fixes: t5559: make SSL/TLS the default t5559: fix test failures with LIB_HTTPD_SSL t/lib-httpd: enable HTTP/2 "h2" protocol, not just h2c t/lib-httpd: respect $HTTPD_PROTO in expect_askpass() t5551: drop curl trace lines without headers t5551: handle v2 protocol in cookie test t5551: simplify expected cookie file t5551: handle v2 protocol in upload-pack service test t5551: handle v2 protocol when checking curl trace t5551: stop forcing clone to run with v0 protocol t5551: handle HTTP/2 when checking curl trace t5551: lower-case headers in expected curl trace t5551: drop redundant grep for Accept-Language t5541: simplify and move "no empty path components" test t5541: stop marking "used receive-pack service" test as v0 only t5541: run "used receive-pack service" test earlier This is a backport onto the `maint-2.39` branch, starting to take care of making that branch's CI builds healthy again. Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> 16 April 2024, 21:58:06 UTC
c67cf4c test-lib: ignore uninteresting LSan output When I run the tests in leak-checking mode the same way our CI job does, like: make SANITIZE=leak \ GIT_TEST_PASSING_SANITIZE_LEAK=true \ GIT_TEST_SANITIZE_LEAK_LOG=true \ test then LSan can racily produce useless entries in the log files that look like this: ==git==3034393==Unable to get registers from thread 3034307. I think they're mostly harmless based on the source here: https://github.com/llvm/llvm-project/blob/7e0a52e8e9ef6394bb62e0b56e17fa23e7262411/compiler-rt/lib/lsan/lsan_common.cpp#L414 which reads: PtraceRegistersStatus have_registers = suspended_threads.GetRegistersAndSP(i, &registers, &sp); if (have_registers != REGISTERS_AVAILABLE) { Report("Unable to get registers from thread %llu.\n", os_id); // If unable to get SP, consider the entire stack to be reachable unless // GetRegistersAndSP failed with ESRCH. if (have_registers == REGISTERS_UNAVAILABLE_FATAL) continue; sp = stack_begin; } The program itself still runs fine and LSan doesn't cause us to abort. But test-lib.sh looks for any non-empty LSan logs and marks the test as a failure anyway, under the assumption that we simply missed the failing exit code somehow. I don't think I've ever seen this happen in the CI job, but running locally using clang-14 on an 8-core machine, I can't seem to make it through a full run of the test suite without having at least one failure. And it's a different one every time (though they do seem to often be related to packing tests, which makes sense, since that is one of our biggest users of threaded code). We can hack around this by only counting LSan log files that contain a line that doesn't match our known-uninteresting pattern. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 16 April 2024, 21:58:04 UTC
cf5dcd8 ci(linux-asan/linux-ubsan): let's save some time Every once in a while, the `git-p4` tests flake for reasons outside of our control. It typically fails with "Connection refused" e.g. here: https://github.com/git/git/actions/runs/5969707156/job/16196057724 [...] + git p4 clone --dest=/home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git //depot Initialized empty Git repository in /home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git/.git/ Perforce client error: Connect to server failed; check $P4PORT. TCP connect to localhost:9807 failed. connect: 127.0.0.1:9807: Connection refused failure accessing depot: could not run p4 Importing from //depot into /home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git [...] This happens in other jobs, too, but in the `linux-asan`/`linux-ubsan` jobs it hurts the most because those jobs often take an _awfully_ long time to run, therefore re-running a failed `linux-asan`/`linux-ubsan` jobs is _very_ costly. The purpose of the `linux-asan`/`linux-ubsan` jobs is to exercise the C code of Git, anyway, and any part of Git's source code that the `git-p4` tests run and that would benefit from the attention of ASAN/UBSAN are run better in other tests anyway, as debugging C code run via Python scripts can get a bit hairy. In fact, it is not even just `git-p4` that is the problem (even if it flakes often enough to be problematic in the CI builds), but really the part about Python scripts. So let's just skip any Python parts of the tests from being run in that job. For good measure, also skip the Subversion tests because debugging C code run via Perl scripts is as much fun as debugging C code run via Python scripts. And it will reduce the time this very expensive job takes, which is a big benefit. Backported to `maint-2.39` as another step to get that branch's CI builds back to a healthy state. Backported-from: 6ba913629f (ci(linux-asan-ubsan): let's save some time, 2023-08-29) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 16 April 2024, 21:58:04 UTC
3167b60 ci: upgrade to using macos-13 In April, GitHub announced that the `macos-13` pool is available: https://github.blog/changelog/2023-04-24-github-actions-macos-13-is-now-available/. It is only a matter of time until the `macos-12` pool is going away, therefore we should switch now, without pressure of a looming deadline. Since the `macos-13` runners no longer include Python2, we also drop specifically testing with Python2 and switch uniformly to Python3, see https://github.com/actions/runner-images/blob/HEAD/images/macos/macos-13-Readme.md for details about the software available on the `macos-13` pool's runners. Also, on macOS 13, Homebrew seems to install a `gcc@9` package that no longer comes with a regular `unistd.h` (there seems only to be a `ssp/unistd.h`), and hence builds would fail with: In file included from base85.c:1: git-compat-util.h:223:10: fatal error: unistd.h: No such file or directory 223 | #include <unistd.h> | ^~~~~~~~~~ compilation terminated. The reason why we install GCC v9.x explicitly is historical, and back in the days it was because it was the _newest_ version available via Homebrew: 176441bfb58 (ci: build Git with GCC 9 in the 'osx-gcc' build job, 2019-11-27). To reinstate the spirit of that commit _and_ to fix that build failure, let's switch to the now-newest GCC version: v13.x. Backported-from: 682a868f67 (ci: upgrade to using macos-13, 2023-11-03) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 16 April 2024, 21:58:03 UTC
bddc176 Merge branch 'jh/fsmonitor-darwin-modernize' Stop using deprecated macOS API in fsmonitor. * jh/fsmonitor-darwin-modernize: fsmonitor: eliminate call to deprecated FSEventStream function This backport to `maint-2.39` is needed to be able to build on `macos-13`, which we need to update to as we restore the CI health of that branch. Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> 16 April 2024, 21:55:55 UTC
5d312ec remote-curl: add Transfer-Encoding header only for older curl As of curl 7.66.0, we don't need to manually specify a "chunked" Transfer-Encoding header. Instead, modern curl deduces the need for it in a POST that has a POSTFIELDSIZE of -1 and uses READFUNCTION rather than POSTFIELDS. That version is recent enough that we can't just drop the header; we need to do so conditionally. Since it's only a single line, it seems like the simplest thing would just be to keep setting it unconditionally (after all, the #ifdefs are much longer than the actual code). But there's another wrinkle: HTTP/2. Curl may choose to use HTTP/2 under the hood if the server supports it. And in that protocol, we do not use the chunked encoding for streaming at all. Most versions of curl handle this just fine by recognizing and removing the header. But there's a regression in curl 8.7.0 and 8.7.1 where it doesn't, and large requests over HTTP/2 are broken (which t5559 notices). That regression has since been fixed upstream, but not yet released. Make the setting of this header conditional, which will let Git work even with those buggy curl versions. And as a bonus, it serves as a reminder that we can eventually clean up the code as we bump the supported curl versions. This is a backport of 92a209bf24 (remote-curl: add Transfer-Encoding header only for older curl, 2024-04-05) into the `maint-2.39` branch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 10 April 2024, 17:24:48 UTC
ea61df9 INSTALL: bump libcurl version to 7.21.3 Our documentation claims we support curl versions back to 7.19.5. But we can no longer compile with that version since adding an unconditional use of CURLOPT_RESOLVE in 511cfd3bff (http: add custom hostname to IP address resolutions, 2022-05-16). That feature wasn't added to libcurl until 7.21.3. We could add #ifdefs to make this work back to 7.19.5. But given that nobody noticed the compilation failure in the intervening two years, it makes more sense to bump the version in the documentation to 7.21.3 (which is itself over 13 years old). We could perhaps go forward even more (which would let us drop some cruft from git-curl-compat.h), but this should be an obviously safe jump, and we can move forward later. Note that user-visible syntax for CURLOPT_RESOLVE has grown new features in subsequent curl versions. Our documentation mentions "+" and "-" entries, which require more recent versions than 7.21.3. We could perhaps clarify that in our docs, but it's probably not worth cluttering them with restrictions of ancient curl versions. This is a backport of c28ee09503 (INSTALL: bump libcurl version to 7.21.3, 2024-04-02) into the `maint-2.39` branch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 10 April 2024, 17:24:48 UTC
580097b http: reset POSTFIELDSIZE when clearing curl handle In get_active_slot(), we return a CURL handle that may have been used before (reusing them is good because it lets curl reuse the same connection across many requests). We set a few curl options back to defaults that may have been modified by previous requests. We reset POSTFIELDS to NULL, but do not reset POSTFIELDSIZE (which defaults to "-1"). This usually doesn't matter because most POSTs will set both fields together anyway. But there is one exception: when handling a large request in remote-curl's post_rpc(), we don't set _either_, and instead set a READFUNCTION to stream data into libcurl. This can interact weirdly with a stale POSTFIELDSIZE setting, because curl will assume it should read only some set number of bytes from our READFUNCTION. However, it has worked in practice because we also manually set a "Transfer-Encoding: chunked" header, which libcurl uses as a clue to set the POSTFIELDSIZE to -1 itself. So everything works, but we're better off resetting the size manually for a few reasons: - there was a regression in curl 8.7.0 where the chunked header detection didn't kick in, causing any large HTTP requests made by Git to fail. This has since been fixed (but not yet released). In the issue, curl folks recommended setting it explicitly to -1: https://github.com/curl/curl/issues/13229#issuecomment-2029826058 and it indeed works around the regression. So even though it won't be strictly necessary after the fix there, this will help folks who end up using the affected libcurl versions. - it's consistent with what a new curl handle would look like. Since get_active_slot() may or may not return a used handle, this reduces the possibility of heisenbugs that only appear with certain request patterns. Note that the recommendation in the curl issue is to actually drop the manual Transfer-Encoding header. Modern libcurl will add the header itself when streaming from a READFUNCTION. However, that code wasn't added until 802aa5ae2 (HTTP: use chunked Transfer-Encoding for HTTP_POST if size unknown, 2019-07-22), which is in curl 7.66.0. We claim to support back to 7.19.5, so those older versions still need the manual header. This is a backport of 3242311742 (http: reset POSTFIELDSIZE when clearing curl handle, 2024-04-02) into the `maint-2.39` branch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 10 April 2024, 17:24:48 UTC
0763c3a http: update curl http/2 info matching for curl 8.3.0 To redact header lines in http/2 curl traces, we have to parse past some prefix bytes that curl sticks in the info lines it passes to us. That changed once already, and we adapted in db30130165 (http: handle both "h2" and "h2h3" in curl info lines, 2023-06-17). Now it has changed again, in curl's fbacb14c4 (http2: cleanup trace messages, 2023-08-04), which was released in curl 8.3.0. Running a build of git linked against that version will fail to redact the trace (and as before, t5559 notices and complains). The format here is a little more complicated than the other ones, as it now includes a "stream id". This is not constant but is always numeric, so we can easily parse past it. We'll continue to match the old versions, of course, since we want to work with many different versions of curl. We can't even select one format at compile time, because the behavior depends on the runtime version of curl we use, not the version we build against. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 15 September 2023, 17:54:11 UTC
39fa527 http: factor out matching of curl http/2 trace lines We have to parse out curl's http/2 trace lines so we can redact their headers. We already match two different types of lines from various vintages of curl. In preparation for adding another (which will be slightly more complex), let's pull the matching into its own function, rather than doing it in the middle of a conditional. While we're doing so, let's expand the comment a bit to describe the two matches. That probably should have been part of db30130165 (http: handle both "h2" and "h2h3" in curl info lines, 2023-06-17), but will become even more important as we add new types. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 15 September 2023, 17:54:08 UTC
db30130 http: handle both "h2" and "h2h3" in curl info lines When redacting auth tokens in trace output from curl, we look for http/2 headers of the form "h2h3 [header: value]". This comes from b637a41ebe (http: redact curl h2h3 headers in info, 2022-11-11). But the "h2h3" prefix changed to just "h2" in curl's fc2f1e547 (http2: support HTTP/2 to forward proxies, non-tunneling, 2023-04-14). That's in released version curl 8.1.0; linking against that version means we'll fail to correctly redact the trace. Our t5559.17 notices and fails. We can fix this by matching either prefix, which should handle both old and new versions. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 17 June 2023, 16:08:31 UTC
9bbde12 Git 2.39.3 Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2023, 19:16:08 UTC
1562897 Sync with 2.38.5 * maint-2.38: (32 commits) Git 2.38.5 Git 2.37.7 Git 2.36.6 Git 2.35.8 Git 2.34.8 Git 2.33.8 Git 2.32.7 Git 2.31.8 tests: avoid using `test_i18ncmp` Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion ... 17 April 2023, 19:16:08 UTC
ec58344 Git 2.38.5 Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2023, 19:16:07 UTC
c96ecfe Sync with 2.37.7 * maint-2.37: (31 commits) Git 2.37.7 Git 2.36.6 Git 2.35.8 Git 2.34.8 Git 2.33.8 Git 2.32.7 Git 2.31.8 tests: avoid using `test_i18ncmp` Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion t0003: GETTEXT_POISON fix, part 1 ... 17 April 2023, 19:16:06 UTC
d27ae36 Git 2.37.7 Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2023, 19:16:05 UTC
1df551c Sync with 2.36.6 * maint-2.36: (30 commits) Git 2.36.6 Git 2.35.8 Git 2.34.8 Git 2.33.8 Git 2.32.7 Git 2.31.8 tests: avoid using `test_i18ncmp` Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion t0003: GETTEXT_POISON fix, part 1 t0033: GETTEXT_POISON fix ... 17 April 2023, 19:16:04 UTC
ecaa3db Git 2.36.6 Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2023, 19:16:03 UTC
62298de Sync with 2.35.8 * maint-2.35: (29 commits) Git 2.35.8 Git 2.34.8 Git 2.33.8 Git 2.32.7 Git 2.31.8 tests: avoid using `test_i18ncmp` Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion t0003: GETTEXT_POISON fix, part 1 t0033: GETTEXT_POISON fix http: support CURLOPT_PROTOCOLS_STR ... 17 April 2023, 19:16:02 UTC
7380a72 Git 2.35.8 Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2023, 19:16:00 UTC
8cd052e Sync with 2.34.8 * maint-2.34: (28 commits) Git 2.34.8 Git 2.33.8 Git 2.32.7 Git 2.31.8 tests: avoid using `test_i18ncmp` Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion t0003: GETTEXT_POISON fix, part 1 t0033: GETTEXT_POISON fix http: support CURLOPT_PROTOCOLS_STR http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION ... 17 April 2023, 19:15:59 UTC
abcb63f Git 2.34.8 Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2023, 19:15:57 UTC
d6e9f67 Sync with 2.33.8 * maint-2.33: (27 commits) Git 2.33.8 Git 2.32.7 Git 2.31.8 tests: avoid using `test_i18ncmp` Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion t0003: GETTEXT_POISON fix, part 1 t0033: GETTEXT_POISON fix http: support CURLOPT_PROTOCOLS_STR http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT ... 17 April 2023, 19:15:56 UTC
3a19048 Git 2.33.8 Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2023, 19:15:54 UTC
bcd874d Sync with 2.32.7 * maint-2.32: (26 commits) Git 2.32.7 Git 2.31.8 tests: avoid using `test_i18ncmp` Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion t0003: GETTEXT_POISON fix, part 1 t0033: GETTEXT_POISON fix http: support CURLOPT_PROTOCOLS_STR http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT ci: install python on ubuntu ... 17 April 2023, 19:15:52 UTC
b8787a9 Git 2.32.7 Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2023, 19:15:51 UTC
31f7fe5 Sync with 2.31.8 * maint-2.31: (25 commits) Git 2.31.8 tests: avoid using `test_i18ncmp` Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion t0003: GETTEXT_POISON fix, part 1 t0033: GETTEXT_POISON fix http: support CURLOPT_PROTOCOLS_STR http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT ci: install python on ubuntu ci: use the same version of p4 on both Linux and macOS ... 17 April 2023, 19:15:49 UTC
ea56f91 Git 2.31.8 Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2023, 19:15:47 UTC
92957d8 tests: avoid using `test_i18ncmp` Since `test_i18ncmp` was deprecated in v2.31.*, the instances added in v2.30.9 needed to be converted to `test_cmp` calls. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2023, 19:15:45 UTC
b524e89 Sync with 2.30.9 * maint-2.30: (23 commits) Git 2.30.9 gettext: avoid using gettext if the locale dir is not present apply --reject: overwrite existing `.rej` symlink if it exists http.c: clear the 'finished' member once we are done with it clone.c: avoid "exceeds maximum object size" error with GCC v12.x range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() t5604: GETTEXT_POISON fix, conclusion t5604: GETTEXT_POISON fix, part 1 t5619: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, conclusion t0003: GETTEXT_POISON fix, part 1 t0033: GETTEXT_POISON fix http: support CURLOPT_PROTOCOLS_STR http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT ci: install python on ubuntu ci: use the same version of p4 on both Linux and macOS ci: remove the pipe after "p4 -V" to catch errors github-actions: run gcc-8 on ubuntu-20.04 image ... 17 April 2023, 19:15:44 UTC
668f2d5 Git 2.30.9 Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2023, 19:15:43 UTC
528290f Merge branch 'tb/config-copy-or-rename-in-file-injection' Avoids issues with renaming or deleting sections with long lines, where configuration values may be interpreted as sections, leading to configuration injection. Addresses CVE-2023-29007. * tb/config-copy-or-rename-in-file-injection: config.c: disallow overly-long lines in `copy_or_rename_section_in_file()` config.c: avoid integer truncation in `copy_or_rename_section_in_file()` config: avoid fixed-sized buffer when renaming/deleting a section t1300: demonstrate failure when renaming sections with long lines Signed-off-by: Taylor Blau <me@ttaylorr.com> 17 April 2023, 19:15:42 UTC
4fe5d0b Merge branch 'avoid-using-uninitialized-gettext' Avoids the overhead of calling `gettext` when initialization of the translated messages was skipped. Addresses CVE-2023-25815. * avoid-using-uninitialized-gettext: (1 commit) gettext: avoid using gettext if the locale dir is not present 17 April 2023, 19:15:42 UTC
18e2b1c Merge branch 'js/apply-overwrite-rej-symlink-if-exists' into maint-2.30 Address CVE-2023-25652 by deleting any existing `.rej` symbolic links instead of following them. * js/apply-overwrite-rej-symlink-if-exists: apply --reject: overwrite existing `.rej` symlink if it exists Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> 17 April 2023, 19:15:41 UTC
3bb3d6b config.c: disallow overly-long lines in `copy_or_rename_section_in_file()` As a defense-in-depth measure to guard against any potentially-unknown buffer overflows in `copy_or_rename_section_in_file()`, refuse to work with overly-long lines in a gitconfig. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> 17 April 2023, 19:15:40 UTC
e91cfe6 config.c: avoid integer truncation in `copy_or_rename_section_in_file()` There are a couple of spots within `copy_or_rename_section_in_file()` that incorrectly use an `int` to track an offset within a string, which may truncate or wrap around to a negative value. Historically it was impossible to have a line longer than 1024 bytes anyway, since we used fgets() with a fixed-size buffer of exactly that length. But the recent change to use a strbuf permits us to read lines of arbitrary length, so it's possible for a malicious input to cause us to overflow past INT_MAX and do an out-of-bounds array read. Practically speaking, however, this should never happen, since it requires 2GB section names or values, which are unrealistic in non-malicious circumstances. Co-authored-by: Jeff King <peff@peff.net> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> 17 April 2023, 19:15:40 UTC
a5bb10f config: avoid fixed-sized buffer when renaming/deleting a section When renaming (or deleting) a section of configuration, Git uses the function `git_config_copy_or_rename_section_in_file()` to rewrite the configuration file after applying the rename or deletion to the given section. To do this, Git repeatedly calls `fgets()` to read the existing configuration data into a fixed size buffer. When the configuration value under `old_name` exceeds the size of the buffer, we will call `fgets()` an additional time even if there is no newline in the configuration file, since our read length is capped at `sizeof(buf)`. If the first character of the buffer (after zero or more characters satisfying `isspace()`) is a '[', Git will incorrectly treat it as beginning a new section when the original section is being removed. In other words, a configuration value satisfying this criteria can incorrectly be considered as a new secftion instead of a variable in the original section. Avoid this issue by using a variable-width buffer in the form of a strbuf rather than a fixed-with region on the stack. A couple of small points worth noting: - Using a strbuf will cause us to allocate arbitrary sizes to match the length of each line. In practice, we don't expect any reasonable configuration files to have lines that long, and a bandaid will be introduced in a later patch to ensure that this is the case. - We are using strbuf_getwholeline() here instead of strbuf_getline() in order to match `fgets()`'s behavior of leaving the trailing LF character on the buffer (as well as a trailing NUL). This could be changed later, but using strbuf_getwholeline() changes the least about this function's implementation, so it is picked as the safest path. - It is temping to want to replace the loop to skip over characters matching isspace() at the beginning of the buffer with a convenience function like `strbuf_ltrim()`. But this is the wrong approach for a couple of reasons: First, it involves a potentially large and expensive `memmove()` which we would like to avoid. Second, and more importantly, we also *do* want to preserve those spaces to avoid changing the output of other sections. In all, this patch is a minimal replacement of the fixed-width buffer in `git_config_copy_or_rename_section_in_file()` to instead use a `struct strbuf`. Reported-by: André Baptista <andre@ethiack.com> Reported-by: Vítor Pinho <vitor@ethiack.com> Helped-by: Patrick Steinhardt <ps@pks.im> Co-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Taylor Blau <me@ttaylorr.com> 17 April 2023, 19:15:40 UTC
2919821 t1300: demonstrate failure when renaming sections with long lines When renaming a configuration section which has an entry whose length exceeds the size of our buffer in config.c's implementation of `git_config_copy_or_rename_section_in_file()`, Git will incorrectly form a new configuration section with part of the data in the section being removed. In this instance, our first configuration file looks something like: [b] c = d <spaces> [a] e = f [a] g = h Here, we have two configuration values, "b.c", and "a.g". The value "[a] e = f" belongs to the configuration value "b.c", and does not form its own section. However, when renaming the section 'a' to 'xyz', Git will write back "[xyz]\ne = f", but "[xyz]" is still attached to the value of "b.c", which is why "e = f" on its own line becomes a new entry called "b.e". A slightly different example embeds the section being renamed within another section. Demonstrate this failure in a test in t1300, which we will fix in the following commit. Co-authored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Taylor Blau <me@ttaylorr.com> 17 April 2023, 19:15:39 UTC
c4137be gettext: avoid using gettext if the locale dir is not present In cc5e1bf99247 (gettext: avoid initialization if the locale dir is not present, 2018-04-21) Git was taught to avoid a costly gettext start-up when there are not even any localized messages to work with. But we still called `gettext()` and `ngettext()` functions. Which caused a problem in Git for Windows when the libgettext that is consumed from the MSYS2 project stopped using a runtime prefix in https://github.com/msys2/MINGW-packages/pull/10461 Due to that change, we now use an unintialized gettext machinery that might get auto-initialized _using an unintended locale directory_: `C:\mingw64\share\locale`. Let's record the fact when the gettext initialization was skipped, and skip calling the gettext functions accordingly. This addresses CVE-2023-25815. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2023, 19:15:39 UTC
9db0571 apply --reject: overwrite existing `.rej` symlink if it exists The `git apply --reject` is expected to write out `.rej` files in case one or more hunks fail to apply cleanly. Historically, the command overwrites any existing `.rej` files. The idea being that apply/reject/edit cycles are relatively common, and the generated `.rej` files are not considered precious. But the command does not overwrite existing `.rej` symbolic links, and instead follows them. This is unsafe because the same patch could potentially create such a symbolic link and point at arbitrary paths outside the current worktree, and `git apply` would write the contents of the `.rej` file into that location. Therefore, let's make sure that any existing `.rej` file or symbolic link is removed before writing it. Reported-by: RyotaK <ryotak.mail@gmail.com> Helped-by: Taylor Blau <me@ttaylorr.com> Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Linus Torvalds <torvalds@linuxfoundation.org> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 17 April 2023, 19:15:38 UTC
2f3b28f Merge branch 'js/gettext-poison-fixes' The `maint-2.30` branch accumulated quite a few fixes over the past two years. Most of those fixes were originally based on newer versions, and while the patches cherry-picked cleanly, we weren't diligent enough to pay attention to the CI builds and the GETTEXT_POISON job regressed. This topic branch fixes that. * js/gettext-poison-fixes t0033: GETTEXT_POISON fix t0003: GETTEXT_POISON fix, part 1 t0003: GETTEXT_POISON fix, conclusion t5619: GETTEXT_POISON fix t5604: GETTEXT_POISON fix, part 1 t5604: GETTEXT_POISON fix, conclusion 17 April 2023, 19:15:37 UTC
4989c35 Merge branch 'ds/github-actions-use-newer-ubuntu' Update the version of Ubuntu used for GitHub Actions CI from 18.04 to 22.04. * ds/github-actions-use-newer-ubuntu: ci: update 'static-analysis' to Ubuntu 22.04 17 April 2023, 19:15:36 UTC
fef08dd ci: update 'static-analysis' to Ubuntu 22.04 GitHub Actions scheduled a brownout of Ubuntu 18.04, which canceled all runs of the 'static-analysis' job in our CI runs. Update to 22.04 to avoid this as the brownout later turns into a complete deprecation. The use of 18.04 was set in d051ed77ee6 (.github/workflows/main.yml: run static-analysis on bionic, 2021-02-08) due to the lack of Coccinelle being available on 20.04 (which continues today). Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 17 April 2023, 16:17:53 UTC
8453685 Makefile: force -O0 when compiling with SANITIZE=leak Cherry pick commit d3775de0 (Makefile: force -O0 when compiling with SANITIZE=leak, 2022-10-18), as otherwise the leak checker at GitHub Actions CI seems to fail with a false positive. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 23 March 2023, 08:17:23 UTC
e4cb369 Merge branch 'backport/jk/range-diff-fixes' "git range-diff" code clean-up. Needed to pacify modern GCC versions. * jk/range-diff-fixes: range-diff: use ssize_t for parsed "len" in read_patches() range-diff: handle unterminated lines in read_patches() range-diff: drop useless "offset" variable from read_patches() 22 March 2023, 17:00:36 UTC
3c7896e Merge branch 'backport/jk/curl-avoid-deprecated-api' into maint-2.30 Deal with a few deprecation warning from cURL library. * jk/curl-avoid-deprecated-api: http: support CURLOPT_PROTOCOLS_STR http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT 22 March 2023, 17:00:36 UTC
6f5ff3a Merge branch 'backport/jx/ci-ubuntu-fix' into maint-2.30 Adjust the GitHub CI to newer ubuntu release. * jx/ci-ubuntu-fix: github-actions: run gcc-8 on ubuntu-20.04 image ci: install python on ubuntu ci: use the same version of p4 on both Linux and macOS ci: remove the pipe after "p4 -V" to catch errors 22 March 2023, 17:00:35 UTC
0737200 Merge branch 'backport/jc/http-clear-finished-pointer' into maint-2.30 Meant to go with js/ci-gcc-12-fixes. source: <xmqq7d68ytj8.fsf_-_@gitster.g> * jc/http-clear-finished-pointer: http.c: clear the 'finished' member once we are done with it 22 March 2023, 17:00:34 UTC
0a1dc55 Merge branch 'backport/js/ci-gcc-12-fixes' Fixes real problems noticed by gcc 12 and works around false positives. * js/ci-gcc-12-fixes: nedmalloc: avoid new compile error compat/win32/syslog: fix use-after-realloc 22 March 2023, 17:00:34 UTC
5843080 http.c: clear the 'finished' member once we are done with it In http.c, the run_active_slot() function allows the given "slot" to make progress by calling step_active_slots() in a loop repeatedly, and the loop is not left until the request held in the slot completes. Ages ago, we used to use the slot->in_use member to get out of the loop, which misbehaved when the request in "slot" completes (at which time, the result of the request is copied away from the slot, and the in_use member is cleared, making the slot ready to be reused), and the "slot" gets reused to service a different request (at which time, the "slot" becomes in_use again, even though it is for a different request). The loop terminating condition mistakenly thought that the original request has yet to be completed. Today's code, after baa7b67d (HTTP slot reuse fixes, 2006-03-10) fixed this issue, uses a separate "slot->finished" member that is set in run_active_slot() to point to an on-stack variable, and the code that completes the request in finish_active_slot() clears the on-stack variable via the pointer to signal that the particular request held by the slot has completed. It also clears the in_use member (as before that fix), so that the slot itself can safely be reused for an unrelated request. One thing that is not quite clean in this arrangement is that, unless the slot gets reused, at which point the finished member is reset to NULL, the member keeps the value of &finished, which becomes a dangling pointer into the stack when run_active_slot() returns. Clear the finished member before the control leaves the function, which has a side effect of unconfusing compilers like recent GCC 12 that is over-eager to warn against such an assignment. Signed-off-by: Junio C Hamano <gitster@pobox.com> 22 March 2023, 16:58:29 UTC
321854a clone.c: avoid "exceeds maximum object size" error with GCC v12.x Technically, the pointer difference `end - start` _could_ be negative, and when cast to an (unsigned) `size_t` that would cause problems. In this instance, the symptom is: dir.c: In function 'git_url_basename': dir.c:3087:13: error: 'memchr' specified bound [9223372036854775808, 0] exceeds maximum object size 9223372036854775807 [-Werror=stringop-overread] CC ewah/bitmap.o 3087 | if (memchr(start, '/', end - start) == NULL | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ While it is a bit far-fetched to think that `end` (which is defined as `repo + strlen(repo)`) and `start` (which starts at `repo` and never steps beyond the NUL terminator) could result in such a negative difference, GCC has no way of knowing that. See also https://gcc.gnu.org/bugzilla//show_bug.cgi?id=85783. Let's just add a safety check, primarily for GCC's benefit. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 22 March 2023, 16:53:32 UTC
0c8d22a t5604: GETTEXT_POISON fix, conclusion In fade728df122 (apply: fix writing behind newly created symbolic links, 2023-02-02), we backported a patch onto v2.30.* that was originally based on a much newer version. The v2.30.* release train still has the GETTEXT_POISON CI job, though, and hence needs `test_i18n*` in its tests. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 12 March 2023, 19:31:56 UTC
7c811ed t5604: GETTEXT_POISON fix, part 1 In bffc762f87ae (dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS, 2023-01-24), we backported a patch onto v2.30.* that was originally based on a much newer version. The v2.30.* release train still has the GETTEXT_POISON CI job, though, and hence needs `test_i18n*` in its tests. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 12 March 2023, 19:31:56 UTC
a2b2173 t5619: GETTEXT_POISON fix In cf8f6ce02a13 (clone: delay picking a transport until after get_repo_path(), 2023-01-24), we backported a patch onto v2.30.* that was originally based on a much newer version. The v2.30.* release train still has the GETTEXT_POISON CI job, though, and hence needs `test_i18n*` in its tests. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 12 March 2023, 19:31:56 UTC
c025b4b range-diff: use ssize_t for parsed "len" in read_patches() As we iterate through the buffer containing git-log output, parsing lines, we use an "int" to store the size of an individual line. This should be a size_t, as we have no guarantee that there is not a malicious 2GB+ commit-message line in the output. Overflowing this integer probably doesn't do anything _too_ terrible. We are not using the value to size a buffer, so the worst case is probably an out-of-bounds read from before the array. But it's easy enough to fix. Note that we have to use ssize_t here, since we also store the length result from parse_git_diff_header(), which may return a negative value for error. That function actually returns an int itself, which has a similar overflow problem, but I'll leave that for another day. Much of the apply.c code uses ints and should be converted as a whole; in the meantime, a negative return from parse_git_diff_header() will be interpreted as an error, and we'll bail (so we can't handle such a case, but given that it's likely to be malicious anyway, the important thing is we don't have any memory errors). Signed-off-by: Jeff King <peff@peff.net> Acked-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 12 March 2023, 19:31:55 UTC
a36df79 range-diff: handle unterminated lines in read_patches() When parsing our buffer of output from git-log, we have a find_end_of_line() helper that finds the next newline, and gives us the number of bytes to move past it, or the size of the whole remaining buffer if there is no newline. But trying to handle both those cases leads to some oddities: - we try to overwrite the newline with NUL in the caller, by writing over line[len-1]. This is at best redundant, since the helper will already have done so if it saw a newline. But if it didn't see a newline, it's actively wrong; we'll overwrite the byte at the end of the (unterminated) line. We could solve this just dropping the extra NUL assignment in the caller and just letting the helper do the right thing. But... - if we see a "diff --git" line, we'll restore the newline on top of the NUL byte, so we can pass the string to parse_git_diff_header(). But if there was no newline in the first place, we can't do this. There's no place to put it (the current code writes a newline over whatever byte we obliterated earlier). The best we can do is feed the complete remainder of the buffer to the function (which is, in fact, a string, by virtue of being a strbuf). To solve this, the caller needs to know whether we actually found a newline or not. We could modify find_end_of_line() to return that information, but we can further observe that it has only one caller. So let's just inline it in that caller. Nobody seems to have noticed this case, probably because git-log would never produce input that doesn't end with a newline. Arguably we could just return an error as soon as we see that the output does not end in a newline. But the code to do so actually ends up _longer_, mostly because of the cleanup we have to do in handling the error. Signed-off-by: Jeff King <peff@peff.net> Acked-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 12 March 2023, 19:31:55 UTC
d99728b t0003: GETTEXT_POISON fix, conclusion In 3c50032ff528 (attr: ignore overly large gitattributes files, 2022-12-01), we backported a patch onto v2.30.* that was originally based on a much newer version. The v2.30.* release train still has the GETTEXT_POISON CI job, though, and hence needs `test_i18n*` in its tests. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 12 March 2023, 19:31:55 UTC
e4298cc t0003: GETTEXT_POISON fix, part 1 In dfa6b32b5e59 (attr: ignore attribute lines exceeding 2048 bytes, 2022-12-01), we backported a patch onto v2.30.* that was originally based on a much newer version. The v2.30.* release train still has the GETTEXT_POISON CI job, though, and hence needs `test_i18n*` in its tests. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 12 March 2023, 19:31:55 UTC
8516dac t0033: GETTEXT_POISON fix In e47363e5a8bd (t0033: add tests for safe.directory, 2022-04-13), we backported a patch onto v2.30.* that was originally based on a much newer version. The v2.30.* release train still has the GETTEXT_POISON CI job, though, and hence needs `test_i18n*` in its tests. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 12 March 2023, 19:31:55 UTC
07f91e5 http: support CURLOPT_PROTOCOLS_STR The CURLOPT_PROTOCOLS (and matching CURLOPT_REDIR_PROTOCOLS) flag was deprecated in curl 7.85.0, and using it generate compiler warnings as of curl 7.87.0. The path forward is to use CURLOPT_PROTOCOLS_STR, but we can't just do so unilaterally, as it was only introduced less than a year ago in 7.85.0. Until that version becomes ubiquitous, we have to either disable the deprecation warning or conditionally use the "STR" variant on newer versions of libcurl. This patch switches to the new variant, which is nice for two reasons: - we don't have to worry that silencing curl's deprecation warnings might cause us to miss other more useful ones - we'd eventually want to move to the new variant anyway, so this gets us set up (albeit with some extra ugly boilerplate for the conditional) There are a lot of ways to split up the two cases. One way would be to abstract the storage type (strbuf versus a long), how to append (strbuf_addstr vs bitwise OR), how to initialize, which CURLOPT to use, and so on. But the resulting code looks pretty magical: GIT_CURL_PROTOCOL_TYPE allowed = GIT_CURL_PROTOCOL_TYPE_INIT; if (...http is allowed...) GIT_CURL_PROTOCOL_APPEND(&allowed, "http", CURLOPT_HTTP); and you end up with more "#define GIT_CURL_PROTOCOL_TYPE" macros than actual code. On the other end of the spectrum, we could just implement two separate functions, one that handles a string list and one that handles bits. But then we end up repeating our list of protocols (http, https, ftp, ftp). This patch takes the middle ground. The run-time code is always there to handle both types, and we just choose which one to feed to curl. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 12 March 2023, 19:31:54 UTC
18bc8eb range-diff: drop useless "offset" variable from read_patches() The "offset" variable was was introduced in 44b67cb62b (range-diff: split lines manually, 2019-07-11), but it has never done anything useful. We use it to count up the number of bytes we've consumed, but we never look at the result. It was probably copied accidentally from an almost-identical loop in apply.c:find_header() (and the point of that commit was to make use of the parse_git_diff_header() function which underlies both). Because the variable was set but not used, most compilers didn't seem to notice, but the upcoming clang-14 does complain about it, via its -Wunused-but-set-variable warning. Signed-off-by: Jeff King <peff@peff.net> Acked-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 12 March 2023, 19:31:54 UTC
a69043d ci: install python on ubuntu Python is missing from the default ubuntu-22.04 runner image, which prevents git-p4 from working. To install python on ubuntu, we need to provide the correct package names: * On Ubuntu 18.04 (bionic), "/usr/bin/python2" is provided by the "python" package, and "/usr/bin/python3" is provided by the "python3" package. * On Ubuntu 20.04 (focal) and above, "/usr/bin/python2" is provided by the "python2" package which has a different name from bionic, and "/usr/bin/python3" is provided by "python3". Since the "ubuntu-latest" runner image has a higher version, its safe to use "python2" or "python3" package name. Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 12 March 2023, 19:31:54 UTC
b0e3e2d http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION The IOCTLFUNCTION option has been deprecated, and generates a compiler warning in recent versions of curl. We can switch to using SEEKFUNCTION instead. It was added in 2008 via curl 7.18.0; our INSTALL file already indicates we require at least curl 7.19.4. But there's one catch: curl says we should use CURL_SEEKFUNC_{OK,FAIL}, and those didn't arrive until 7.19.5. One workaround would be to use a bare 0/1 here (or define our own macros). But let's just bump the minimum required version to 7.19.5. That version is only a minor version bump from our existing requirement, and is only a 2 month time bump for versions that are almost 13 years old. So it's not likely that anybody cares about the distinction. Switching means we have to rewrite the ioctl functions into seek functions. In some ways they are simpler (seeking is the only operation), but in some ways more complex (the ioctl allowed only a full rewind, but now we can seek to arbitrary offsets). Curl will only ever use SEEK_SET (per their documentation), so I didn't bother implementing anything else, since it would naturally be completely untested. This seems unlikely to change, but I added an assertion just in case. Likewise, I doubt curl will ever try to seek outside of the buffer sizes we've told it, but I erred on the defensive side here, rather than do an out-of-bounds read. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 12 March 2023, 19:31:54 UTC
fda237c http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT The two options do exactly the same thing, but the latter has been deprecated and in recent versions of curl may produce a compiler warning. Since the UPLOAD form is available everywhere (it was introduced in the year 2000 by curl 7.1), we can just switch to it. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 12 March 2023, 19:31:54 UTC
79e0626 ci: use the same version of p4 on both Linux and macOS There would be a segmentation fault when running p4 v16.2 on ubuntu 22.04 which is the latest version of ubuntu runner image for github actions. By checking each version from [1], p4d version 21.1 and above can work properly on ubuntu 22.04. But version 22.x will break some p4 test cases. So p4 version 21.x is exactly the version we can use. With this update, the versions of p4 for Linux and macOS happen to be the same. So we can add the version number directly into the "P4WHENCE" variable, and reuse it in p4 installation for macOS. By removing the "LINUX_P4_VERSION" variable from "ci/lib.sh", the comment left above has nothing to do with p4, but still applies to git-lfs. Since we have a fixed version of git-lfs installed on Linux, we may have a different version on macOS. [1]: https://cdist2.perforce.com/perforce/ Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 12 March 2023, 19:31:53 UTC
20854bc ci: remove the pipe after "p4 -V" to catch errors When installing p4 as a dependency, we used to pipe output of "p4 -V" and "p4d -V" to validate the installation and output a condensed version information. But this would hide potential errors of p4 and would stop with an empty output. E.g.: p4d version 16.2 running on ubuntu 22.04 causes sigfaults, even before it produces any output. By removing the pipe after "p4 -V" and "p4d -V", we may get a verbose output, and stop immediately on errors because we have "set -e" in "ci/lib.sh". Since we won't look at these trace logs unless something fails, just including the raw output seems most sensible. Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 12 March 2023, 19:31:53 UTC
86f6f4f nedmalloc: avoid new compile error GCC v12.x complains thusly: compat/nedmalloc/nedmalloc.c: In function 'DestroyCaches': compat/nedmalloc/nedmalloc.c:326:12: error: the comparison will always evaluate as 'true' for the address of 'caches' will never be NULL [-Werror=address] 326 | if(p->caches) | ^ compat/nedmalloc/nedmalloc.c:196:22: note: 'caches' declared here 196 | threadcache *caches[THREADCACHEMAXCACHES]; | ^~~~~~ ... and it is correct, of course. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 12 March 2023, 19:31:53 UTC
c03ffcf github-actions: run gcc-8 on ubuntu-20.04 image GitHub starts to upgrade its runner image "ubuntu-latest" from version "ubuntu-20.04" to version "ubuntu-22.04". It will fail to find and install "gcc-8" package on the new runner image. Change the runner image of the `linux-gcc` job from "ubuntu-latest" to "ubuntu-20.04" in order to install "gcc-8" as a dependency. Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 12 March 2023, 19:31:53 UTC
417fb91 compat/win32/syslog: fix use-after-realloc Git for Windows' SDK recently upgraded to GCC v12.x which points out that the `pos` variable might be used even after the corresponding memory was `realloc()`ed and therefore potentially no longer valid. Since a subset of this SDK is used in Git's CI/PR builds, we need to fix this to continue to be able to benefit from the CI/PR runs. Note: This bug has been with us since 2a6b149c64f6 (mingw: avoid using strbuf in syslog, 2011-10-06), and while it looks tempting to replace the hand-rolled string manipulation with a `strbuf`-based one, that commit's message explains why we cannot do that: The `syslog()` function is called as part of the function in `daemon.c` which is set as the `die()` routine, and since `strbuf_grow()` can call that function if it runs out of memory, this would cause a nasty infinite loop that we do not want to re-introduce. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 12 March 2023, 19:31:52 UTC
8f2146d t5559: make SSL/TLS the default The point of t5559 is run the regular t5551 tests with HTTP/2. But it does so with the "h2c" protocol, which uses cleartext upgrades from HTTP/1.1 to HTTP/2 (rather than learning about HTTP/2 support during the TLS negotiation). This has a few problems: - it's not very indicative of the real world. In practice, most servers that support HTTP/2 will also support TLS. - support for upgrading does not seem as robust. In particular, we've run into bugs in some versions of Apache's mod_http2 that trigger only with the upgrade mode. See: https://lore.kernel.org/git/Y8ztIqYgVCPILJlO@coredump.intra.peff.net/ So the upside is that this change makes our HTTP/2 tests more robust and more realistic. The downside is that if we can't set up SSL for any reason, we'll skip the tests (even though you _might_ have been able to run the HTTP/2 tests the old way). We could probably have a conditional fallback, but it would be complicated for little gain, and it's not even clear it would help (i.e., would any test environment even have HTTP/2 but not SSL support?). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 23 February 2023, 21:01:18 UTC
8619002 t5559: fix test failures with LIB_HTTPD_SSL One test needs to be tweaked in order for t5559 to pass with SSL/TLS set up. When we make our initial clone, we check that the curl trace of requests is what we expected. But we need to fix two things: - along with ignoring "data" lines from the trace, we need to ignore "SSL data" lines - when TLS is used, the server is able to tell the client (via ALPN) that it supports HTTP/2 before the first HTTP request is made. So rather than request an upgrade using an HTTP header, it can just speak HTTP/2 immediately With this patch, running: LIB_HTTPD_SSL=1 ./t5559-http-fetch-smart-http2.sh works, whereas it did not before. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 23 February 2023, 21:01:17 UTC
3c14419 t/lib-httpd: enable HTTP/2 "h2" protocol, not just h2c Commit 73c49a4474 (t: run t5551 tests with both HTTP and HTTP/2, 2022-11-11) added Apache config to enable HTTP/2. However, it only enabled the "h2c" protocol, which allows cleartext HTTP/2 (generally based on an upgrade header during an HTTP/1.1 request). This is what t5559 is generally testing, since by default we don't set up SSL/TLS. However, it should be possible to run t5559 with LIB_HTTPD_SSL set. In that case, Apache will advertise support for HTTP/2 via ALPN during the TLS handshake. But we need to tell it support "h2" (the non-cleartext version) to do so. Without that, then curl does not even try to do the HTTP/1.1 upgrade (presumably because after seeing that we did TLS but didn't get the ALPN indicator, it assumes it would be fruitless). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 23 February 2023, 21:01:17 UTC
9d15b1e t/lib-httpd: respect $HTTPD_PROTO in expect_askpass() When the HTTP tests are run with LIB_HTTPD_SSL in the environment, then we access the test server as https://. This causes expect_askpass to complain, because it tries to blindly match "http://" in the prompt shown to the user. We can adjust this to use $HTTPD_PROTO, which is set during the setup phase. Note that this is enough for t5551 and t5559 to pass when run with https, but there are similar problems in other scripts that will need to be fixed before the whole suite can run with LIB_HTTPD_SSL. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 23 February 2023, 21:01:15 UTC
back to top