undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
3dbb0ff340
git-commit-vandalism/t/t5551-http-fetch-smart.sh

708 lines
22 KiB
Bash
Raw Normal View History

test smart http fetch and push The top level directory "/smart/" of the test Apache server is mapped through our git-http-backend CGI, but uses the same underlying repository space as the server's document root. This is the most simple installation possible. Server logs are checked to verify the client has accessed only the smart URLs during the test. During fetch testing the headers are also logged from libcurl to ensure we are making a reasonably sane HTTP request, and getting back reasonably sane response headers from the CGI. When validating the request headers used during smart fetch we munge away the actual Content-Length and replace it with the placeholder "xxx". This avoids unnecessary varability in the test caused by an unrelated change in the requested capabilities in the first want line of the request. However, we still want to look for and verify that Content-Length was used, because smaller payloads should be using Content-Length and not "Transfer-Encoding: chunked". When validating the server response headers we must discard both Content-Length and Transfer-Encoding, as Apache2 can use either format to return our response. During development of this test I observed Apache returning both forms, depending on when the processes got CPU time. If our CGI returned the pack data quickly, Apache just buffered the whole thing and returned a Content-Length. If our CGI took just a bit too long to complete, Apache flushed its buffer and instead used "Transfer-Encoding: chunked". Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-31 01:47:47 +01:00
#!/bin/sh
t5551: handle HTTP/2 when checking curl trace We check that the curl trace of a clone has the lines we expect, but this won't work when we run the test under t5559, because a few details are different under HTTP/2 (but nobody noticed because it only happens when you manually set GIT_TEST_PROTOCOL_VERSION to "0"). We can handle both HTTP protocols with a few tweaks: - we'll drop the HTTP "101 Switching Protocols" response, as well as various protocol upgrade headers. These details aren't interesting to us. We just want to make sure the correct protocol was used (and we do in the main request/response lines). - successful HTTP/2 responses just say "200" and not "200 OK"; we can normalize these - replace HTTP/1.1 with a variable in the request/response lines. We can use the existing $HTTP_PROTO for this, as it's already set to "HTTP/2" when appropriate. We do need to tweak the fallback value to "HTTP/1.1" to match what curl will write (prior to this patch, the fallback value didn't matter at all; we only checked if it was the literal string "HTTP/2"). Note that several lines still expect HTTP/1.1 unconditionally. The first request does so because the client requests an upgrade during the request. The POST request and response do so because you can't do an upgrade if there is a request body. (This will all be different if we trigger HTTP/2 via ALPN, but the tests aren't yet capable of that). This is enough to let: GIT_TEST_PROTOCOL_VERSION=0 ./t5559-http-fetch-smart-http2.sh pass the "clone http repository" test (but there are some other failures later on). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:56:17 +01:00
: ${HTTP_PROTO:=HTTP/1.1}
t: run t5551 tests with both HTTP and HTTP/2 We have occasionally seen bugs that affect Git running only against an HTTP/2 web server, not an HTTP one. For instance, b66c77a64e (http: match headers case-insensitively when redacting, 2021-09-22). But since we have no test coverage using HTTP/2, we only uncover these bugs in the wild. That commit gives a recipe for converting our Apache setup to support HTTP/2, but: - it's not necessarily portable - we don't want to just test HTTP/2; we really want to do a variety of basic tests for _both_ protocols This patch handles both problems by running a duplicate of t5551 (labeled as t5559 here) with an alternate-universe setup that enables HTTP/2. So we'll continue to run t5551 as before, but run the same battery of tests again with HTTP/2. If HTTP/2 isn't supported on a given platform, then t5559 should bail during the webserver setup, and gracefully skip all tests (unless GIT_TEST_HTTPD has been changed from "auto" to "yes", where the point is to complain when webserver setup fails). In theory other http-related test scripts could benefit from the same duplication, but doing t5551 should give us a reasonable check of basic functionality, and would have caught both bugs we've seen in the wild with HTTP/2. A few notes on the implementation: - a script enables the server side config by calling enable_http2 before starting the webserver. This avoids even trying to load any HTTP/2 config for t5551 (which is what lets it keep working with regular HTTP even on systems that don't support it). This also sets a prereq which can be used by individual tests. - As discussed in b66c77a64e, the http2 module isn't compatible with the "prefork" mpm, so we need to pick something else. I chose "event" here, which works on my Debian system, but it's possible there are platforms which would prefer something else. We can adjust that later if somebody finds such a platform. - The test "large fetch-pack requests can be sent using chunked encoding" makes sure we use a chunked transfer-encoding by looking for that header in the trace. But since HTTP/2 has its own streaming mechanisms, we won't find such a header. We could skip the test entirely by marking it with !HTTP2. But there's some value in making sure that the fetch itself succeeded. So instead, we'll confirm that either we're using HTTP2 _or_ we saw the expected chunked header. - the redaction tests fail under HTTP/2 with recent versions of curl. This is a bug! I've marked them with !HTTP2 here to skip them under t5559 for the moment. Using test_expect_failure would be more appropriate, but would require a bunch of boilerplate. Since we'll be fixing them momentarily, let's just skip them for now to keep the test suite bisectable, and we can re-enable them in the commit that fixes the bug. - one alternative layout would be to push most of t5551 into a lib-t5551.sh script, then source it from both t5551 and t5559. Keeping t5551 intact seemed a little simpler, as its one less level of indirection for people fixing bugs/regressions in the non-HTTP/2 tests. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-11 23:35:05 +01:00
test_description="test smart fetching over http via http-backend ($HTTP_PROTO)"
t55[4-9]*: adjust the references to the default branch name "main" This trick was performed via $ (cd t && sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \ -e 's/Master/Main/g' -e 's/retsam/niam/g' \ -- t55[4-9]*.sh t556x*) This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main` for those tests. Note that t5541 uses the reversed `master` name: `retsam`. We replace it by the equivalent for `main`: `niam`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-19 00:44:34 +01:00
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
tests: mark tests relying on the current default for `init.defaultBranch` In addition to the manual adjustment to let the `linux-gcc` CI job run the test suite with `master` and then with `main`, this patch makes sure that GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME is set in all test scripts that currently rely on the initial branch name being `master by default. To determine which test scripts to mark up, the first step was to force-set the default branch name to `master` in - all test scripts that contain the keyword `master`, - t4211, which expects `t/t4211/history.export` with a hard-coded ref to initialize the default branch, - t5560 because it sources `t/t556x_common` which uses `master`, - t8002 and t8012 because both source `t/annotate-tests.sh` which also uses `master`) This trick was performed by this command: $ sed -i '/^ *\. \.\/\(test-lib\|lib-\(bash\|cvs\|git-svn\)\|gitweb-lib\)\.sh$/i\ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\ ' $(git grep -l master t/t[0-9]*.sh) \ t/t4211*.sh t/t5560*.sh t/t8002*.sh t/t8012*.sh After that, careful, manual inspection revealed that some of the test scripts containing the needle `master` do not actually rely on a specific default branch name: either they mention `master` only in a comment, or they initialize that branch specificially, or they do not actually refer to the current default branch. Therefore, the aforementioned modification was undone in those test scripts thusly: $ git checkout HEAD -- \ t/t0027-auto-crlf.sh t/t0060-path-utils.sh \ t/t1011-read-tree-sparse-checkout.sh \ t/t1305-config-include.sh t/t1309-early-config.sh \ t/t1402-check-ref-format.sh t/t1450-fsck.sh \ t/t2024-checkout-dwim.sh \ t/t2106-update-index-assume-unchanged.sh \ t/t3040-subprojects-basic.sh t/t3301-notes.sh \ t/t3308-notes-merge.sh t/t3423-rebase-reword.sh \ t/t3436-rebase-more-options.sh \ t/t4015-diff-whitespace.sh t/t4257-am-interactive.sh \ t/t5323-pack-redundant.sh t/t5401-update-hooks.sh \ t/t5511-refspec.sh t/t5526-fetch-submodules.sh \ t/t5529-push-errors.sh t/t5530-upload-pack-error.sh \ t/t5548-push-porcelain.sh \ t/t5552-skipping-fetch-negotiator.sh \ t/t5572-pull-submodule.sh t/t5608-clone-2gb.sh \ t/t5614-clone-submodules-shallow.sh \ t/t7508-status.sh t/t7606-merge-custom.sh \ t/t9302-fast-import-unpack-limit.sh We excluded one set of test scripts in these commands, though: the range of `git p4` tests. The reason? `git p4` stores the (foreign) remote branch in the branch called `p4/master`, which is obviously not the default branch. Manual analysis revealed that only five of these tests actually require a specific default branch name to pass; They were modified thusly: $ sed -i '/^ *\. \.\/lib-git-p4\.sh$/i\ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\ ' t/t980[0167]*.sh t/t9811*.sh Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-19 00:44:19 +01:00
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
test smart http fetch and push The top level directory "/smart/" of the test Apache server is mapped through our git-http-backend CGI, but uses the same underlying repository space as the server's document root. This is the most simple installation possible. Server logs are checked to verify the client has accessed only the smart URLs during the test. During fetch testing the headers are also logged from libcurl to ensure we are making a reasonably sane HTTP request, and getting back reasonably sane response headers from the CGI. When validating the request headers used during smart fetch we munge away the actual Content-Length and replace it with the placeholder "xxx". This avoids unnecessary varability in the test caused by an unrelated change in the requested capabilities in the first want line of the request. However, we still want to look for and verify that Content-Length was used, because smaller payloads should be using Content-Length and not "Transfer-Encoding: chunked". When validating the server response headers we must discard both Content-Length and Transfer-Encoding, as Apache2 can use either format to return our response. During development of this test I observed Apache returning both forms, depending on when the processes got CPU time. If our CGI returned the pack data quickly, Apache just buffered the whole thing and returned a Content-Length. If our CGI took just a bit too long to complete, Apache flushed its buffer and instead used "Transfer-Encoding: chunked". Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-31 01:47:47 +01:00
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-httpd.sh
t: run t5551 tests with both HTTP and HTTP/2 We have occasionally seen bugs that affect Git running only against an HTTP/2 web server, not an HTTP one. For instance, b66c77a64e (http: match headers case-insensitively when redacting, 2021-09-22). But since we have no test coverage using HTTP/2, we only uncover these bugs in the wild. That commit gives a recipe for converting our Apache setup to support HTTP/2, but: - it's not necessarily portable - we don't want to just test HTTP/2; we really want to do a variety of basic tests for _both_ protocols This patch handles both problems by running a duplicate of t5551 (labeled as t5559 here) with an alternate-universe setup that enables HTTP/2. So we'll continue to run t5551 as before, but run the same battery of tests again with HTTP/2. If HTTP/2 isn't supported on a given platform, then t5559 should bail during the webserver setup, and gracefully skip all tests (unless GIT_TEST_HTTPD has been changed from "auto" to "yes", where the point is to complain when webserver setup fails). In theory other http-related test scripts could benefit from the same duplication, but doing t5551 should give us a reasonable check of basic functionality, and would have caught both bugs we've seen in the wild with HTTP/2. A few notes on the implementation: - a script enables the server side config by calling enable_http2 before starting the webserver. This avoids even trying to load any HTTP/2 config for t5551 (which is what lets it keep working with regular HTTP even on systems that don't support it). This also sets a prereq which can be used by individual tests. - As discussed in b66c77a64e, the http2 module isn't compatible with the "prefork" mpm, so we need to pick something else. I chose "event" here, which works on my Debian system, but it's possible there are platforms which would prefer something else. We can adjust that later if somebody finds such a platform. - The test "large fetch-pack requests can be sent using chunked encoding" makes sure we use a chunked transfer-encoding by looking for that header in the trace. But since HTTP/2 has its own streaming mechanisms, we won't find such a header. We could skip the test entirely by marking it with !HTTP2. But there's some value in making sure that the fetch itself succeeded. So instead, we'll confirm that either we're using HTTP2 _or_ we saw the expected chunked header. - the redaction tests fail under HTTP/2 with recent versions of curl. This is a bug! I've marked them with !HTTP2 here to skip them under t5559 for the moment. Using test_expect_failure would be more appropriate, but would require a bunch of boilerplate. Since we'll be fixing them momentarily, let's just skip them for now to keep the test suite bisectable, and we can re-enable them in the commit that fixes the bug. - one alternative layout would be to push most of t5551 into a lib-t5551.sh script, then source it from both t5551 and t5559. Keeping t5551 intact seemed a little simpler, as its one less level of indirection for people fixing bugs/regressions in the non-HTTP/2 tests. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-11 23:35:05 +01:00
test "$HTTP_PROTO" = "HTTP/2" && enable_http2
test smart http fetch and push The top level directory "/smart/" of the test Apache server is mapped through our git-http-backend CGI, but uses the same underlying repository space as the server's document root. This is the most simple installation possible. Server logs are checked to verify the client has accessed only the smart URLs during the test. During fetch testing the headers are also logged from libcurl to ensure we are making a reasonably sane HTTP request, and getting back reasonably sane response headers from the CGI. When validating the request headers used during smart fetch we munge away the actual Content-Length and replace it with the placeholder "xxx". This avoids unnecessary varability in the test caused by an unrelated change in the requested capabilities in the first want line of the request. However, we still want to look for and verify that Content-Length was used, because smaller payloads should be using Content-Length and not "Transfer-Encoding: chunked". When validating the server response headers we must discard both Content-Length and Transfer-Encoding, as Apache2 can use either format to return our response. During development of this test I observed Apache returning both forms, depending on when the processes got CPU time. If our CGI returned the pack data quickly, Apache just buffered the whole thing and returned a Content-Length. If our CGI took just a bit too long to complete, Apache flushed its buffer and instead used "Transfer-Encoding: chunked". Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-31 01:47:47 +01:00
start_httpd
t: run t5551 tests with both HTTP and HTTP/2 We have occasionally seen bugs that affect Git running only against an HTTP/2 web server, not an HTTP one. For instance, b66c77a64e (http: match headers case-insensitively when redacting, 2021-09-22). But since we have no test coverage using HTTP/2, we only uncover these bugs in the wild. That commit gives a recipe for converting our Apache setup to support HTTP/2, but: - it's not necessarily portable - we don't want to just test HTTP/2; we really want to do a variety of basic tests for _both_ protocols This patch handles both problems by running a duplicate of t5551 (labeled as t5559 here) with an alternate-universe setup that enables HTTP/2. So we'll continue to run t5551 as before, but run the same battery of tests again with HTTP/2. If HTTP/2 isn't supported on a given platform, then t5559 should bail during the webserver setup, and gracefully skip all tests (unless GIT_TEST_HTTPD has been changed from "auto" to "yes", where the point is to complain when webserver setup fails). In theory other http-related test scripts could benefit from the same duplication, but doing t5551 should give us a reasonable check of basic functionality, and would have caught both bugs we've seen in the wild with HTTP/2. A few notes on the implementation: - a script enables the server side config by calling enable_http2 before starting the webserver. This avoids even trying to load any HTTP/2 config for t5551 (which is what lets it keep working with regular HTTP even on systems that don't support it). This also sets a prereq which can be used by individual tests. - As discussed in b66c77a64e, the http2 module isn't compatible with the "prefork" mpm, so we need to pick something else. I chose "event" here, which works on my Debian system, but it's possible there are platforms which would prefer something else. We can adjust that later if somebody finds such a platform. - The test "large fetch-pack requests can be sent using chunked encoding" makes sure we use a chunked transfer-encoding by looking for that header in the trace. But since HTTP/2 has its own streaming mechanisms, we won't find such a header. We could skip the test entirely by marking it with !HTTP2. But there's some value in making sure that the fetch itself succeeded. So instead, we'll confirm that either we're using HTTP2 _or_ we saw the expected chunked header. - the redaction tests fail under HTTP/2 with recent versions of curl. This is a bug! I've marked them with !HTTP2 here to skip them under t5559 for the moment. Using test_expect_failure would be more appropriate, but would require a bunch of boilerplate. Since we'll be fixing them momentarily, let's just skip them for now to keep the test suite bisectable, and we can re-enable them in the commit that fixes the bug. - one alternative layout would be to push most of t5551 into a lib-t5551.sh script, then source it from both t5551 and t5559. Keeping t5551 intact seemed a little simpler, as its one less level of indirection for people fixing bugs/regressions in the non-HTTP/2 tests. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-11 23:35:05 +01:00
test_expect_success HTTP2 'enable client-side http/2' '
git config --global http.version HTTP/2
'
test smart http fetch and push The top level directory "/smart/" of the test Apache server is mapped through our git-http-backend CGI, but uses the same underlying repository space as the server's document root. This is the most simple installation possible. Server logs are checked to verify the client has accessed only the smart URLs during the test. During fetch testing the headers are also logged from libcurl to ensure we are making a reasonably sane HTTP request, and getting back reasonably sane response headers from the CGI. When validating the request headers used during smart fetch we munge away the actual Content-Length and replace it with the placeholder "xxx". This avoids unnecessary varability in the test caused by an unrelated change in the requested capabilities in the first want line of the request. However, we still want to look for and verify that Content-Length was used, because smaller payloads should be using Content-Length and not "Transfer-Encoding: chunked". When validating the server response headers we must discard both Content-Length and Transfer-Encoding, as Apache2 can use either format to return our response. During development of this test I observed Apache returning both forms, depending on when the processes got CPU time. If our CGI returned the pack data quickly, Apache just buffered the whole thing and returned a Content-Length. If our CGI took just a bit too long to complete, Apache flushed its buffer and instead used "Transfer-Encoding: chunked". Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-31 01:47:47 +01:00
test_expect_success 'setup repository' '
t5551: do not assume the "matching" push is the default Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-01-16 03:05:07 +01:00
git config push.default matching &&
test smart http fetch and push The top level directory "/smart/" of the test Apache server is mapped through our git-http-backend CGI, but uses the same underlying repository space as the server's document root. This is the most simple installation possible. Server logs are checked to verify the client has accessed only the smart URLs during the test. During fetch testing the headers are also logged from libcurl to ensure we are making a reasonably sane HTTP request, and getting back reasonably sane response headers from the CGI. When validating the request headers used during smart fetch we munge away the actual Content-Length and replace it with the placeholder "xxx". This avoids unnecessary varability in the test caused by an unrelated change in the requested capabilities in the first want line of the request. However, we still want to look for and verify that Content-Length was used, because smaller payloads should be using Content-Length and not "Transfer-Encoding: chunked". When validating the server response headers we must discard both Content-Length and Transfer-Encoding, as Apache2 can use either format to return our response. During development of this test I observed Apache returning both forms, depending on when the processes got CPU time. If our CGI returned the pack data quickly, Apache just buffered the whole thing and returned a Content-Length. If our CGI took just a bit too long to complete, Apache flushed its buffer and instead used "Transfer-Encoding: chunked". Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-31 01:47:47 +01:00
echo content >file &&
git add file &&
git commit -m one
'
test_expect_success 'create http-accessible bare repository' '
mkdir "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
git --bare init
) &&
git remote add public "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
t55[4-9]*: adjust the references to the default branch name "main" This trick was performed via $ (cd t && sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \ -e 's/Master/Main/g' -e 's/retsam/niam/g' \ -- t55[4-9]*.sh t556x*) This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main` for those tests. Note that t5541 uses the reversed `master` name: `retsam`. We replace it by the equivalent for `main`: `niam`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-19 00:44:34 +01:00
git push public main:main
test smart http fetch and push The top level directory "/smart/" of the test Apache server is mapped through our git-http-backend CGI, but uses the same underlying repository space as the server's document root. This is the most simple installation possible. Server logs are checked to verify the client has accessed only the smart URLs during the test. During fetch testing the headers are also logged from libcurl to ensure we are making a reasonably sane HTTP request, and getting back reasonably sane response headers from the CGI. When validating the request headers used during smart fetch we munge away the actual Content-Length and replace it with the placeholder "xxx". This avoids unnecessary varability in the test caused by an unrelated change in the requested capabilities in the first want line of the request. However, we still want to look for and verify that Content-Length was used, because smaller payloads should be using Content-Length and not "Transfer-Encoding: chunked". When validating the server response headers we must discard both Content-Length and Transfer-Encoding, as Apache2 can use either format to return our response. During development of this test I observed Apache returning both forms, depending on when the processes got CPU time. If our CGI returned the pack data quickly, Apache just buffered the whole thing and returned a Content-Length. If our CGI took just a bit too long to complete, Apache flushed its buffer and instead used "Transfer-Encoding: chunked". Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-31 01:47:47 +01:00
'
t: test basic smart-http authentication We do not currently test authentication over smart-http at all. In theory, it should work exactly as it does for dumb http (which we do test). It does indeed work for these simple tests, but this patch lays the groundwork for more complex tests in future patches. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-08-27 15:25:36 +02:00
setup_askpass_helper
test smart http fetch and push The top level directory "/smart/" of the test Apache server is mapped through our git-http-backend CGI, but uses the same underlying repository space as the server's document root. This is the most simple installation possible. Server logs are checked to verify the client has accessed only the smart URLs during the test. During fetch testing the headers are also logged from libcurl to ensure we are making a reasonably sane HTTP request, and getting back reasonably sane response headers from the CGI. When validating the request headers used during smart fetch we munge away the actual Content-Length and replace it with the placeholder "xxx". This avoids unnecessary varability in the test caused by an unrelated change in the requested capabilities in the first want line of the request. However, we still want to look for and verify that Content-Length was used, because smaller payloads should be using Content-Length and not "Transfer-Encoding: chunked". When validating the server response headers we must discard both Content-Length and Transfer-Encoding, as Apache2 can use either format to return our response. During development of this test I observed Apache returning both forms, depending on when the processes got CPU time. If our CGI returned the pack data quickly, Apache just buffered the whole thing and returned a Content-Length. If our CGI took just a bit too long to complete, Apache flushed its buffer and instead used "Transfer-Encoding: chunked". Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-31 01:47:47 +01:00
test_expect_success 'clone http repository' '
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>
2023-02-23 12:07:29 +01:00
if test_have_prereq HTTP2 && test "$HTTPD_PROTO" = "https"
then
# ALPN lets us immediately use HTTP/2; likewise, POSTs with
# bodies can use it because they do not need to upgrade
INITIAL_PROTO=HTTP/2
else
# either we are not using HTTP/2, or the initial
# request is sent via HTTP/1.1 and asks for upgrade
INITIAL_PROTO=HTTP/1.1
fi &&
t5551: handle v2 protocol when checking curl trace After cloning an http repository, we check the curl trace to make sure the expected requests were made. But since the expected trace was never updated to handle v2, it is only run when you ask the test suite to run in v0 mode (which hardly anybody does). Let's update it to handle both protocols. This isn't too hard since v2 just sends an extra header and an extra request. So we can just annotate those extra lines and strip them out for v0 (and drop the annotations for v2). I didn't bother handling v1 here, as it's not really of practical interest (it would drop the extra v2 request, but still have the "git-protocol" lines). There's a similar tweak needed at the end. Since we check the "accept-encoding" value loosely, we grep for it rather than finding it in the verbatim trace. This grep insists that there are exactly 2 matches, but of course in v2 with the extra request there are 3. We could tweak the number, but it's simpler still to just check that we saw at least one match. The verbatim check already confirmed how many instances of the header we have; we're really just checking here that "gzip" is in the value (it's possible, of course, that the headers could have different values, but that seems like an unlikely bug). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:59:00 +01:00
cat >exp.raw <<-EOF &&
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>
2023-02-23 12:07:29 +01:00
> GET /smart/repo.git/info/refs?service=git-upload-pack $INITIAL_PROTO
t5551: lower-case headers in expected curl trace There's a test in t5551 which checks the curl trace (after simplifying it a bit). It doesn't work with HTTP/2, because in that case curl outputs all of the headers in lower-case. Even though this test is run with HTTP/2 by t5559, nobody has noticed because checking the trace only happens if GIT_TEST_PROTOCOL_VERSION is manually set to "0". Let's fix this by lower-casing all of the header names in the trace, and then checking for those in our expected code (this is easier than making HTTP/2 traces look like HTTP/1.1, since HTTP/1.1 uses title-casing). Sadly, we can't quite do this in our existing sed script. This works if you have GNU sed: s/^\\([><]\\) \\([A-Za-z0-9-]*:\\)/\1 \L\2\E/ but \L is a GNU-ism, and I don't think there's a portable solution. We could just "tr A-Z a-z" on the way in, of course, but that makes the non-header parts harder to read (e.g., lowercase "post" requests). But to paraphrase Baron Munchausen, I have learned from experience that a modicum of Perl can be most efficacious. Note that this doesn't quite get the test passing with t5559; there are more fixes needed on top. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:54:02 +01:00
> accept: */*
> accept-encoding: ENCODINGS
> accept-language: ko-KR, *;q=0.9
> pragma: no-cache
t5551: handle v2 protocol when checking curl trace After cloning an http repository, we check the curl trace to make sure the expected requests were made. But since the expected trace was never updated to handle v2, it is only run when you ask the test suite to run in v0 mode (which hardly anybody does). Let's update it to handle both protocols. This isn't too hard since v2 just sends an extra header and an extra request. So we can just annotate those extra lines and strip them out for v0 (and drop the annotations for v2). I didn't bother handling v1 here, as it's not really of practical interest (it would drop the extra v2 request, but still have the "git-protocol" lines). There's a similar tweak needed at the end. Since we check the "accept-encoding" value loosely, we grep for it rather than finding it in the verbatim trace. This grep insists that there are exactly 2 matches, but of course in v2 with the extra request there are 3. We could tweak the number, but it's simpler still to just check that we saw at least one match. The verbatim check already confirmed how many instances of the header we have; we're really just checking here that "gzip" is in the value (it's possible, of course, that the headers could have different values, but that seems like an unlikely bug). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:59:00 +01:00
{V2} > git-protocol: version=2
t5551: handle HTTP/2 when checking curl trace We check that the curl trace of a clone has the lines we expect, but this won't work when we run the test under t5559, because a few details are different under HTTP/2 (but nobody noticed because it only happens when you manually set GIT_TEST_PROTOCOL_VERSION to "0"). We can handle both HTTP protocols with a few tweaks: - we'll drop the HTTP "101 Switching Protocols" response, as well as various protocol upgrade headers. These details aren't interesting to us. We just want to make sure the correct protocol was used (and we do in the main request/response lines). - successful HTTP/2 responses just say "200" and not "200 OK"; we can normalize these - replace HTTP/1.1 with a variable in the request/response lines. We can use the existing $HTTP_PROTO for this, as it's already set to "HTTP/2" when appropriate. We do need to tweak the fallback value to "HTTP/1.1" to match what curl will write (prior to this patch, the fallback value didn't matter at all; we only checked if it was the literal string "HTTP/2"). Note that several lines still expect HTTP/1.1 unconditionally. The first request does so because the client requests an upgrade during the request. The POST request and response do so because you can't do an upgrade if there is a request body. (This will all be different if we trigger HTTP/2 via ALPN, but the tests aren't yet capable of that). This is enough to let: GIT_TEST_PROTOCOL_VERSION=0 ./t5559-http-fetch-smart-http2.sh pass the "clone http repository" test (but there are some other failures later on). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:56:17 +01:00
< $HTTP_PROTO 200 OK
t5551: lower-case headers in expected curl trace There's a test in t5551 which checks the curl trace (after simplifying it a bit). It doesn't work with HTTP/2, because in that case curl outputs all of the headers in lower-case. Even though this test is run with HTTP/2 by t5559, nobody has noticed because checking the trace only happens if GIT_TEST_PROTOCOL_VERSION is manually set to "0". Let's fix this by lower-casing all of the header names in the trace, and then checking for those in our expected code (this is easier than making HTTP/2 traces look like HTTP/1.1, since HTTP/1.1 uses title-casing). Sadly, we can't quite do this in our existing sed script. This works if you have GNU sed: s/^\\([><]\\) \\([A-Za-z0-9-]*:\\)/\1 \L\2\E/ but \L is a GNU-ism, and I don't think there's a portable solution. We could just "tr A-Z a-z" on the way in, of course, but that makes the non-header parts harder to read (e.g., lowercase "post" requests). But to paraphrase Baron Munchausen, I have learned from experience that a modicum of Perl can be most efficacious. Note that this doesn't quite get the test passing with t5559; there are more fixes needed on top. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:54:02 +01:00
< pragma: no-cache
< cache-control: no-cache, max-age=0, must-revalidate
< content-type: application/x-git-upload-pack-advertisement
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>
2023-02-23 12:07:29 +01:00
> POST /smart/repo.git/git-upload-pack $INITIAL_PROTO
t5551: lower-case headers in expected curl trace There's a test in t5551 which checks the curl trace (after simplifying it a bit). It doesn't work with HTTP/2, because in that case curl outputs all of the headers in lower-case. Even though this test is run with HTTP/2 by t5559, nobody has noticed because checking the trace only happens if GIT_TEST_PROTOCOL_VERSION is manually set to "0". Let's fix this by lower-casing all of the header names in the trace, and then checking for those in our expected code (this is easier than making HTTP/2 traces look like HTTP/1.1, since HTTP/1.1 uses title-casing). Sadly, we can't quite do this in our existing sed script. This works if you have GNU sed: s/^\\([><]\\) \\([A-Za-z0-9-]*:\\)/\1 \L\2\E/ but \L is a GNU-ism, and I don't think there's a portable solution. We could just "tr A-Z a-z" on the way in, of course, but that makes the non-header parts harder to read (e.g., lowercase "post" requests). But to paraphrase Baron Munchausen, I have learned from experience that a modicum of Perl can be most efficacious. Note that this doesn't quite get the test passing with t5559; there are more fixes needed on top. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:54:02 +01:00
> accept-encoding: ENCODINGS
> content-type: application/x-git-upload-pack-request
> accept: application/x-git-upload-pack-result
> accept-language: ko-KR, *;q=0.9
t5551: handle v2 protocol when checking curl trace After cloning an http repository, we check the curl trace to make sure the expected requests were made. But since the expected trace was never updated to handle v2, it is only run when you ask the test suite to run in v0 mode (which hardly anybody does). Let's update it to handle both protocols. This isn't too hard since v2 just sends an extra header and an extra request. So we can just annotate those extra lines and strip them out for v0 (and drop the annotations for v2). I didn't bother handling v1 here, as it's not really of practical interest (it would drop the extra v2 request, but still have the "git-protocol" lines). There's a similar tweak needed at the end. Since we check the "accept-encoding" value loosely, we grep for it rather than finding it in the verbatim trace. This grep insists that there are exactly 2 matches, but of course in v2 with the extra request there are 3. We could tweak the number, but it's simpler still to just check that we saw at least one match. The verbatim check already confirmed how many instances of the header we have; we're really just checking here that "gzip" is in the value (it's possible, of course, that the headers could have different values, but that seems like an unlikely bug). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:59:00 +01:00
{V2} > git-protocol: version=2
t5551: lower-case headers in expected curl trace There's a test in t5551 which checks the curl trace (after simplifying it a bit). It doesn't work with HTTP/2, because in that case curl outputs all of the headers in lower-case. Even though this test is run with HTTP/2 by t5559, nobody has noticed because checking the trace only happens if GIT_TEST_PROTOCOL_VERSION is manually set to "0". Let's fix this by lower-casing all of the header names in the trace, and then checking for those in our expected code (this is easier than making HTTP/2 traces look like HTTP/1.1, since HTTP/1.1 uses title-casing). Sadly, we can't quite do this in our existing sed script. This works if you have GNU sed: s/^\\([><]\\) \\([A-Za-z0-9-]*:\\)/\1 \L\2\E/ but \L is a GNU-ism, and I don't think there's a portable solution. We could just "tr A-Z a-z" on the way in, of course, but that makes the non-header parts harder to read (e.g., lowercase "post" requests). But to paraphrase Baron Munchausen, I have learned from experience that a modicum of Perl can be most efficacious. Note that this doesn't quite get the test passing with t5559; there are more fixes needed on top. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:54:02 +01:00
> content-length: xxx
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>
2023-02-23 12:07:29 +01:00
< $INITIAL_PROTO 200 OK
t5551: lower-case headers in expected curl trace There's a test in t5551 which checks the curl trace (after simplifying it a bit). It doesn't work with HTTP/2, because in that case curl outputs all of the headers in lower-case. Even though this test is run with HTTP/2 by t5559, nobody has noticed because checking the trace only happens if GIT_TEST_PROTOCOL_VERSION is manually set to "0". Let's fix this by lower-casing all of the header names in the trace, and then checking for those in our expected code (this is easier than making HTTP/2 traces look like HTTP/1.1, since HTTP/1.1 uses title-casing). Sadly, we can't quite do this in our existing sed script. This works if you have GNU sed: s/^\\([><]\\) \\([A-Za-z0-9-]*:\\)/\1 \L\2\E/ but \L is a GNU-ism, and I don't think there's a portable solution. We could just "tr A-Z a-z" on the way in, of course, but that makes the non-header parts harder to read (e.g., lowercase "post" requests). But to paraphrase Baron Munchausen, I have learned from experience that a modicum of Perl can be most efficacious. Note that this doesn't quite get the test passing with t5559; there are more fixes needed on top. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:54:02 +01:00
< pragma: no-cache
< cache-control: no-cache, max-age=0, must-revalidate
< content-type: application/x-git-upload-pack-result
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>
2023-02-23 12:07:29 +01:00
{V2} > POST /smart/repo.git/git-upload-pack $INITIAL_PROTO
t5551: handle v2 protocol when checking curl trace After cloning an http repository, we check the curl trace to make sure the expected requests were made. But since the expected trace was never updated to handle v2, it is only run when you ask the test suite to run in v0 mode (which hardly anybody does). Let's update it to handle both protocols. This isn't too hard since v2 just sends an extra header and an extra request. So we can just annotate those extra lines and strip them out for v0 (and drop the annotations for v2). I didn't bother handling v1 here, as it's not really of practical interest (it would drop the extra v2 request, but still have the "git-protocol" lines). There's a similar tweak needed at the end. Since we check the "accept-encoding" value loosely, we grep for it rather than finding it in the verbatim trace. This grep insists that there are exactly 2 matches, but of course in v2 with the extra request there are 3. We could tweak the number, but it's simpler still to just check that we saw at least one match. The verbatim check already confirmed how many instances of the header we have; we're really just checking here that "gzip" is in the value (it's possible, of course, that the headers could have different values, but that seems like an unlikely bug). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:59:00 +01:00
{V2} > accept-encoding: ENCODINGS
{V2} > content-type: application/x-git-upload-pack-request
{V2} > accept: application/x-git-upload-pack-result
{V2} > accept-language: ko-KR, *;q=0.9
{V2} > git-protocol: version=2
{V2} > content-length: xxx
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>
2023-02-23 12:07:29 +01:00
{V2} < $INITIAL_PROTO 200 OK
t5551: handle v2 protocol when checking curl trace After cloning an http repository, we check the curl trace to make sure the expected requests were made. But since the expected trace was never updated to handle v2, it is only run when you ask the test suite to run in v0 mode (which hardly anybody does). Let's update it to handle both protocols. This isn't too hard since v2 just sends an extra header and an extra request. So we can just annotate those extra lines and strip them out for v0 (and drop the annotations for v2). I didn't bother handling v1 here, as it's not really of practical interest (it would drop the extra v2 request, but still have the "git-protocol" lines). There's a similar tweak needed at the end. Since we check the "accept-encoding" value loosely, we grep for it rather than finding it in the verbatim trace. This grep insists that there are exactly 2 matches, but of course in v2 with the extra request there are 3. We could tweak the number, but it's simpler still to just check that we saw at least one match. The verbatim check already confirmed how many instances of the header we have; we're really just checking here that "gzip" is in the value (it's possible, of course, that the headers could have different values, but that seems like an unlikely bug). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:59:00 +01:00
{V2} < pragma: no-cache
{V2} < cache-control: no-cache, max-age=0, must-revalidate
{V2} < content-type: application/x-git-upload-pack-result
t5551: move setup code inside test_expect blocks Move setup code inside test_expect blocks, to catch unexpected failures in the setup steps, and bring the test scripts in line with our modern test style. Suggested-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-17 23:46:27 +02:00
EOF
remote-curl: send Accept-Language header to server Git server end's ability to accept Accept-Language header was introduced in f18604bbf2 (http: add Accept-Language header if possible, 2015-01-28), but this is only used by very early phase of the transfer, which is HTTP GET request to discover references. For other phases, like POST request in the smart HTTP, the server does not know what language the client speaks. Teach git client to learn end-user's preferred language and throw accept-language header to the server side. Once the server gets this header, it has the ability to talk to end-user with language they understand. This would be very helpful for many non-English speakers. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Li Linchao <lilinchao@oschina.cn> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-11 07:58:54 +02:00
t5551: handle v2 protocol when checking curl trace After cloning an http repository, we check the curl trace to make sure the expected requests were made. But since the expected trace was never updated to handle v2, it is only run when you ask the test suite to run in v0 mode (which hardly anybody does). Let's update it to handle both protocols. This isn't too hard since v2 just sends an extra header and an extra request. So we can just annotate those extra lines and strip them out for v0 (and drop the annotations for v2). I didn't bother handling v1 here, as it's not really of practical interest (it would drop the extra v2 request, but still have the "git-protocol" lines). There's a similar tweak needed at the end. Since we check the "accept-encoding" value loosely, we grep for it rather than finding it in the verbatim trace. This grep insists that there are exactly 2 matches, but of course in v2 with the extra request there are 3. We could tweak the number, but it's simpler still to just check that we saw at least one match. The verbatim check already confirmed how many instances of the header we have; we're really just checking here that "gzip" is in the value (it's possible, of course, that the headers could have different values, but that seems like an unlikely bug). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:59:00 +01:00
if test "$GIT_TEST_PROTOCOL_VERSION" = 0
then
sed "/^{V2}/d" <exp.raw >exp
else
sed "s/^{V2} //" <exp.raw >exp
fi &&
t5551: stop forcing clone to run with v0 protocol In the "clone http repository" test, we check the curl trace to make sure the expected requests were made. This whole script was marked to handle only the v0 protocol in d790ee1707 (tests: fix protocol version for overspecifications, 2019-02-25). That makes sense, since v2 requires an extra request, so tests as specific as this would fail unless modified. Later, in preparation for v2 becoming the default, this was tweaked by 8a1b0978ab (test: request GIT_TEST_PROTOCOL_VERSION=0 when appropriate, 2019-12-23). There we run the trace check only if the user has explicitly asked to test protocol version 0. But it also forced the clone itself to run with the v0 protocol. This makes the check for "can we expect a v0 trace" silly; it will always be v0. But much worse, it means that the clone we are testing is not like the one that normal users would run. They would use the defaults, which are now v2. And since this is supposed to be a basic check of clone-over-http, we should do the same. Let's fix this by dropping the extra v0 override. The test still passes because the trace checking only kicks in if we asked to use v0 explicitly (this is the same as before; even though we were running a v0 clone, unless you specifically set GIT_TEST_PROTOCOL_VERSION=0, the trace check was always skipped). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:57:27 +01:00
GIT_TRACE_CURL=true LANGUAGE="ko_KR.UTF-8" \
tests: define GIT_TEST_PROTOCOL_VERSION Define a GIT_TEST_PROTOCOL_VERSION environment variable meant to be used from tests. When set, this ensures protocol.version is at least the given value, allowing the entire test suite to be run as if this configuration is in place for all repositories. As of this patch, all tests pass whether GIT_TEST_PROTOCOL_VERSION is unset or set to 0. Some tests fail when GIT_TEST_PROTOCOL_VERSION is set to 1 or 2, but this will be dealt with in subsequent patches. This is based on work by Ævar Arnfjörð Bjarmason. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-25 22:54:06 +01:00
git clone --quiet $HTTPD_URL/smart/repo.git clone 2>err &&
test smart http fetch and push The top level directory "/smart/" of the test Apache server is mapped through our git-http-backend CGI, but uses the same underlying repository space as the server's document root. This is the most simple installation possible. Server logs are checked to verify the client has accessed only the smart URLs during the test. During fetch testing the headers are also logged from libcurl to ensure we are making a reasonably sane HTTP request, and getting back reasonably sane response headers from the CGI. When validating the request headers used during smart fetch we munge away the actual Content-Length and replace it with the placeholder "xxx". This avoids unnecessary varability in the test caused by an unrelated change in the requested capabilities in the first want line of the request. However, we still want to look for and verify that Content-Length was used, because smaller payloads should be using Content-Length and not "Transfer-Encoding: chunked". When validating the server response headers we must discard both Content-Length and Transfer-Encoding, as Apache2 can use either format to return our response. During development of this test I observed Apache returning both forms, depending on when the processes got CPU time. If our CGI returned the pack data quickly, Apache just buffered the whole thing and returned a Content-Length. If our CGI took just a bit too long to complete, Apache flushed its buffer and instead used "Transfer-Encoding: chunked". Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-31 01:47:47 +01:00
test_cmp file clone/file &&
tr '\''\015'\'' Q <err |
t5551: lower-case headers in expected curl trace There's a test in t5551 which checks the curl trace (after simplifying it a bit). It doesn't work with HTTP/2, because in that case curl outputs all of the headers in lower-case. Even though this test is run with HTTP/2 by t5559, nobody has noticed because checking the trace only happens if GIT_TEST_PROTOCOL_VERSION is manually set to "0". Let's fix this by lower-casing all of the header names in the trace, and then checking for those in our expected code (this is easier than making HTTP/2 traces look like HTTP/1.1, since HTTP/1.1 uses title-casing). Sadly, we can't quite do this in our existing sed script. This works if you have GNU sed: s/^\\([><]\\) \\([A-Za-z0-9-]*:\\)/\1 \L\2\E/ but \L is a GNU-ism, and I don't think there's a portable solution. We could just "tr A-Z a-z" on the way in, of course, but that makes the non-header parts harder to read (e.g., lowercase "post" requests). But to paraphrase Baron Munchausen, I have learned from experience that a modicum of Perl can be most efficacious. Note that this doesn't quite get the test passing with t5559; there are more fixes needed on top. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:54:02 +01:00
perl -pe '\''
s/(Send|Recv) header: ([A-Za-z0-9-]+):/
"$1 header: " . lc($2) . ":"
/e;
'\'' |
test smart http fetch and push The top level directory "/smart/" of the test Apache server is mapped through our git-http-backend CGI, but uses the same underlying repository space as the server's document root. This is the most simple installation possible. Server logs are checked to verify the client has accessed only the smart URLs during the test. During fetch testing the headers are also logged from libcurl to ensure we are making a reasonably sane HTTP request, and getting back reasonably sane response headers from the CGI. When validating the request headers used during smart fetch we munge away the actual Content-Length and replace it with the placeholder "xxx". This avoids unnecessary varability in the test caused by an unrelated change in the requested capabilities in the first want line of the request. However, we still want to look for and verify that Content-Length was used, because smaller payloads should be using Content-Length and not "Transfer-Encoding: chunked". When validating the server response headers we must discard both Content-Length and Transfer-Encoding, as Apache2 can use either format to return our response. During development of this test I observed Apache returning both forms, depending on when the processes got CPU time. If our CGI returned the pack data quickly, Apache just buffered the whole thing and returned a Content-Length. If our CGI took just a bit too long to complete, Apache flushed its buffer and instead used "Transfer-Encoding: chunked". Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-31 01:47:47 +01:00
sed -e "
s/Q\$//
t5551: drop curl trace lines without headers We pick apart a curl trace, looking for "=> Send header:" and so on, and matching against an expected set of requests and responses. We remove "== Info" lines entirely. However, our parser is fooled when running the test with LIB_HTTPD_SSL on Ubuntu 20.04 (as found in our linux-gcc CI job), as curl hands us an "Info" buffer with a newline, and we get: == Info: successfully set certificate verify locations: == Info: CAfile: /etc/ssl/certs/ca-certificates.crt CApath: /etc/ssl/certs => Send SSL data[...] which results in the "CApath" line ending up in the cleaned-up output, causing the test to fail. Arguably the tracing code should detect this and put it on two separate "== Info" lines. But this is actually a curl bug, fixed by their 80d73bcca (tls: provide the CApath verbose log on its own line, 2020-08-18). It's simpler to just work around it here. Since we are using GIT_TRACE_CURL, every line should just start with one of "<=", "==", or "=>", and we can throw away anything else. In fact, we can just replace the pattern for deleting "*" lines. Those were from the old GIT_CURL_VERBOSE output, but we switched over in 14e24114d9 (t5551-http-fetch-smart.sh: use the GIT_TRACE_CURL environment var, 2016-09-05). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 12:05:07 +01:00
/^[^<=]/d
t5551-http-fetch-smart.sh: use the GIT_TRACE_CURL environment var Use the new GIT_TRACE_CURL environment variable instead of the deprecated GIT_CURL_VERBOSE. Signed-off-by: Elia Pinto <gitter.spiros@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-05 12:24:44 +02:00
/^== Info:/d
/^=> Send header, /d
/^=> Send header:$/d
/^<= Recv header, /d
/^<= Recv header:$/d
s/=> Send header: //
s/= Recv header://
/^<= Recv data/d
/^=> Send data/d
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>
2023-02-23 12:07:29 +01:00
/^<= Recv SSL data/d
/^=> Send SSL data/d
t5551-http-fetch: Work around some libcurl versions Some versions of libcurl report their output when GIT_CURL_VERBOSE is set differently than other versions do. At least one variant (version unknown but likely pre-7.18.1) reports the POST payload to stderr, and omits the blank line after each HTTP request/response. We clip these lines out of the stderr output now before doing the compare, so we aren't surprised by this trivial difference. Reported-by: Tarmigan <tarmigan+git@gmail.com> Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-11-09 19:10:36 +01:00
/^$/d
/^< $/d
test smart http fetch and push The top level directory "/smart/" of the test Apache server is mapped through our git-http-backend CGI, but uses the same underlying repository space as the server's document root. This is the most simple installation possible. Server logs are checked to verify the client has accessed only the smart URLs during the test. During fetch testing the headers are also logged from libcurl to ensure we are making a reasonably sane HTTP request, and getting back reasonably sane response headers from the CGI. When validating the request headers used during smart fetch we munge away the actual Content-Length and replace it with the placeholder "xxx". This avoids unnecessary varability in the test caused by an unrelated change in the requested capabilities in the first want line of the request. However, we still want to look for and verify that Content-Length was used, because smaller payloads should be using Content-Length and not "Transfer-Encoding: chunked". When validating the server response headers we must discard both Content-Length and Transfer-Encoding, as Apache2 can use either format to return our response. During development of this test I observed Apache returning both forms, depending on when the processes got CPU time. If our CGI returned the pack data quickly, Apache just buffered the whole thing and returned a Content-Length. If our CGI took just a bit too long to complete, Apache flushed its buffer and instead used "Transfer-Encoding: chunked". Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-31 01:47:47 +01:00
/^[^><]/{
s/^/> /
}
t5551: handle HTTP/2 when checking curl trace We check that the curl trace of a clone has the lines we expect, but this won't work when we run the test under t5559, because a few details are different under HTTP/2 (but nobody noticed because it only happens when you manually set GIT_TEST_PROTOCOL_VERSION to "0"). We can handle both HTTP protocols with a few tweaks: - we'll drop the HTTP "101 Switching Protocols" response, as well as various protocol upgrade headers. These details aren't interesting to us. We just want to make sure the correct protocol was used (and we do in the main request/response lines). - successful HTTP/2 responses just say "200" and not "200 OK"; we can normalize these - replace HTTP/1.1 with a variable in the request/response lines. We can use the existing $HTTP_PROTO for this, as it's already set to "HTTP/2" when appropriate. We do need to tweak the fallback value to "HTTP/1.1" to match what curl will write (prior to this patch, the fallback value didn't matter at all; we only checked if it was the literal string "HTTP/2"). Note that several lines still expect HTTP/1.1 unconditionally. The first request does so because the client requests an upgrade during the request. The POST request and response do so because you can't do an upgrade if there is a request body. (This will all be different if we trigger HTTP/2 via ALPN, but the tests aren't yet capable of that). This is enough to let: GIT_TEST_PROTOCOL_VERSION=0 ./t5559-http-fetch-smart-http2.sh pass the "clone http repository" test (but there are some other failures later on). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:56:17 +01:00
/^< HTTP/ {
s/200$/200 OK/
}
/^< HTTP\\/1.1 101/d
/^[><] connection: /d
/^[><] upgrade: /d
/^> http2-settings: /d
t5551: lower-case headers in expected curl trace There's a test in t5551 which checks the curl trace (after simplifying it a bit). It doesn't work with HTTP/2, because in that case curl outputs all of the headers in lower-case. Even though this test is run with HTTP/2 by t5559, nobody has noticed because checking the trace only happens if GIT_TEST_PROTOCOL_VERSION is manually set to "0". Let's fix this by lower-casing all of the header names in the trace, and then checking for those in our expected code (this is easier than making HTTP/2 traces look like HTTP/1.1, since HTTP/1.1 uses title-casing). Sadly, we can't quite do this in our existing sed script. This works if you have GNU sed: s/^\\([><]\\) \\([A-Za-z0-9-]*:\\)/\1 \L\2\E/ but \L is a GNU-ism, and I don't think there's a portable solution. We could just "tr A-Z a-z" on the way in, of course, but that makes the non-header parts harder to read (e.g., lowercase "post" requests). But to paraphrase Baron Munchausen, I have learned from experience that a modicum of Perl can be most efficacious. Note that this doesn't quite get the test passing with t5559; there are more fixes needed on top. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:54:02 +01:00
/^> user-agent: /d
/^> host: /d
t5551-http-fetch: Work around broken Accept header in libcurl Unfortunately at least one version of libcurl has a bug causing it to include "Accept: */*" in the same POST request where we have already asked for "Accept: application/x-git-upload-pack-response". This is a bug in libcurl, not Git, or our test vector. The application has explicitly asked the server for a single content type, but libcurl has mistakenly also told the server the client application will accept */*, which is any content type. Based on the libcurl change log, this "Accept: */*" header bug may have been fixed in version 7.18.1 released March 30, 2008: http://curl.haxx.se/changes.html#7_18_1 Rather than require users to upgrade libcurl we change the test vector to trim this line out of the 2nd request. Reported-by: Tarmigan <tarmigan+git@gmail.com> Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-11-09 19:10:37 +01:00
/^> POST /,$ {
/^> Accept: [*]\\/[*]/d
}
t5551: lower-case headers in expected curl trace There's a test in t5551 which checks the curl trace (after simplifying it a bit). It doesn't work with HTTP/2, because in that case curl outputs all of the headers in lower-case. Even though this test is run with HTTP/2 by t5559, nobody has noticed because checking the trace only happens if GIT_TEST_PROTOCOL_VERSION is manually set to "0". Let's fix this by lower-casing all of the header names in the trace, and then checking for those in our expected code (this is easier than making HTTP/2 traces look like HTTP/1.1, since HTTP/1.1 uses title-casing). Sadly, we can't quite do this in our existing sed script. This works if you have GNU sed: s/^\\([><]\\) \\([A-Za-z0-9-]*:\\)/\1 \L\2\E/ but \L is a GNU-ism, and I don't think there's a portable solution. We could just "tr A-Z a-z" on the way in, of course, but that makes the non-header parts harder to read (e.g., lowercase "post" requests). But to paraphrase Baron Munchausen, I have learned from experience that a modicum of Perl can be most efficacious. Note that this doesn't quite get the test passing with t5559; there are more fixes needed on top. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:54:02 +01:00
s/^> content-length: .*/> content-length: xxx/
t5551-http-fetch: Work around some libcurl versions Some versions of libcurl report their output when GIT_CURL_VERBOSE is set differently than other versions do. At least one variant (version unknown but likely pre-7.18.1) reports the POST payload to stderr, and omits the blank line after each HTTP request/response. We clip these lines out of the stderr output now before doing the compare, so we aren't surprised by this trivial difference. Reported-by: Tarmigan <tarmigan+git@gmail.com> Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-11-09 19:10:36 +01:00
/^> 00..want /d
/^> 00.*done/d
test smart http fetch and push The top level directory "/smart/" of the test Apache server is mapped through our git-http-backend CGI, but uses the same underlying repository space as the server's document root. This is the most simple installation possible. Server logs are checked to verify the client has accessed only the smart URLs during the test. During fetch testing the headers are also logged from libcurl to ensure we are making a reasonably sane HTTP request, and getting back reasonably sane response headers from the CGI. When validating the request headers used during smart fetch we munge away the actual Content-Length and replace it with the placeholder "xxx". This avoids unnecessary varability in the test caused by an unrelated change in the requested capabilities in the first want line of the request. However, we still want to look for and verify that Content-Length was used, because smaller payloads should be using Content-Length and not "Transfer-Encoding: chunked". When validating the server response headers we must discard both Content-Length and Transfer-Encoding, as Apache2 can use either format to return our response. During development of this test I observed Apache returning both forms, depending on when the processes got CPU time. If our CGI returned the pack data quickly, Apache just buffered the whole thing and returned a Content-Length. If our CGI took just a bit too long to complete, Apache flushed its buffer and instead used "Transfer-Encoding: chunked". Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-31 01:47:47 +01:00
t5551: lower-case headers in expected curl trace There's a test in t5551 which checks the curl trace (after simplifying it a bit). It doesn't work with HTTP/2, because in that case curl outputs all of the headers in lower-case. Even though this test is run with HTTP/2 by t5559, nobody has noticed because checking the trace only happens if GIT_TEST_PROTOCOL_VERSION is manually set to "0". Let's fix this by lower-casing all of the header names in the trace, and then checking for those in our expected code (this is easier than making HTTP/2 traces look like HTTP/1.1, since HTTP/1.1 uses title-casing). Sadly, we can't quite do this in our existing sed script. This works if you have GNU sed: s/^\\([><]\\) \\([A-Za-z0-9-]*:\\)/\1 \L\2\E/ but \L is a GNU-ism, and I don't think there's a portable solution. We could just "tr A-Z a-z" on the way in, of course, but that makes the non-header parts harder to read (e.g., lowercase "post" requests). But to paraphrase Baron Munchausen, I have learned from experience that a modicum of Perl can be most efficacious. Note that this doesn't quite get the test passing with t5559; there are more fixes needed on top. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:54:02 +01:00
/^< server: /d
/^< expires: /d
/^< date: /d
/^< content-length: /d
/^< transfer-encoding: /d
remote-curl: accept all encodings supported by curl Configure curl to accept all encodings which curl supports instead of only accepting gzip responses. This fixes an issue when using an installation of curl which is built without the "zlib" feature. Since aa90b9697 (Enable info/refs gzip decompression in HTTP client, 2012-09-19) we end up requesting "gzip" encoding anyway despite libcurl not being able to decode it. Worse, instead of getting a clear error message indicating so, we end up falling back to "dumb" http, producing a confusing and difficult to debug result. Since curl doesn't do any checking to verify that it supports the a requested encoding, instead set the curl option `CURLOPT_ENCODING` with an empty string indicating that curl should send an "Accept-Encoding" header containing only the encodings supported by curl. Reported-by: Anton Golubev <anton.golubev@gmail.com> Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-22 20:42:03 +02:00
" >actual &&
t5551: handle v2 protocol when checking curl trace After cloning an http repository, we check the curl trace to make sure the expected requests were made. But since the expected trace was never updated to handle v2, it is only run when you ask the test suite to run in v0 mode (which hardly anybody does). Let's update it to handle both protocols. This isn't too hard since v2 just sends an extra header and an extra request. So we can just annotate those extra lines and strip them out for v0 (and drop the annotations for v2). I didn't bother handling v1 here, as it's not really of practical interest (it would drop the extra v2 request, but still have the "git-protocol" lines). There's a similar tweak needed at the end. Since we check the "accept-encoding" value loosely, we grep for it rather than finding it in the verbatim trace. This grep insists that there are exactly 2 matches, but of course in v2 with the extra request there are 3. We could tweak the number, but it's simpler still to just check that we saw at least one match. The verbatim check already confirmed how many instances of the header we have; we're really just checking here that "gzip" is in the value (it's possible, of course, that the headers could have different values, but that seems like an unlikely bug). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:59:00 +01:00
sed -e "s/^> accept-encoding: .*/> accept-encoding: ENCODINGS/" \
actual >actual.smudged &&
test_cmp exp actual.smudged &&
tests: fix protocol version for overspecifications These tests are also marked with a NEEDSWORK comment. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-25 22:54:12 +01:00
t5551: handle v2 protocol when checking curl trace After cloning an http repository, we check the curl trace to make sure the expected requests were made. But since the expected trace was never updated to handle v2, it is only run when you ask the test suite to run in v0 mode (which hardly anybody does). Let's update it to handle both protocols. This isn't too hard since v2 just sends an extra header and an extra request. So we can just annotate those extra lines and strip them out for v0 (and drop the annotations for v2). I didn't bother handling v1 here, as it's not really of practical interest (it would drop the extra v2 request, but still have the "git-protocol" lines). There's a similar tweak needed at the end. Since we check the "accept-encoding" value loosely, we grep for it rather than finding it in the verbatim trace. This grep insists that there are exactly 2 matches, but of course in v2 with the extra request there are 3. We could tweak the number, but it's simpler still to just check that we saw at least one match. The verbatim check already confirmed how many instances of the header we have; we're really just checking here that "gzip" is in the value (it's possible, of course, that the headers could have different values, but that seems like an unlikely bug). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:59:00 +01:00
grep "accept-encoding:.*gzip" actual >actual.gzip
test smart http fetch and push The top level directory "/smart/" of the test Apache server is mapped through our git-http-backend CGI, but uses the same underlying repository space as the server's document root. This is the most simple installation possible. Server logs are checked to verify the client has accessed only the smart URLs during the test. During fetch testing the headers are also logged from libcurl to ensure we are making a reasonably sane HTTP request, and getting back reasonably sane response headers from the CGI. When validating the request headers used during smart fetch we munge away the actual Content-Length and replace it with the placeholder "xxx". This avoids unnecessary varability in the test caused by an unrelated change in the requested capabilities in the first want line of the request. However, we still want to look for and verify that Content-Length was used, because smaller payloads should be using Content-Length and not "Transfer-Encoding: chunked". When validating the server response headers we must discard both Content-Length and Transfer-Encoding, as Apache2 can use either format to return our response. During development of this test I observed Apache returning both forms, depending on when the processes got CPU time. If our CGI returned the pack data quickly, Apache just buffered the whole thing and returned a Content-Length. If our CGI took just a bit too long to complete, Apache flushed its buffer and instead used "Transfer-Encoding: chunked". Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-31 01:47:47 +01:00
'
test_expect_success 'fetch changes via http' '
echo content >>file &&
git commit -a -m two &&
t: fix trivial &&-chain breakage These are tests which are missing a link in their &&-chain, but during a setup phase. We may fail to notice failure in commands that build the test environment, but these are typically not expected to fail at all (but it's still good to double-check that our test environment is what we expect). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-20 11:07:15 +01:00
git push public &&
test smart http fetch and push The top level directory "/smart/" of the test Apache server is mapped through our git-http-backend CGI, but uses the same underlying repository space as the server's document root. This is the most simple installation possible. Server logs are checked to verify the client has accessed only the smart URLs during the test. During fetch testing the headers are also logged from libcurl to ensure we are making a reasonably sane HTTP request, and getting back reasonably sane response headers from the CGI. When validating the request headers used during smart fetch we munge away the actual Content-Length and replace it with the placeholder "xxx". This avoids unnecessary varability in the test caused by an unrelated change in the requested capabilities in the first want line of the request. However, we still want to look for and verify that Content-Length was used, because smaller payloads should be using Content-Length and not "Transfer-Encoding: chunked". When validating the server response headers we must discard both Content-Length and Transfer-Encoding, as Apache2 can use either format to return our response. During development of this test I observed Apache returning both forms, depending on when the processes got CPU time. If our CGI returned the pack data quickly, Apache just buffered the whole thing and returned a Content-Length. If our CGI took just a bit too long to complete, Apache flushed its buffer and instead used "Transfer-Encoding: chunked". Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-31 01:47:47 +01:00
(cd clone && git pull) &&
test_cmp file clone/file
'
test_expect_success 'used upload-pack service' '
t5551: handle v2 protocol in upload-pack service test We perform a clone and a fetch, and then check that we saw the expected requests in Apache's access log. In the v2 protocol, there will be one extra request to /git-upload-pack for each operation (since the initial /info/refs probe is just used to upgrade the protocol). As a result, this test is a noop unless the use of the v0 protocol is forced. Which means that hardly anybody runs it, since you have to do so manually. Let's update it to handle v2 and run it always. We could do this by just conditionally adding in the extra POST lines. But if we look at the origin of the test in 7da4e2280c (test smart http fetch and push, 2009-10-30), the point is really just to make sure that the smart git-upload-pack service was used at all. So rather than counting up the individual requests, let's just make sure we saw each of the expected types. This is a bit looser, but makes maintenance easier. Since we're now matching with grep, we can also loosen the HTTP/1.1 match, which allows this test to pass when run with HTTP/2 via t5559. That lets: GIT_TEST_PROTOCOL_VERSION=0 ./t5559-http-fetch-smart-http2.sh run to completion, which previously failed (and of course it works if you use v2, as well). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 12:00:38 +01:00
strip_access_log >log &&
grep "GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/[0-9.]* 200" log &&
grep "POST /smart/repo.git/git-upload-pack HTTP/[0-9.]* 200" log
test smart http fetch and push The top level directory "/smart/" of the test Apache server is mapped through our git-http-backend CGI, but uses the same underlying repository space as the server's document root. This is the most simple installation possible. Server logs are checked to verify the client has accessed only the smart URLs during the test. During fetch testing the headers are also logged from libcurl to ensure we are making a reasonably sane HTTP request, and getting back reasonably sane response headers from the CGI. When validating the request headers used during smart fetch we munge away the actual Content-Length and replace it with the placeholder "xxx". This avoids unnecessary varability in the test caused by an unrelated change in the requested capabilities in the first want line of the request. However, we still want to look for and verify that Content-Length was used, because smaller payloads should be using Content-Length and not "Transfer-Encoding: chunked". When validating the server response headers we must discard both Content-Length and Transfer-Encoding, as Apache2 can use either format to return our response. During development of this test I observed Apache returning both forms, depending on when the processes got CPU time. If our CGI returned the pack data quickly, Apache just buffered the whole thing and returned a Content-Length. If our CGI took just a bit too long to complete, Apache flushed its buffer and instead used "Transfer-Encoding: chunked". Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-31 01:47:47 +01:00
'
smart-http: Don't change POST to GET when following redirect For a long time (29508e1 "Isolate shared HTTP request functionality", Fri Nov 18 11:02:58 2005), we've followed HTTP redirects with CURLOPT_FOLLOWLOCATION. However, when the remote HTTP server returns a redirect the default libcurl action is to change a POST request into a GET request while following the redirect, but the remote http backend does not expect that. Fix this by telling libcurl to always keep the request as type POST with CURLOPT_POSTREDIR. For users of libcurl older than 7.19.1, use CURLOPT_POST301 instead, which only follows 301s instead of both 301s and 302s. Signed-off-by: Andreas Schwab <schwab@linux-m68k.org> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-09-25 06:20:35 +02:00
test_expect_success 'follow redirects (301)' '
git clone $HTTPD_URL/smart-redir-perm/repo.git --quiet repo-p
'
test_expect_success 'follow redirects (302)' '
git clone $HTTPD_URL/smart-redir-temp/repo.git --quiet repo-t
'
remote-curl: rewrite base url from info/refs redirects For efficiency and security reasons, an earlier commit in this series taught http_get_* to re-write the base url based on redirections we saw while making a specific request. This commit wires that option into the info/refs request, meaning that a redirect from http://example.com/foo.git/info/refs to https://example.com/bar.git/info/refs will behave as if "https://example.com/bar.git" had been provided to git in the first place. The tests bear some explanation. We introduce two new hierearchies into the httpd test config: 1. Requests to /smart-redir-limited will work only for the initial info/refs request, but not any subsequent requests. As a result, we can confirm whether the client is re-rooting its requests after the initial contact, since otherwise it will fail (it will ask for "repo.git/git-upload-pack", which is not redirected). 2. Requests to smart-redir-auth will redirect, and require auth after the redirection. Since we are using the redirected base for further requests, we also update the credential struct, in order not to mislead the user (or credential helpers) about which credential is needed. We can therefore check the GIT_ASKPASS prompts to make sure we are prompting for the new location. Because we have neither multiple servers nor https support in our test setup, we can only redirect between paths, meaning we need to turn on credential.useHttpPath to see the difference. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2013-09-28 10:35:35 +02:00
test_expect_success 'redirects re-root further requests' '
git clone $HTTPD_URL/smart-redir-limited/repo.git repo-redir-limited
'
http: always update the base URL for redirects If a malicious server redirects the initial ref advertisement, it may be able to leak sha1s from other, unrelated servers that the client has access to. For example, imagine that Alice is a git user, she has access to a private repository on a server hosted by Bob, and Mallory runs a malicious server and wants to find out about Bob's private repository. Mallory asks Alice to clone an unrelated repository from her over HTTP. When Alice's client contacts Mallory's server for the initial ref advertisement, the server issues an HTTP redirect for Bob's server. Alice contacts Bob's server and gets the ref advertisement for the private repository. If there is anything to fetch, she then follows up by asking the server for one or more sha1 objects. But who is the server? If it is still Mallory's server, then Alice will leak the existence of those sha1s to her. Since commit c93c92f30 (http: update base URLs when we see redirects, 2013-09-28), the client usually rewrites the base URL such that all further requests will go to Bob's server. But this is done by textually matching the URL. If we were originally looking for "http://mallory/repo.git/info/refs", and we got pointed at "http://bob/other.git/info/refs", then we know that the right root is "http://bob/other.git". If the redirect appears to change more than just the root, we punt and continue to use the original server. E.g., imagine the redirect adds a URL component that Bob's server will ignore, like "http://bob/other.git/info/refs?dummy=1". We can solve this by aborting in this case rather than silently continuing to use Mallory's server. In addition to protecting from sha1 leakage, it's arguably safer and more sane to refuse a confusing redirect like that in general. For example, part of the motivation in c93c92f30 is avoiding accidentally sending credentials over clear http, just to get a response that says "try again over https". So even in a non-malicious case, we'd prefer to err on the side of caution. The downside is that it's possible this will break a legitimate but complicated server-side redirection scheme. The setup given in the newly added test does work, but it's convoluted enough that we don't need to care about it. A more plausible case would be a server which redirects a request for "info/refs?service=git-upload-pack" to just "info/refs" (because it does not do smart HTTP, and for some reason really dislikes query parameters). Right now we would transparently downgrade to dumb-http, but with this patch, we'd complain (and the user would have to set GIT_SMART_HTTP=0 to fetch). Reported-by: Jann Horn <jannh@google.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-06 19:24:35 +01:00
test_expect_success 're-rooting dies on insane schemes' '
test_must_fail git clone $HTTPD_URL/insane-redir/repo.git insane
'
t: test basic smart-http authentication We do not currently test authentication over smart-http at all. In theory, it should work exactly as it does for dumb http (which we do test). It does indeed work for these simple tests, but this patch lays the groundwork for more complex tests in future patches. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-08-27 15:25:36 +02:00
test_expect_success 'clone from password-protected repository' '
echo two >expect &&
use distinct username/password for http auth tests The httpd server we set up to test git's http client code knows about a single account, in which both the username and password are "user@host" (the unusual use of the "@" here is to verify that we handle the character correctly when URL escaped). This means that we may miss a certain class of errors in which the username and password are mixed up internally by git. We can make our tests more robust by having distinct values for the username and password. In addition to tweaking the server passwd file and the client URL, we must teach the "askpass" harness to accept multiple values. As a bonus, this makes the setup of some tests more obvious; when we are expecting git to ask only about the password, we can seed the username askpass response with a bogus value. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-01-02 08:38:35 +01:00
set_askpass user@host pass@host &&
t: test basic smart-http authentication We do not currently test authentication over smart-http at all. In theory, it should work exactly as it does for dumb http (which we do test). It does indeed work for these simple tests, but this patch lays the groundwork for more complex tests in future patches. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-08-27 15:25:36 +02:00
git clone --bare "$HTTPD_URL/auth/smart/repo.git" smart-auth &&
expect_askpass both user@host &&
git --git-dir=smart-auth log -1 --format=%s >actual &&
test_cmp expect actual
'
t: test http access to "half-auth" repositories Some sites set up http access to repositories such that fetching is anonymous and unauthenticated, but pushing is authenticated. While there are multiple ways to do this, the technique advertised in the git-http-backend manpage is to block access to locations matching "/git-receive-pack$". Let's emulate that advice in our test setup, which makes it clear that this advice does not actually work. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-08-27 15:25:53 +02:00
test_expect_success 'clone from auth-only-for-push repository' '
echo two >expect &&
set_askpass wrong &&
git clone --bare "$HTTPD_URL/auth-push/smart/repo.git" smart-noauth &&
expect_askpass none &&
git --git-dir=smart-noauth log -1 --format=%s >actual &&
test_cmp expect actual
'
remote-curl: retry failed requests for auth even with gzip Commit b81401c taught the post_rpc function to retry the http request after prompting for credentials. However, it did not handle two cases: 1. If we have a large request, we do not retry. That's OK, since we would have sent a probe (with retry) already. 2. If we are gzipping the request, we do not retry. That was considered OK, because the intended use was for push (e.g., listing refs is OK, but actually pushing objects is not), and we never gzip on push. This patch teaches post_rpc to retry even a gzipped request. This has two advantages: 1. It is possible to configure a "half-auth" state for fetching, where the set of refs and their sha1s are advertised, but one cannot actually fetch objects. This is not a recommended configuration, as it leaks some information about what is in the repository (e.g., an attacker can try brute-forcing possible content in your repository and checking whether it matches your branch sha1). However, it can be slightly more convenient, since a no-op fetch will not require a password at all. 2. It future-proofs us should we decide to ever gzip more requests. Signed-off-by: Jeff King <peff@peff.net>
2012-10-31 12:29:16 +01:00
test_expect_success 'clone from auth-only-for-objects repository' '
echo two >expect &&
use distinct username/password for http auth tests The httpd server we set up to test git's http client code knows about a single account, in which both the username and password are "user@host" (the unusual use of the "@" here is to verify that we handle the character correctly when URL escaped). This means that we may miss a certain class of errors in which the username and password are mixed up internally by git. We can make our tests more robust by having distinct values for the username and password. In addition to tweaking the server passwd file and the client URL, we must teach the "askpass" harness to accept multiple values. As a bonus, this makes the setup of some tests more obvious; when we are expecting git to ask only about the password, we can seed the username askpass response with a bogus value. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-01-02 08:38:35 +01:00
set_askpass user@host pass@host &&
remote-curl: retry failed requests for auth even with gzip Commit b81401c taught the post_rpc function to retry the http request after prompting for credentials. However, it did not handle two cases: 1. If we have a large request, we do not retry. That's OK, since we would have sent a probe (with retry) already. 2. If we are gzipping the request, we do not retry. That was considered OK, because the intended use was for push (e.g., listing refs is OK, but actually pushing objects is not), and we never gzip on push. This patch teaches post_rpc to retry even a gzipped request. This has two advantages: 1. It is possible to configure a "half-auth" state for fetching, where the set of refs and their sha1s are advertised, but one cannot actually fetch objects. This is not a recommended configuration, as it leaks some information about what is in the repository (e.g., an attacker can try brute-forcing possible content in your repository and checking whether it matches your branch sha1). However, it can be slightly more convenient, since a no-op fetch will not require a password at all. 2. It future-proofs us should we decide to ever gzip more requests. Signed-off-by: Jeff King <peff@peff.net>
2012-10-31 12:29:16 +01:00
git clone --bare "$HTTPD_URL/auth-fetch/smart/repo.git" half-auth &&
expect_askpass both user@host &&
git --git-dir=half-auth log -1 --format=%s >actual &&
test_cmp expect actual
'
test_expect_success 'no-op half-auth fetch does not require a password' '
set_askpass wrong &&
t5551: mark half-auth no-op fetch test as v0-only When using protocol v0, upload-pack over HTTP permits a "half-auth" configuration in which, at the web server layer, the info/refs path is not protected by authentication but the git-upload-pack path is, so that a user can perform fetches that do not download any objects without authentication, but still needs authentication to download objects. But protocol v2 does not support this, because both ref and pack are obtained from the git-upload-pack path. Mark the test verifying this behavior as protocol v0-only, with a description of what needs to be done to make v2 support this. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-22 20:01:39 +01:00
# NEEDSWORK: When using HTTP(S), protocol v0 supports a "half-auth"
# configuration with authentication required only when downloading
# objects and not refs, by having the HTTP server only require
# authentication for the "git-upload-pack" path and not "info/refs".
# This is not possible with protocol v2, since both objects and refs
# are obtained from the "git-upload-pack" path. A solution to this is
# to teach the server and client to be able to inline ls-refs requests
docs: move protocol-related docs to man section 5 Continue the move of existing Documentation/technical/* protocol and file-format documentation into our main documentation space. By moving the things that discuss the protocol we can properly link from e.g. lsrefs.unborn and protocol.version documentation to a manpage we build by default. So far we have been using the "gitformat-" prefix for the documentation we've been moving over from Documentation/technical/*, but for protocol documentation let's use "gitprotocol-*". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-04 18:28:36 +02:00
# as an Extra Parameter (see "git help gitformat-pack-protocol"), so that
# "info/refs" can serve refs, just like it does in protocol v0.
t5551: mark half-auth no-op fetch test as v0-only When using protocol v0, upload-pack over HTTP permits a "half-auth" configuration in which, at the web server layer, the info/refs path is not protected by authentication but the git-upload-pack path is, so that a user can perform fetches that do not download any objects without authentication, but still needs authentication to download objects. But protocol v2 does not support this, because both ref and pack are obtained from the git-upload-pack path. Mark the test verifying this behavior as protocol v0-only, with a description of what needs to be done to make v2 support this. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-03-22 20:01:39 +01:00
GIT_TEST_PROTOCOL_VERSION=0 git --git-dir=half-auth fetch &&
remote-curl: retry failed requests for auth even with gzip Commit b81401c taught the post_rpc function to retry the http request after prompting for credentials. However, it did not handle two cases: 1. If we have a large request, we do not retry. That's OK, since we would have sent a probe (with retry) already. 2. If we are gzipping the request, we do not retry. That was considered OK, because the intended use was for push (e.g., listing refs is OK, but actually pushing objects is not), and we never gzip on push. This patch teaches post_rpc to retry even a gzipped request. This has two advantages: 1. It is possible to configure a "half-auth" state for fetching, where the set of refs and their sha1s are advertised, but one cannot actually fetch objects. This is not a recommended configuration, as it leaks some information about what is in the repository (e.g., an attacker can try brute-forcing possible content in your repository and checking whether it matches your branch sha1). However, it can be slightly more convenient, since a no-op fetch will not require a password at all. 2. It future-proofs us should we decide to ever gzip more requests. Signed-off-by: Jeff King <peff@peff.net>
2012-10-31 12:29:16 +01:00
expect_askpass none
'
remote-curl: rewrite base url from info/refs redirects For efficiency and security reasons, an earlier commit in this series taught http_get_* to re-write the base url based on redirections we saw while making a specific request. This commit wires that option into the info/refs request, meaning that a redirect from http://example.com/foo.git/info/refs to https://example.com/bar.git/info/refs will behave as if "https://example.com/bar.git" had been provided to git in the first place. The tests bear some explanation. We introduce two new hierearchies into the httpd test config: 1. Requests to /smart-redir-limited will work only for the initial info/refs request, but not any subsequent requests. As a result, we can confirm whether the client is re-rooting its requests after the initial contact, since otherwise it will fail (it will ask for "repo.git/git-upload-pack", which is not redirected). 2. Requests to smart-redir-auth will redirect, and require auth after the redirection. Since we are using the redirected base for further requests, we also update the credential struct, in order not to mislead the user (or credential helpers) about which credential is needed. We can therefore check the GIT_ASKPASS prompts to make sure we are prompting for the new location. Because we have neither multiple servers nor https support in our test setup, we can only redirect between paths, meaning we need to turn on credential.useHttpPath to see the difference. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2013-09-28 10:35:35 +02:00
test_expect_success 'redirects send auth to new location' '
use distinct username/password for http auth tests The httpd server we set up to test git's http client code knows about a single account, in which both the username and password are "user@host" (the unusual use of the "@" here is to verify that we handle the character correctly when URL escaped). This means that we may miss a certain class of errors in which the username and password are mixed up internally by git. We can make our tests more robust by having distinct values for the username and password. In addition to tweaking the server passwd file and the client URL, we must teach the "askpass" harness to accept multiple values. As a bonus, this makes the setup of some tests more obvious; when we are expecting git to ask only about the password, we can seed the username askpass response with a bogus value. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-01-02 08:38:35 +01:00
set_askpass user@host pass@host &&
remote-curl: rewrite base url from info/refs redirects For efficiency and security reasons, an earlier commit in this series taught http_get_* to re-write the base url based on redirections we saw while making a specific request. This commit wires that option into the info/refs request, meaning that a redirect from http://example.com/foo.git/info/refs to https://example.com/bar.git/info/refs will behave as if "https://example.com/bar.git" had been provided to git in the first place. The tests bear some explanation. We introduce two new hierearchies into the httpd test config: 1. Requests to /smart-redir-limited will work only for the initial info/refs request, but not any subsequent requests. As a result, we can confirm whether the client is re-rooting its requests after the initial contact, since otherwise it will fail (it will ask for "repo.git/git-upload-pack", which is not redirected). 2. Requests to smart-redir-auth will redirect, and require auth after the redirection. Since we are using the redirected base for further requests, we also update the credential struct, in order not to mislead the user (or credential helpers) about which credential is needed. We can therefore check the GIT_ASKPASS prompts to make sure we are prompting for the new location. Because we have neither multiple servers nor https support in our test setup, we can only redirect between paths, meaning we need to turn on credential.useHttpPath to see the difference. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2013-09-28 10:35:35 +02:00
git -c credential.useHttpPath=true \
clone $HTTPD_URL/smart-redir-auth/repo.git repo-redir-auth &&
expect_askpass both user@host auth/smart/repo.git
'
http: redact curl h2h3 headers in info With GIT_TRACE_CURL=1 or GIT_CURL_VERBOSE=1, sensitive headers like "Authorization" and "Cookie" get redacted. However, since [1], curl's h2h3 module (invoked when using HTTP/2) also prints headers in its "info", which don't get redacted. For example, echo 'github.com TRUE / FALSE 1698960413304 o foo=bar' >cookiefile && GIT_TRACE_CURL=1 GIT_TRACE_CURL_NO_DATA=1 git \ -c 'http.cookiefile=cookiefile' \ -c 'http.version=' \ ls-remote https://github.com/git/git refs/heads/main 2>output && grep 'cookie' output produces output like: 23:04:16.920495 http.c:678 == Info: h2h3 [cookie: o=foo=bar] 23:04:16.920562 http.c:637 => Send header: cookie: o=<redacted> Teach http.c to check for h2h3 headers in info and redact them using the existing header redaction logic. This fixes the broken redaction logic that we noted in the previous commit, so mark the redaction tests as passing under HTTP2. [1] https://github.com/curl/curl/commit/f8c3724aa90472c0e617ddbbc420aa199971eb77 Helped-by: Jeff King <peff@peff.net> Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-11 23:35:06 +01:00
test_expect_success 'GIT_TRACE_CURL redacts auth details' '
t5551: test that GIT_TRACE_CURL redacts password Verify that when GIT_TRACE_CURL is set, Git prints out "Authorization: Basic <redacted>" instead of the base64-encoded authorization details. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-11 19:43:09 +02:00
rm -rf redact-auth trace &&
set_askpass user@host pass@host &&
GIT_TRACE_CURL="$(pwd)/trace" git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth &&
expect_askpass both user@host &&
# Ensure that there is no "Basic" followed by a base64 string, but that
# the auth details are redacted
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-23 00:11:53 +02:00
! grep -i "Authorization: Basic [0-9a-zA-Z+/]" trace &&
grep -i "Authorization: Basic <redacted>" trace
t5551: test that GIT_TRACE_CURL redacts password Verify that when GIT_TRACE_CURL is set, Git prints out "Authorization: Basic <redacted>" instead of the base64-encoded authorization details. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-11 19:43:09 +02:00
'
http: redact curl h2h3 headers in info With GIT_TRACE_CURL=1 or GIT_CURL_VERBOSE=1, sensitive headers like "Authorization" and "Cookie" get redacted. However, since [1], curl's h2h3 module (invoked when using HTTP/2) also prints headers in its "info", which don't get redacted. For example, echo 'github.com TRUE / FALSE 1698960413304 o foo=bar' >cookiefile && GIT_TRACE_CURL=1 GIT_TRACE_CURL_NO_DATA=1 git \ -c 'http.cookiefile=cookiefile' \ -c 'http.version=' \ ls-remote https://github.com/git/git refs/heads/main 2>output && grep 'cookie' output produces output like: 23:04:16.920495 http.c:678 == Info: h2h3 [cookie: o=foo=bar] 23:04:16.920562 http.c:637 => Send header: cookie: o=<redacted> Teach http.c to check for h2h3 headers in info and redact them using the existing header redaction logic. This fixes the broken redaction logic that we noted in the previous commit, so mark the redaction tests as passing under HTTP2. [1] https://github.com/curl/curl/commit/f8c3724aa90472c0e617ddbbc420aa199971eb77 Helped-by: Jeff King <peff@peff.net> Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-11 23:35:06 +01:00
test_expect_success 'GIT_CURL_VERBOSE redacts auth details' '
http, imap-send: stop using CURLOPT_VERBOSE Whenever GIT_CURL_VERBOSE is set, teach Git to behave as if GIT_TRACE_CURL=1 and GIT_TRACE_CURL_NO_DATA=1 is set, instead of setting CURLOPT_VERBOSE. This is to prevent inadvertent revelation of sensitive data. In particular, GIT_CURL_VERBOSE redacts neither the "Authorization" header nor any cookies specified by GIT_REDACT_COOKIES. Unifying the tracing mechanism also has the future benefit that any improvements to the tracing mechanism will benefit both users of GIT_CURL_VERBOSE and GIT_TRACE_CURL, and we do not need to remember to implement any improvement twice. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-11 19:43:10 +02:00
rm -rf redact-auth trace &&
set_askpass user@host pass@host &&
GIT_CURL_VERBOSE=1 git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth 2>trace &&
expect_askpass both user@host &&
# Ensure that there is no "Basic" followed by a base64 string, but that
# the auth details are redacted
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-23 00:11:53 +02:00
! grep -i "Authorization: Basic [0-9a-zA-Z+/]" trace &&
grep -i "Authorization: Basic <redacted>" trace
http, imap-send: stop using CURLOPT_VERBOSE Whenever GIT_CURL_VERBOSE is set, teach Git to behave as if GIT_TRACE_CURL=1 and GIT_TRACE_CURL_NO_DATA=1 is set, instead of setting CURLOPT_VERBOSE. This is to prevent inadvertent revelation of sensitive data. In particular, GIT_CURL_VERBOSE redacts neither the "Authorization" header nor any cookies specified by GIT_REDACT_COOKIES. Unifying the tracing mechanism also has the future benefit that any improvements to the tracing mechanism will benefit both users of GIT_CURL_VERBOSE and GIT_TRACE_CURL, and we do not need to remember to implement any improvement twice. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-11 19:43:10 +02:00
'
http: redact all cookies, teach GIT_TRACE_REDACT=0 In trace output (when GIT_TRACE_CURL is true), redact the values of all HTTP cookies by default. Now that auth headers (since the implementation of GIT_TRACE_CURL in 74c682d3c6 ("http.c: implement the GIT_TRACE_CURL environment variable", 2016-05-24)) and cookie values (since this commit) are redacted by default in these traces, also allow the user to inhibit these redactions through an environment variable. Since values of all cookies are now redacted by default, GIT_REDACT_COOKIES (which previously allowed users to select individual cookies to redact) now has no effect. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-06-05 23:21:36 +02:00
test_expect_success 'GIT_TRACE_CURL does not redact auth details if GIT_TRACE_REDACT=0' '
rm -rf redact-auth trace &&
set_askpass user@host pass@host &&
GIT_TRACE_REDACT=0 GIT_TRACE_CURL="$(pwd)/trace" \
git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth &&
expect_askpass both user@host &&
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-23 00:11:53 +02:00
grep -i "Authorization: Basic [0-9a-zA-Z+/]" trace
http: redact all cookies, teach GIT_TRACE_REDACT=0 In trace output (when GIT_TRACE_CURL is true), redact the values of all HTTP cookies by default. Now that auth headers (since the implementation of GIT_TRACE_CURL in 74c682d3c6 ("http.c: implement the GIT_TRACE_CURL environment variable", 2016-05-24)) and cookie values (since this commit) are redacted by default in these traces, also allow the user to inhibit these redactions through an environment variable. Since values of all cookies are now redacted by default, GIT_REDACT_COOKIES (which previously allowed users to select individual cookies to redact) now has no effect. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-06-05 23:21:36 +02:00
'
remote-curl: let users turn off smart http Usually there is no need for users to specify whether an http remote is smart or dumb; the protocol is designed so that a single initial request is made, and the client can determine the server's capability from the response. However, some misconfigured dumb-only servers may not like the initial request by a smart client, as it contains a query string. Until recently, commit 703e6e7 worked around this by making a second request. However, that commit was recently reverted due to its side effect of masking the initial request's error code. Since git has had that workaround for several years, we don't know exactly how many such misconfigured servers are out there. The reversion of 703e6e7 assumes they are rare enough not to worry about. Still, that reversion leaves somebody who does run into such a server with no escape hatch at all. Let's give them an environment variable they can tweak to perform the "dumb" request. This is intentionally not a documented interface. It's overly simple and is really there for debugging in case somebody does complain about git not working with their server. A real user-facing interface would entail a per-remote or per-URL config variable. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-09-20 23:30:58 +02:00
test_expect_success 'disable dumb http on server' '
git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
config http.getanyfile false
'
test_expect_success 'GIT_SMART_HTTP can disable smart http' '
(GIT_SMART_HTTP=0 &&
export GIT_SMART_HTTP &&
cd clone &&
test_must_fail git fetch)
'
Verify Content-Type from smart HTTP servers Before parsing a suspected smart-HTTP response verify the returned Content-Type matches the standard. This protects a client from attempting to process a payload that smells like a smart-HTTP server response. JGit has been doing this check on all responses since the dawn of time. I mistakenly failed to include it in git-core when smart HTTP was introduced. At the time I didn't know how to get the Content-Type from libcurl. I punted, meant to circle back and fix this, and just plain forgot about it. Signed-off-by: Shawn Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-01-31 22:02:07 +01:00
test_expect_success 'invalid Content-Type rejected' '
t: fix severe &&-chain breakage These are tests which are missing a link in their &&-chain, in a location which causes a significant portion of the test to be missed (e.g., the test effectively does nothing, or consists of a long string of actions and output comparisons, and we throw away the exit code of at least one part of the string). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-20 11:06:15 +01:00
test_must_fail git clone $HTTPD_URL/broken_smart/repo.git 2>actual &&
t5551: use 'test_i18ngrep' to check translated output The two tests 'invalid Content-Type rejected' and 'server-side error detected' in 't5551-http-fetch-smart.sh' use "plain" 'grep' to check that 'git clone' failed with the expected error message, but the messages they are checking are translated, and, consequently, these tests fail when the test script is run with GIT_TEST_GETTEXT_POISON enabled. Use 'test_i18ngrep' instead. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-24 14:44:46 +02:00
test_i18ngrep "not valid:" actual
Verify Content-Type from smart HTTP servers Before parsing a suspected smart-HTTP response verify the returned Content-Type matches the standard. This protects a client from attempting to process a payload that smells like a smart-HTTP server response. JGit has been doing this check on all responses since the dawn of time. I mistakenly failed to include it in git-core when smart HTTP was introduced. At the time I didn't know how to get the Content-Type from libcurl. I punted, meant to circle back and fix this, and just plain forgot about it. Signed-off-by: Shawn Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-01-31 22:02:07 +01:00
'
http-backend: respect GIT_NAMESPACE with dumb clients Filter the list of refs returned via the dumb HTTP protocol according to the active namespace, consistent with other clients of the upload-pack service. Signed-off-by: John Koleszar <jkoleszar@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-04-10 02:55:08 +02:00
test_expect_success 'create namespaced refs' '
test_commit namespaced &&
t55[4-9]*: adjust the references to the default branch name "main" This trick was performed via $ (cd t && sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \ -e 's/Master/Main/g' -e 's/retsam/niam/g' \ -- t55[4-9]*.sh t556x*) This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main` for those tests. Note that t5541 uses the reversed `master` name: `retsam`. We replace it by the equivalent for `main`: `niam`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-19 00:44:34 +01:00
git push public HEAD:refs/namespaces/ns/refs/heads/main &&
http-backend: respect GIT_NAMESPACE with dumb clients Filter the list of refs returned via the dumb HTTP protocol according to the active namespace, consistent with other clients of the upload-pack service. Signed-off-by: John Koleszar <jkoleszar@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-04-10 02:55:08 +02:00
git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
t55[4-9]*: adjust the references to the default branch name "main" This trick was performed via $ (cd t && sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \ -e 's/Master/Main/g' -e 's/retsam/niam/g' \ -- t55[4-9]*.sh t556x*) This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main` for those tests. Note that t5541 uses the reversed `master` name: `retsam`. We replace it by the equivalent for `main`: `niam`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-19 00:44:34 +01:00
symbolic-ref refs/namespaces/ns/HEAD refs/namespaces/ns/refs/heads/main
http-backend: respect GIT_NAMESPACE with dumb clients Filter the list of refs returned via the dumb HTTP protocol according to the active namespace, consistent with other clients of the upload-pack service. Signed-off-by: John Koleszar <jkoleszar@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-04-10 02:55:08 +02:00
'
test_expect_success 'smart clone respects namespace' '
git clone "$HTTPD_URL/smart_namespace/repo.git" ns-smart &&
echo namespaced >expect &&
git --git-dir=ns-smart/.git log -1 --format=%s >actual &&
test_cmp expect actual
'
test_expect_success 'dumb clone via http-backend respects namespace' '
git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
config http.getanyfile true &&
GIT_SMART_HTTP=0 git clone \
"$HTTPD_URL/smart_namespace/repo.git" ns-dumb &&
echo namespaced >expect &&
git --git-dir=ns-dumb/.git log -1 --format=%s >actual &&
test_cmp expect actual
'
http: add http.savecookies option to write out HTTP cookies HTTP servers may send Set-Cookie headers in a response and expect them to be set on subsequent requests. By default, libcurl behavior is to store such cookies in memory and reuse them across requests within a single session. However, it may also make sense, depending on the server and the cookies, to store them across sessions. Provide users an option to enable this behavior, writing cookies out to the same file specified in http.cookiefile. Signed-off-by: Dave Borowitz <dborowitz@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-07-24 00:40:17 +02:00
test_expect_success 'cookies stored in http.cookiefile when http.savecookies set' '
t5551: move setup code inside test_expect blocks Move setup code inside test_expect blocks, to catch unexpected failures in the setup steps, and bring the test scripts in line with our modern test style. Suggested-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-17 23:46:27 +02:00
cat >cookies.txt <<-\EOF &&
127.0.0.1 FALSE /smart_cookies/ FALSE 0 othername othervalue
EOF
t5551: compare sorted cookies files In t5551 we check that we save cookies correctly to a file when http.cookiefile and http.savecookies are set. To do so we create an expect file that expects the cookies in a certain order. However after e2ef8d6fa ("cookies: support creation-time attribute for cookies", 2018-08-28) in curl.git (released in curl 7.61.1) that order changed. We document the file format as "Netscape/Mozilla cookie file format (see curl(1))", so any format produced by libcurl should be fine here. Sort the files, to be agnostic to the order of the cookies, and make the test pass with both curl versions > 7.61.1 and earlier curl versions. Reported-by: Todd Zullinger <tmz@pobox.com> Helped-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-17 23:46:28 +02:00
sort >expect_cookies.txt <<-\EOF &&
t5551: move setup code inside test_expect blocks Move setup code inside test_expect blocks, to catch unexpected failures in the setup steps, and bring the test scripts in line with our modern test style. Suggested-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-17 23:46:27 +02:00
127.0.0.1 FALSE /smart_cookies/ FALSE 0 othername othervalue
t5551: handle v2 protocol in cookie test After making a request, we check that it stored the expected cookies. This depends on the protocol version, because the cookies we store depend on the exact requests we made (and for ls-remote, v2 will always hit /git-upload-pack to get the refs, whereas v0 is happy with the initial ref advertisement). As a result, hardly anybody runs this test, as you'd have to manually set GIT_TEST_PROTOCOL_VERSION=0 to do so. Let's teach it to handle both protocol versions. One way to do this would be to make the expectation conditional on the protocol used. But there's a simpler solution. The reason that v0 doesn't hit /git-upload-pack is that ls-remote doesn't fetch any objects. If we instead do a fetch (making sure there's an actual object to grab), then both v0 and v2 will hit the same endpoints and set the same cookies. Note that we do have to clean up our new tag here; otherwise it confuses the later "clone 2,000 tags" test. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 12:02:31 +01:00
127.0.0.1 FALSE /smart_cookies/repo.git/ FALSE 0 name value
t5551: move setup code inside test_expect blocks Move setup code inside test_expect blocks, to catch unexpected failures in the setup steps, and bring the test scripts in line with our modern test style. Suggested-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-17 23:46:27 +02:00
127.0.0.1 FALSE /smart_cookies/repo.git/info/ FALSE 0 name value
EOF
http: add http.savecookies option to write out HTTP cookies HTTP servers may send Set-Cookie headers in a response and expect them to be set on subsequent requests. By default, libcurl behavior is to store such cookies in memory and reuse them across requests within a single session. However, it may also make sense, depending on the server and the cookies, to store them across sessions. Provide users an option to enable this behavior, writing cookies out to the same file specified in http.cookiefile. Signed-off-by: Dave Borowitz <dborowitz@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-07-24 00:40:17 +02:00
git config http.cookiefile cookies.txt &&
git config http.savecookies true &&
tests: fix protocol version for overspecifications These tests are also marked with a NEEDSWORK comment. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-25 22:54:12 +01:00
t5551: handle v2 protocol in cookie test After making a request, we check that it stored the expected cookies. This depends on the protocol version, because the cookies we store depend on the exact requests we made (and for ls-remote, v2 will always hit /git-upload-pack to get the refs, whereas v0 is happy with the initial ref advertisement). As a result, hardly anybody runs this test, as you'd have to manually set GIT_TEST_PROTOCOL_VERSION=0 to do so. Let's teach it to handle both protocol versions. One way to do this would be to make the expectation conditional on the protocol used. But there's a simpler solution. The reason that v0 doesn't hit /git-upload-pack is that ls-remote doesn't fetch any objects. If we instead do a fetch (making sure there's an actual object to grab), then both v0 and v2 will hit the same endpoints and set the same cookies. Note that we do have to clean up our new tag here; otherwise it confuses the later "clone 2,000 tags" test. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 12:02:31 +01:00
test_when_finished "
git --git-dir=\"\$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" \
tag -d cookie-tag
" &&
git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
tag -m "foo" cookie-tag &&
git fetch $HTTPD_URL/smart_cookies/repo.git cookie-tag &&
grep "^[^#]" cookies.txt | sort >cookies_stripped.txt &&
test_cmp expect_cookies.txt cookies_stripped.txt
http: add http.savecookies option to write out HTTP cookies HTTP servers may send Set-Cookie headers in a response and expect them to be set on subsequent requests. By default, libcurl behavior is to store such cookies in memory and reuse them across requests within a single session. However, it may also make sense, depending on the server and the cookies, to store them across sessions. Provide users an option to enable this behavior, writing cookies out to the same file specified in http.cookiefile. Signed-off-by: Dave Borowitz <dborowitz@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-07-24 00:40:17 +02:00
'
upload-pack: fix transfer.hiderefs over smart-http When upload-pack advertises the refs (either for a normal, non-stateless request, or for the initial contact in a stateless one), we call for_each_ref with the send_ref function as its callback. send_ref, in turn, calls mark_our_ref, which checks whether the ref is hidden, and sets OUR_REF or HIDDEN_REF on the object as appropriate. If it is hidden, mark_our_ref also returns "1" to signal send_ref that the ref should not be advertised. If we are not advertising refs, (i.e., the follow-up invocation by an http client to send its "want" lines), we use mark_our_ref directly as a callback to for_each_ref. Its marking does the right thing, but when it then returns "1" to for_each_ref, the latter interprets this as an error and stops iterating. As a result, we skip marking all of the refs that come lexicographically after it. Any "want" lines from the client asking for those objects will fail, as they were not properly marked with OUR_REF. To solve this, we introduce a wrapper callback around mark_our_ref which always returns 0 (even if the ref is hidden, we want to keep iterating). We also tweak the signature of mark_our_ref to exclude unnecessary parameters that were present only to conform to the callback interface. This should make it less likely for somebody to accidentally use it as a callback in the future. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-13 05:42:12 +01:00
test_expect_success 'transfer.hiderefs works over smart-http' '
test_commit hidden &&
test_commit visible &&
git push public HEAD^:refs/heads/a HEAD:refs/heads/b &&
git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" \
config transfer.hiderefs refs/heads/a &&
git clone --bare "$HTTPD_URL/smart/repo.git" hidden.git &&
test_must_fail git -C hidden.git rev-parse --verify a &&
git -C hidden.git rev-parse --verify b
'
t5551: factor out tag creation One of our tests in t5551 creates a large number of tags, and jumps through some hoops to do it efficiently. Let's factor that out into a function so we can make other similar tests. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-05-20 09:36:43 +02:00
# create an arbitrary number of tags, numbered from tag-$1 to tag-$2
create_tags () {
rm -f marks &&
for i in $(test_seq "$1" "$2")
remote-curl: main test case for the OS command line overflow This is main test case for the original problem that triggered this patch series. We create a repo with 50k tags and then test whether git-clone over the smart HTTP protocol succeeds. Note that we construct the repo in a slightly different way than the original script used to reproduce the problem. This is because the original script just created 50k tags all pointing to the same commit, so if there was a bug where remote-curl.c was not passing all the refs to fetch-pack we wouldn't know. The clone would succeed even if only one tag was passed, because all the other tags were pointing at the same SHA and would be considered present. Instead we create a repo with 50k independent (dangling) commits and then tag each of those commits with a unique tag. This way if one of the tags is not given to fetch-pack, later stages of the clone would complain about it. This allows us to test both that the command line overflow was fixed, as well as that it was fixed in a way that doesn't leave out any of the refs. Signed-off-by: Ivan Todoroski <grnch@gmx.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-04-02 17:17:03 +02:00
do
t5551: factor out tag creation One of our tests in t5551 creates a large number of tags, and jumps through some hoops to do it efficiently. Let's factor that out into a function so we can make other similar tests. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-05-20 09:36:43 +02:00
# don't use here-doc, because it requires a process
# per loop iteration
echo "commit refs/heads/too-many-refs-$1" &&
echo "mark :$i" &&
echo "committer git <git@example.com> $i +0000" &&
echo "data 0" &&
echo "M 644 inline bla.txt" &&
echo "data 4" &&
echo "bla" &&
remote-curl: main test case for the OS command line overflow This is main test case for the original problem that triggered this patch series. We create a repo with 50k tags and then test whether git-clone over the smart HTTP protocol succeeds. Note that we construct the repo in a slightly different way than the original script used to reproduce the problem. This is because the original script just created 50k tags all pointing to the same commit, so if there was a bug where remote-curl.c was not passing all the refs to fetch-pack we wouldn't know. The clone would succeed even if only one tag was passed, because all the other tags were pointing at the same SHA and would be considered present. Instead we create a repo with 50k independent (dangling) commits and then tag each of those commits with a unique tag. This way if one of the tags is not given to fetch-pack, later stages of the clone would complain about it. This allows us to test both that the command line overflow was fixed, as well as that it was fixed in a way that doesn't leave out any of the refs. Signed-off-by: Ivan Todoroski <grnch@gmx.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-04-02 17:17:03 +02:00
# make every commit dangling by always
# rewinding the branch after each commit
t5551: factor out tag creation One of our tests in t5551 creates a large number of tags, and jumps through some hoops to do it efficiently. Let's factor that out into a function so we can make other similar tests. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-05-20 09:36:43 +02:00
echo "reset refs/heads/too-many-refs-$1" &&
echo "from :$1"
remote-curl: main test case for the OS command line overflow This is main test case for the original problem that triggered this patch series. We create a repo with 50k tags and then test whether git-clone over the smart HTTP protocol succeeds. Note that we construct the repo in a slightly different way than the original script used to reproduce the problem. This is because the original script just created 50k tags all pointing to the same commit, so if there was a bug where remote-curl.c was not passing all the refs to fetch-pack we wouldn't know. The clone would succeed even if only one tag was passed, because all the other tags were pointing at the same SHA and would be considered present. Instead we create a repo with 50k independent (dangling) commits and then tag each of those commits with a unique tag. This way if one of the tags is not given to fetch-pack, later stages of the clone would complain about it. This allows us to test both that the command line overflow was fixed, as well as that it was fixed in a way that doesn't leave out any of the refs. Signed-off-by: Ivan Todoroski <grnch@gmx.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-04-02 17:17:03 +02:00
done | git fast-import --export-marks=marks &&
# now assign tags to all the dangling commits we created above
t: use perl instead of "$PERL_PATH" where applicable As of the last commit, we can use "perl" instead of "$PERL_PATH" when running tests, as the former is now a function which uses the latter. As the shorter "perl" is easier on the eyes, let's switch to using it everywhere. This is not quite a mechanical s/$PERL_PATH/perl/ replacement, though. There are some places where we invoke perl from a script we generate on the fly, and those scripts do not have access to our internal shell functions. The result can be double-checked by running: ln -s /bin/false bin-wrappers/perl make test which continues to pass even after this patch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-10-29 02:23:03 +01:00
tag=$(perl -e "print \"bla\" x 30") &&
t5551: do not use unportable sed '\+' The set-up step to prepare a repository with 50000 tags used a non-porable '\+' to match one-or-more. The error was not caught because the next test that uses that repository did not even bother to check if these expected tags were actually cloned to the resulting repository. Fix the sed construct to use BRE and update the "clone" test that wanted to test cloning from such a repository with many refs to check the resulting repository. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-05-13 00:50:59 +02:00
sed -e "s|^:\([^ ]*\) \(.*\)$|\2 refs/tags/$tag-\1|" <marks >>packed-refs
t5551: factor out tag creation One of our tests in t5551 creates a large number of tags, and jumps through some hoops to do it efficiently. Let's factor that out into a function so we can make other similar tests. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-05-20 09:36:43 +02:00
}
Merge branch 'jk/http-backend-deadlock-2.2' into jk/http-backend-deadlock-2.3 * jk/http-backend-deadlock-2.2: http-backend: spool ref negotiation requests to buffer t5551: factor out tag creation http-backend: fix die recursion with custom handler
2015-05-26 05:44:04 +02:00
test_expect_success 'create 2,000 tags in the repo' '
t5551: factor out tag creation One of our tests in t5551 creates a large number of tags, and jumps through some hoops to do it efficiently. Let's factor that out into a function so we can make other similar tests. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-05-20 09:36:43 +02:00
(
cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
Merge branch 'jk/http-backend-deadlock-2.2' into jk/http-backend-deadlock-2.3 * jk/http-backend-deadlock-2.2: http-backend: spool ref negotiation requests to buffer t5551: factor out tag creation http-backend: fix die recursion with custom handler
2015-05-26 05:44:04 +02:00
create_tags 1 2000
remote-curl: main test case for the OS command line overflow This is main test case for the original problem that triggered this patch series. We create a repo with 50k tags and then test whether git-clone over the smart HTTP protocol succeeds. Note that we construct the repo in a slightly different way than the original script used to reproduce the problem. This is because the original script just created 50k tags all pointing to the same commit, so if there was a bug where remote-curl.c was not passing all the refs to fetch-pack we wouldn't know. The clone would succeed even if only one tag was passed, because all the other tags were pointing at the same SHA and would be considered present. Instead we create a repo with 50k independent (dangling) commits and then tag each of those commits with a unique tag. This way if one of the tags is not given to fetch-pack, later stages of the clone would complain about it. This allows us to test both that the command line overflow was fixed, as well as that it was fixed in a way that doesn't leave out any of the refs. Signed-off-by: Ivan Todoroski <grnch@gmx.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-04-02 17:17:03 +02:00
)
'
t5551: make EXPENSIVE test cheaper We create 50,000 tags to check that we don't overflow the command-line of fetch-pack. But by using run_with_cmdline_limit, we can get the same effect with a much smaller number of tags. This makes the test fast enough that we can drop the EXPENSIVE prereq, which means people will actually run it. It was not documented to do so, but this test was also the only test of a clone-over-http that requires multiple POSTs during the conversation. We can continue to test that by dropping http.postbuffer to its minimum size, and checking that we get two POSTs. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-13 05:57:05 +01:00
test_expect_success CMDLINE_LIMIT \
'clone the 2,000 tag repo to check OS command line overflow' '
run_with_limited_cmdline git clone $HTTPD_URL/smart/repo.git too-many-refs &&
t5551: do not use unportable sed '\+' The set-up step to prepare a repository with 50000 tags used a non-porable '\+' to match one-or-more. The error was not caught because the next test that uses that repository did not even bother to check if these expected tags were actually cloned to the resulting repository. Fix the sed construct to use BRE and update the "clone" test that wanted to test cloning from such a repository with many refs to check the resulting repository. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-05-13 00:50:59 +02:00
(
cd too-many-refs &&
t5551: make EXPENSIVE test cheaper We create 50,000 tags to check that we don't overflow the command-line of fetch-pack. But by using run_with_cmdline_limit, we can get the same effect with a much smaller number of tags. This makes the test fast enough that we can drop the EXPENSIVE prereq, which means people will actually run it. It was not documented to do so, but this test was also the only test of a clone-over-http that requires multiple POSTs during the conversation. We can continue to test that by dropping http.postbuffer to its minimum size, and checking that we get two POSTs. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-13 05:57:05 +01:00
git for-each-ref refs/tags >actual &&
test_line_count = 2000 actual
t5551: do not use unportable sed '\+' The set-up step to prepare a repository with 50000 tags used a non-porable '\+' to match one-or-more. The error was not caught because the next test that uses that repository did not even bother to check if these expected tags were actually cloned to the resulting repository. Fix the sed construct to use BRE and update the "clone" test that wanted to test cloning from such a repository with many refs to check the resulting repository. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-05-13 00:50:59 +02:00
)
remote-curl: main test case for the OS command line overflow This is main test case for the original problem that triggered this patch series. We create a repo with 50k tags and then test whether git-clone over the smart HTTP protocol succeeds. Note that we construct the repo in a slightly different way than the original script used to reproduce the problem. This is because the original script just created 50k tags all pointing to the same commit, so if there was a bug where remote-curl.c was not passing all the refs to fetch-pack we wouldn't know. The clone would succeed even if only one tag was passed, because all the other tags were pointing at the same SHA and would be considered present. Instead we create a repo with 50k independent (dangling) commits and then tag each of those commits with a unique tag. This way if one of the tags is not given to fetch-pack, later stages of the clone would complain about it. This allows us to test both that the command line overflow was fixed, as well as that it was fixed in a way that doesn't leave out any of the refs. Signed-off-by: Ivan Todoroski <grnch@gmx.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-04-02 17:17:03 +02:00
'
t5551: test usage of chunked encoding explicitly When run using GIT_TEST_PROTOCOL_VERSION=2, a test in t5551 fails because 4 POSTs (probe, ls-refs, probe, fetch) are sent instead of 2 (probe, fetch). One way to resolve this would be to relax the condition (from "= 2" to greater than 1, say), but upon further inspection, the test probably shouldn't be counting the number of POSTs. This test states that large requests are split across POSTs, but this is not correct; the main change is that chunked transfer encoding is used, but the request is still contained within one POST. (The test coincidentally works because Git indeed sends 2 POSTs in the case of a large request, but that is because, as stated above, the first POST is a probing RPC - see post_rpc() in remote-curl.c for more information.) Therefore, instead of counting POSTs, check that chunked transfer encoding is used. This also has the desirable side effect of passing with GIT_TEST_PROTOCOL_VERSION=2. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Acked-by: Derrick Stolee <dstolee@microsoft.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-05 21:26:24 +02:00
test_expect_success 'large fetch-pack requests can be sent using chunked encoding' '
t5551-http-fetch-smart.sh: use the GIT_TRACE_CURL environment var Use the new GIT_TRACE_CURL environment variable instead of the deprecated GIT_CURL_VERBOSE. Signed-off-by: Elia Pinto <gitter.spiros@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-05 12:24:44 +02:00
GIT_TRACE_CURL=true git -c http.postbuffer=65536 \
t5551: make EXPENSIVE test cheaper We create 50,000 tags to check that we don't overflow the command-line of fetch-pack. But by using run_with_cmdline_limit, we can get the same effect with a much smaller number of tags. This makes the test fast enough that we can drop the EXPENSIVE prereq, which means people will actually run it. It was not documented to do so, but this test was also the only test of a clone-over-http that requires multiple POSTs during the conversation. We can continue to test that by dropping http.postbuffer to its minimum size, and checking that we get two POSTs. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-13 05:57:05 +01:00
clone --bare "$HTTPD_URL/smart/repo.git" split.git 2>err &&
t: run t5551 tests with both HTTP and HTTP/2 We have occasionally seen bugs that affect Git running only against an HTTP/2 web server, not an HTTP one. For instance, b66c77a64e (http: match headers case-insensitively when redacting, 2021-09-22). But since we have no test coverage using HTTP/2, we only uncover these bugs in the wild. That commit gives a recipe for converting our Apache setup to support HTTP/2, but: - it's not necessarily portable - we don't want to just test HTTP/2; we really want to do a variety of basic tests for _both_ protocols This patch handles both problems by running a duplicate of t5551 (labeled as t5559 here) with an alternate-universe setup that enables HTTP/2. So we'll continue to run t5551 as before, but run the same battery of tests again with HTTP/2. If HTTP/2 isn't supported on a given platform, then t5559 should bail during the webserver setup, and gracefully skip all tests (unless GIT_TEST_HTTPD has been changed from "auto" to "yes", where the point is to complain when webserver setup fails). In theory other http-related test scripts could benefit from the same duplication, but doing t5551 should give us a reasonable check of basic functionality, and would have caught both bugs we've seen in the wild with HTTP/2. A few notes on the implementation: - a script enables the server side config by calling enable_http2 before starting the webserver. This avoids even trying to load any HTTP/2 config for t5551 (which is what lets it keep working with regular HTTP even on systems that don't support it). This also sets a prereq which can be used by individual tests. - As discussed in b66c77a64e, the http2 module isn't compatible with the "prefork" mpm, so we need to pick something else. I chose "event" here, which works on my Debian system, but it's possible there are platforms which would prefer something else. We can adjust that later if somebody finds such a platform. - The test "large fetch-pack requests can be sent using chunked encoding" makes sure we use a chunked transfer-encoding by looking for that header in the trace. But since HTTP/2 has its own streaming mechanisms, we won't find such a header. We could skip the test entirely by marking it with !HTTP2. But there's some value in making sure that the fetch itself succeeded. So instead, we'll confirm that either we're using HTTP2 _or_ we saw the expected chunked header. - the redaction tests fail under HTTP/2 with recent versions of curl. This is a bug! I've marked them with !HTTP2 here to skip them under t5559 for the moment. Using test_expect_failure would be more appropriate, but would require a bunch of boilerplate. Since we'll be fixing them momentarily, let's just skip them for now to keep the test suite bisectable, and we can re-enable them in the commit that fixes the bug. - one alternative layout would be to push most of t5551 into a lib-t5551.sh script, then source it from both t5551 and t5559. Keeping t5551 intact seemed a little simpler, as its one less level of indirection for people fixing bugs/regressions in the non-HTTP/2 tests. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-11 23:35:05 +01:00
{
test_have_prereq HTTP2 ||
grep "^=> Send header: Transfer-Encoding: chunked" err
}
t5551: make EXPENSIVE test cheaper We create 50,000 tags to check that we don't overflow the command-line of fetch-pack. But by using run_with_cmdline_limit, we can get the same effect with a much smaller number of tags. This makes the test fast enough that we can drop the EXPENSIVE prereq, which means people will actually run it. It was not documented to do so, but this test was also the only test of a clone-over-http that requires multiple POSTs during the conversation. We can continue to test that by dropping http.postbuffer to its minimum size, and checking that we get two POSTs. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-13 05:57:05 +01:00
'
remote-curl: don't hang when a server dies before any output In the event that a HTTP server closes the connection after giving a 200 but before giving any packets, we don't want to hang forever waiting for a response that will never come. Instead, we should die immediately. One case where this happens is when attempting to fetch a dangling object by its object name. In this case, the server dies before sending any data. Prior to this patch, fetch-pack would wait for data from the server, and remote-curl would wait for fetch-pack, causing a deadlock. Despite this patch, there is other possible malformed input that could cause the same deadlock (e.g. a half-finished pktline, or a pktline but no trailing flush). There are a few possible solutions to this: 1. Allowing remote-curl to tell fetch-pack about the EOF (so that fetch-pack could know that no more data is coming until it says something else). This is tricky because an out-of-band signal would be required, or the http response would have to be re-framed inside another layer of pkt-line or something. 2. Make remote-curl understand some of the protocol. It turns out that in addition to understanding pkt-line, it would need to watch for ack/nak. This is somewhat fragile, as information about the protocol would end up in two places. Also, pkt-lines which are already at the length limit would need special handling. Both of these solutions would require a fair amount of work, whereas this hack is easy and solves at least some of the problem. Still to do: it would be good to give a better error message than "fatal: The remote end hung up unexpectedly". Signed-off-by: David Turner <dturner@twosigma.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-18 21:30:49 +01:00
test_expect_success 'test allowreachablesha1inwant' '
test_when_finished "rm -rf test_reachable.git" &&
server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
t55[4-9]*: adjust the references to the default branch name "main" This trick was performed via $ (cd t && sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \ -e 's/Master/Main/g' -e 's/retsam/niam/g' \ -- t55[4-9]*.sh t556x*) This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main` for those tests. Note that t5541 uses the reversed `master` name: `retsam`. We replace it by the equivalent for `main`: `niam`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-19 00:44:34 +01:00
main_sha=$(git -C "$server" rev-parse refs/heads/main) &&
remote-curl: don't hang when a server dies before any output In the event that a HTTP server closes the connection after giving a 200 but before giving any packets, we don't want to hang forever waiting for a response that will never come. Instead, we should die immediately. One case where this happens is when attempting to fetch a dangling object by its object name. In this case, the server dies before sending any data. Prior to this patch, fetch-pack would wait for data from the server, and remote-curl would wait for fetch-pack, causing a deadlock. Despite this patch, there is other possible malformed input that could cause the same deadlock (e.g. a half-finished pktline, or a pktline but no trailing flush). There are a few possible solutions to this: 1. Allowing remote-curl to tell fetch-pack about the EOF (so that fetch-pack could know that no more data is coming until it says something else). This is tricky because an out-of-band signal would be required, or the http response would have to be re-framed inside another layer of pkt-line or something. 2. Make remote-curl understand some of the protocol. It turns out that in addition to understanding pkt-line, it would need to watch for ack/nak. This is somewhat fragile, as information about the protocol would end up in two places. Also, pkt-lines which are already at the length limit would need special handling. Both of these solutions would require a fair amount of work, whereas this hack is easy and solves at least some of the problem. Still to do: it would be good to give a better error message than "fatal: The remote end hung up unexpectedly". Signed-off-by: David Turner <dturner@twosigma.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-18 21:30:49 +01:00
git -C "$server" config uploadpack.allowreachablesha1inwant 1 &&
git init --bare test_reachable.git &&
git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
t55[4-9]*: adjust the references to the default branch name "main" This trick was performed via $ (cd t && sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \ -e 's/Master/Main/g' -e 's/retsam/niam/g' \ -- t55[4-9]*.sh t556x*) This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main` for those tests. Note that t5541 uses the reversed `master` name: `retsam`. We replace it by the equivalent for `main`: `niam`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-19 00:44:34 +01:00
git -C test_reachable.git fetch origin "$main_sha"
remote-curl: don't hang when a server dies before any output In the event that a HTTP server closes the connection after giving a 200 but before giving any packets, we don't want to hang forever waiting for a response that will never come. Instead, we should die immediately. One case where this happens is when attempting to fetch a dangling object by its object name. In this case, the server dies before sending any data. Prior to this patch, fetch-pack would wait for data from the server, and remote-curl would wait for fetch-pack, causing a deadlock. Despite this patch, there is other possible malformed input that could cause the same deadlock (e.g. a half-finished pktline, or a pktline but no trailing flush). There are a few possible solutions to this: 1. Allowing remote-curl to tell fetch-pack about the EOF (so that fetch-pack could know that no more data is coming until it says something else). This is tricky because an out-of-band signal would be required, or the http response would have to be re-framed inside another layer of pkt-line or something. 2. Make remote-curl understand some of the protocol. It turns out that in addition to understanding pkt-line, it would need to watch for ack/nak. This is somewhat fragile, as information about the protocol would end up in two places. Also, pkt-lines which are already at the length limit would need special handling. Both of these solutions would require a fair amount of work, whereas this hack is easy and solves at least some of the problem. Still to do: it would be good to give a better error message than "fatal: The remote end hung up unexpectedly". Signed-off-by: David Turner <dturner@twosigma.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-18 21:30:49 +01:00
'
test_expect_success 'test allowreachablesha1inwant with unreachable' '
test_when_finished "rm -rf test_reachable.git; git reset --hard $(git rev-parse HEAD)" &&
#create unreachable sha
echo content >file2 &&
git add file2 &&
git commit -m two &&
git push public HEAD:refs/heads/doomed &&
git push public :refs/heads/doomed &&
server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
t55[4-9]*: adjust the references to the default branch name "main" This trick was performed via $ (cd t && sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \ -e 's/Master/Main/g' -e 's/retsam/niam/g' \ -- t55[4-9]*.sh t556x*) This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main` for those tests. Note that t5541 uses the reversed `master` name: `retsam`. We replace it by the equivalent for `main`: `niam`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-19 00:44:34 +01:00
main_sha=$(git -C "$server" rev-parse refs/heads/main) &&
remote-curl: don't hang when a server dies before any output In the event that a HTTP server closes the connection after giving a 200 but before giving any packets, we don't want to hang forever waiting for a response that will never come. Instead, we should die immediately. One case where this happens is when attempting to fetch a dangling object by its object name. In this case, the server dies before sending any data. Prior to this patch, fetch-pack would wait for data from the server, and remote-curl would wait for fetch-pack, causing a deadlock. Despite this patch, there is other possible malformed input that could cause the same deadlock (e.g. a half-finished pktline, or a pktline but no trailing flush). There are a few possible solutions to this: 1. Allowing remote-curl to tell fetch-pack about the EOF (so that fetch-pack could know that no more data is coming until it says something else). This is tricky because an out-of-band signal would be required, or the http response would have to be re-framed inside another layer of pkt-line or something. 2. Make remote-curl understand some of the protocol. It turns out that in addition to understanding pkt-line, it would need to watch for ack/nak. This is somewhat fragile, as information about the protocol would end up in two places. Also, pkt-lines which are already at the length limit would need special handling. Both of these solutions would require a fair amount of work, whereas this hack is easy and solves at least some of the problem. Still to do: it would be good to give a better error message than "fatal: The remote end hung up unexpectedly". Signed-off-by: David Turner <dturner@twosigma.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-18 21:30:49 +01:00
git -C "$server" config uploadpack.allowreachablesha1inwant 1 &&
git init --bare test_reachable.git &&
git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
tests: always test fetch of unreachable with v0 Some tests check that fetching an unreachable object fails, but protocol v2 allows such fetches. Unset GIT_TEST_PROTOCOL_VERSION so that these tests are always run using protocol v0. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-25 22:54:08 +01:00
# Some protocol versions (e.g. 2) support fetching
# unadvertised objects, so restrict this test to v0.
test: request GIT_TEST_PROTOCOL_VERSION=0 when appropriate Since 8cbeba0632 (tests: define GIT_TEST_PROTOCOL_VERSION, 2019-02-25), it has been possible to run tests with a newer protocol version by setting the GIT_TEST_PROTOCOL_VERSION envvar to a version number. Tests that assume protocol v0 handle this by explicitly setting GIT_TEST_PROTOCOL_VERSION= or similar constructs like 'test -z "$GIT_TEST_PROTOCOL_VERSION" || return 0' to declare that they only handle the default (v0) protocol. The emphasis there is a bit off: it would be clearer to specify GIT_TEST_PROTOCOL_VERSION=0 to inform the reader that these tests are specifically testing and relying on details of protocol v0. Do so. This way, a reader does not need to know what the default protocol version is, and the tests can continue to work when the default protocol version used by Git advances past v0. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-24 02:01:10 +01:00
test_must_fail env GIT_TEST_PROTOCOL_VERSION=0 \
tests: always test fetch of unreachable with v0 Some tests check that fetching an unreachable object fails, but protocol v2 allows such fetches. Unset GIT_TEST_PROTOCOL_VERSION so that these tests are always run using protocol v0. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-25 22:54:08 +01:00
git -C test_reachable.git fetch origin "$(git rev-parse HEAD)"
remote-curl: don't hang when a server dies before any output In the event that a HTTP server closes the connection after giving a 200 but before giving any packets, we don't want to hang forever waiting for a response that will never come. Instead, we should die immediately. One case where this happens is when attempting to fetch a dangling object by its object name. In this case, the server dies before sending any data. Prior to this patch, fetch-pack would wait for data from the server, and remote-curl would wait for fetch-pack, causing a deadlock. Despite this patch, there is other possible malformed input that could cause the same deadlock (e.g. a half-finished pktline, or a pktline but no trailing flush). There are a few possible solutions to this: 1. Allowing remote-curl to tell fetch-pack about the EOF (so that fetch-pack could know that no more data is coming until it says something else). This is tricky because an out-of-band signal would be required, or the http response would have to be re-framed inside another layer of pkt-line or something. 2. Make remote-curl understand some of the protocol. It turns out that in addition to understanding pkt-line, it would need to watch for ack/nak. This is somewhat fragile, as information about the protocol would end up in two places. Also, pkt-lines which are already at the length limit would need special handling. Both of these solutions would require a fair amount of work, whereas this hack is easy and solves at least some of the problem. Still to do: it would be good to give a better error message than "fatal: The remote end hung up unexpectedly". Signed-off-by: David Turner <dturner@twosigma.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-18 21:30:49 +01:00
'
upload-pack: optionally allow fetching any sha1 It seems a little silly to do a reachabilty check in the case where we trust the user to access absolutely everything in the repository. Also, it's racy in a distributed system -- perhaps one server advertises a ref, but another has since had a force-push to that ref, and perhaps the two HTTP requests end up directed to these different servers. Signed-off-by: David Turner <dturner@twosigma.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-11 18:23:48 +01:00
test_expect_success 'test allowanysha1inwant with unreachable' '
test_when_finished "rm -rf test_reachable.git; git reset --hard $(git rev-parse HEAD)" &&
#create unreachable sha
echo content >file2 &&
git add file2 &&
git commit -m two &&
git push public HEAD:refs/heads/doomed &&
git push public :refs/heads/doomed &&
server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
t55[4-9]*: adjust the references to the default branch name "main" This trick was performed via $ (cd t && sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \ -e 's/Master/Main/g' -e 's/retsam/niam/g' \ -- t55[4-9]*.sh t556x*) This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main` for those tests. Note that t5541 uses the reversed `master` name: `retsam`. We replace it by the equivalent for `main`: `niam`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-19 00:44:34 +01:00
main_sha=$(git -C "$server" rev-parse refs/heads/main) &&
upload-pack: optionally allow fetching any sha1 It seems a little silly to do a reachabilty check in the case where we trust the user to access absolutely everything in the repository. Also, it's racy in a distributed system -- perhaps one server advertises a ref, but another has since had a force-push to that ref, and perhaps the two HTTP requests end up directed to these different servers. Signed-off-by: David Turner <dturner@twosigma.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-11 18:23:48 +01:00
git -C "$server" config uploadpack.allowreachablesha1inwant 1 &&
git init --bare test_reachable.git &&
git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
tests: always test fetch of unreachable with v0 Some tests check that fetching an unreachable object fails, but protocol v2 allows such fetches. Unset GIT_TEST_PROTOCOL_VERSION so that these tests are always run using protocol v0. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-25 22:54:08 +01:00
# Some protocol versions (e.g. 2) support fetching
# unadvertised objects, so restrict this test to v0.
test: request GIT_TEST_PROTOCOL_VERSION=0 when appropriate Since 8cbeba0632 (tests: define GIT_TEST_PROTOCOL_VERSION, 2019-02-25), it has been possible to run tests with a newer protocol version by setting the GIT_TEST_PROTOCOL_VERSION envvar to a version number. Tests that assume protocol v0 handle this by explicitly setting GIT_TEST_PROTOCOL_VERSION= or similar constructs like 'test -z "$GIT_TEST_PROTOCOL_VERSION" || return 0' to declare that they only handle the default (v0) protocol. The emphasis there is a bit off: it would be clearer to specify GIT_TEST_PROTOCOL_VERSION=0 to inform the reader that these tests are specifically testing and relying on details of protocol v0. Do so. This way, a reader does not need to know what the default protocol version is, and the tests can continue to work when the default protocol version used by Git advances past v0. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-24 02:01:10 +01:00
test_must_fail env GIT_TEST_PROTOCOL_VERSION=0 \
tests: always test fetch of unreachable with v0 Some tests check that fetching an unreachable object fails, but protocol v2 allows such fetches. Unset GIT_TEST_PROTOCOL_VERSION so that these tests are always run using protocol v0. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-25 22:54:08 +01:00
git -C test_reachable.git fetch origin "$(git rev-parse HEAD)" &&
upload-pack: optionally allow fetching any sha1 It seems a little silly to do a reachabilty check in the case where we trust the user to access absolutely everything in the repository. Also, it's racy in a distributed system -- perhaps one server advertises a ref, but another has since had a force-push to that ref, and perhaps the two HTTP requests end up directed to these different servers. Signed-off-by: David Turner <dturner@twosigma.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-11 18:23:48 +01:00
git -C "$server" config uploadpack.allowanysha1inwant 1 &&
git -C test_reachable.git fetch origin "$(git rev-parse HEAD)"
'
http-backend: spool ref negotiation requests to buffer When http-backend spawns "upload-pack" to do ref negotiation, it streams the http request body to upload-pack, who then streams the http response back to the client as it reads. In theory, git can go full-duplex; the client can consume our response while it is still sending the request. In practice, however, HTTP is a half-duplex protocol. Even if our client is ready to read and write simultaneously, we may have other HTTP infrastructure in the way, including the webserver that spawns our CGI, or any intermediate proxies. In at least one documented case[1], this leads to deadlock when trying a fetch over http. What happens is basically: 1. Apache proxies the request to the CGI, http-backend. 2. http-backend gzip-inflates the data and sends the result to upload-pack. 3. upload-pack acts on the data and generates output over the pipe back to Apache. Apache isn't reading because it's busy writing (step 1). This works fine most of the time, because the upload-pack output ends up in a system pipe buffer, and Apache reads it as soon as it finishes writing. But if both the request and the response exceed the system pipe buffer size, then we deadlock (Apache blocks writing to http-backend, http-backend blocks writing to upload-pack, and upload-pack blocks writing to Apache). We need to break the deadlock by spooling either the input or the output. In this case, it's ideal to spool the input, because Apache does not start reading either stdout _or_ stderr until we have consumed all of the input. So until we do so, we cannot even get an error message out to the client. The solution is fairly straight-forward: we read the request body into an in-memory buffer in http-backend, freeing up Apache, and then feed the data ourselves to upload-pack. But there are a few important things to note: 1. We limit the in-memory buffer to prevent an obvious denial-of-service attack. This is a new hard limit on requests, but it's unlikely to come into play. The default value is 10MB, which covers even the ridiculous 100,000-ref negotation in the included test (that actually caps out just over 5MB). But it's configurable on the off chance that you don't mind spending some extra memory to make even ridiculous requests work. 2. We must take care only to buffer when we have to. For pushes, the incoming packfile may be of arbitrary size, and we should connect the input directly to receive-pack. There's no deadlock problem here, though, because we do not produce any output until the whole packfile has been read. For upload-pack's initial ref advertisement, we similarly do not need to buffer. Even though we may generate a lot of output, there is no request body at all (i.e., it is a GET, not a POST). [1] http://article.gmane.org/gmane.comp.version-control.git/269020 Test-adapted-from: Dennis Kaarsemaker <dennis@kaarsemaker.net> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-05-20 09:37:09 +02:00
test_expect_success EXPENSIVE 'http can handle enormous ref negotiation' '
Merge branch 'jk/http-backend-deadlock-2.2' into jk/http-backend-deadlock-2.3 * jk/http-backend-deadlock-2.2: http-backend: spool ref negotiation requests to buffer t5551: factor out tag creation http-backend: fix die recursion with custom handler
2015-05-26 05:44:04 +02:00
(
cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
create_tags 2001 50000
) &&
http-backend: spool ref negotiation requests to buffer When http-backend spawns "upload-pack" to do ref negotiation, it streams the http request body to upload-pack, who then streams the http response back to the client as it reads. In theory, git can go full-duplex; the client can consume our response while it is still sending the request. In practice, however, HTTP is a half-duplex protocol. Even if our client is ready to read and write simultaneously, we may have other HTTP infrastructure in the way, including the webserver that spawns our CGI, or any intermediate proxies. In at least one documented case[1], this leads to deadlock when trying a fetch over http. What happens is basically: 1. Apache proxies the request to the CGI, http-backend. 2. http-backend gzip-inflates the data and sends the result to upload-pack. 3. upload-pack acts on the data and generates output over the pipe back to Apache. Apache isn't reading because it's busy writing (step 1). This works fine most of the time, because the upload-pack output ends up in a system pipe buffer, and Apache reads it as soon as it finishes writing. But if both the request and the response exceed the system pipe buffer size, then we deadlock (Apache blocks writing to http-backend, http-backend blocks writing to upload-pack, and upload-pack blocks writing to Apache). We need to break the deadlock by spooling either the input or the output. In this case, it's ideal to spool the input, because Apache does not start reading either stdout _or_ stderr until we have consumed all of the input. So until we do so, we cannot even get an error message out to the client. The solution is fairly straight-forward: we read the request body into an in-memory buffer in http-backend, freeing up Apache, and then feed the data ourselves to upload-pack. But there are a few important things to note: 1. We limit the in-memory buffer to prevent an obvious denial-of-service attack. This is a new hard limit on requests, but it's unlikely to come into play. The default value is 10MB, which covers even the ridiculous 100,000-ref negotation in the included test (that actually caps out just over 5MB). But it's configurable on the off chance that you don't mind spending some extra memory to make even ridiculous requests work. 2. We must take care only to buffer when we have to. For pushes, the incoming packfile may be of arbitrary size, and we should connect the input directly to receive-pack. There's no deadlock problem here, though, because we do not produce any output until the whole packfile has been read. For upload-pack's initial ref advertisement, we similarly do not need to buffer. Even though we may generate a lot of output, there is no request body at all (i.e., it is a GET, not a POST). [1] http://article.gmane.org/gmane.comp.version-control.git/269020 Test-adapted-from: Dennis Kaarsemaker <dennis@kaarsemaker.net> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-05-20 09:37:09 +02:00
git -C too-many-refs fetch -q --tags &&
(
cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
create_tags 50001 100000
) &&
git -C too-many-refs fetch -q --tags &&
git -C too-many-refs for-each-ref refs/tags >tags &&
test_line_count = 100000 tags
'
http: support sending custom HTTP headers We introduce a way to send custom HTTP headers with all requests. This allows us, for example, to send an extra token from build agents for temporary access to private repositories. (This is the use case that triggered this patch.) This feature can be used like this: git -c http.extraheader='Secret: sssh!' fetch $URL $REF Note that `curl_easy_setopt(..., CURLOPT_HTTPHEADER, ...)` takes only a single list, overriding any previous call. This means we have to collect _all_ of the headers we want to use into a single list, and feed it to cURL in one shot. Since we already unconditionally set a "pragma" header when initializing the curl handles, we can add our new headers to that list. For callers which override the default header list (like probe_rpc), we provide `http_copy_default_headers()` so they can do the same trick. Big thanks to Jeff King and Junio Hamano for their outstanding help and patient reviews. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-04-27 14:20:37 +02:00
test_expect_success 'custom http headers' '
t5551: make the test for extra HTTP headers more robust To test that extra HTTP headers are passed correctly, t5551 verifies that a fetch succeeds when two required headers are passed, and that the fetch does not succeed when those headers are not passed. However, this test would also succeed if the configuration required only one header. As Apache's configuration is notoriously tricky (this developer frequently requires StackOverflow's help to understand Apache's documentation), especially when still supporting the 2.2 line, let's just really make sure that the test verifies what we want it to verify. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-09 08:19:00 +02:00
test_must_fail git -c http.extraheader="x-magic-two: cadabra" \
fetch "$HTTPD_URL/smart_headers/repo.git" &&
http: support sending custom HTTP headers We introduce a way to send custom HTTP headers with all requests. This allows us, for example, to send an extra token from build agents for temporary access to private repositories. (This is the use case that triggered this patch.) This feature can be used like this: git -c http.extraheader='Secret: sssh!' fetch $URL $REF Note that `curl_easy_setopt(..., CURLOPT_HTTPHEADER, ...)` takes only a single list, overriding any previous call. This means we have to collect _all_ of the headers we want to use into a single list, and feed it to cURL in one shot. Since we already unconditionally set a "pragma" header when initializing the curl handles, we can add our new headers to that list. For callers which override the default header list (like probe_rpc), we provide `http_copy_default_headers()` so they can do the same trick. Big thanks to Jeff King and Junio Hamano for their outstanding help and patient reviews. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-04-27 14:20:37 +02:00
git -c http.extraheader="x-magic-one: abra" \
-c http.extraheader="x-magic-two: cadabra" \
submodule: ensure that -c http.extraheader is heeded To support this developer's use case of allowing build agents token-based access to private repositories, we introduced the http.extraheader feature, allowing extra HTTP headers to be sent along with every HTTP request. This patch verifies that we can configure these extra HTTP headers via the command-line for use with `git submodule update`, too. Example: git -c http.extraheader="Secret: Sauce" submodule update --init Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-10 09:08:56 +02:00
fetch "$HTTPD_URL/smart_headers/repo.git" &&
git update-index --add --cacheinfo 160000,$(git rev-parse HEAD),sub &&
git config -f .gitmodules submodule.sub.path sub &&
git config -f .gitmodules submodule.sub.url \
"$HTTPD_URL/smart_headers/repo.git" &&
git submodule init sub &&
test_must_fail git submodule update sub &&
git -c http.extraheader="x-magic-one: abra" \
-c http.extraheader="x-magic-two: cadabra" \
submodule update sub
http: support sending custom HTTP headers We introduce a way to send custom HTTP headers with all requests. This allows us, for example, to send an extra token from build agents for temporary access to private repositories. (This is the use case that triggered this patch.) This feature can be used like this: git -c http.extraheader='Secret: sssh!' fetch $URL $REF Note that `curl_easy_setopt(..., CURLOPT_HTTPHEADER, ...)` takes only a single list, overriding any previous call. This means we have to collect _all_ of the headers we want to use into a single list, and feed it to cURL in one shot. Since we already unconditionally set a "pragma" header when initializing the curl handles, we can add our new headers to that list. For callers which override the default header list (like probe_rpc), we provide `http_copy_default_headers()` so they can do the same trick. Big thanks to Jeff King and Junio Hamano for their outstanding help and patient reviews. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-04-27 14:20:37 +02:00
'
fetch-pack: unify ref in and out param When a user fetches: - at least one up-to-date ref and at least one non-up-to-date ref, - using HTTP with protocol v0 (or something else that uses the fetch command of a remote helper) some refs might not be updated after the fetch. This bug was introduced in commit 989b8c4452 ("fetch-pack: put shallow info in output parameter", 2018-06-28) which allowed transports to report the refs that they have fetched in a new out-parameter "fetched_refs". If they do so, transport_fetch_refs() makes this information available to its caller. Users of "fetched_refs" rely on the following 3 properties: (1) it is the complete list of refs that was passed to transport_fetch_refs(), (2) it has shallow information (REF_STATUS_REJECT_SHALLOW set if relevant), and (3) it has updated OIDs if ref-in-want was used (introduced after 989b8c4452). In an effort to satisfy (1), whenever transport_fetch_refs() filters the refs sent to the transport, it re-adds the filtered refs to whatever the transport supplies before returning it to the user. However, the implementation in 989b8c4452 unconditionally re-adds the filtered refs without checking if the transport refrained from reporting anything in "fetched_refs" (which it is allowed to do), resulting in an incomplete list, no longer satisfying (1). An earlier effort to resolve this [1] solved the issue by readding the filtered refs only if the transport did not refrain from reporting in "fetched_refs", but after further discussion, it seems that the better solution is to revert the API change that introduced "fetched_refs". This API change was first suggested as part of a ref-in-want implementation that allowed for ref patterns and, thus, there could be drastic differences between the input refs and the refs actually fetched [2]; we eventually decided to only allow exact ref names, but this API change remained even though its necessity was decreased. Therefore, revert this API change by reverting commit 989b8c4452, and make receive_wanted_refs() update the OIDs in the sought array (like how update_shallow() updates shallow information in the sought array) instead. A test is also included to show that the user-visible bug discussed at the beginning of this commit message no longer exists. [1] https://public-inbox.org/git/20180801171806.GA122458@google.com/ [2] https://public-inbox.org/git/86a128c5fb710a41791e7183207c4d64889f9307.1485381677.git.jonathantanmy@google.com/ Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-01 22:13:20 +02:00
test_expect_success 'using fetch command in remote-curl updates refs' '
SERVER="$HTTPD_DOCUMENT_ROOT_PATH/twobranch" &&
rm -rf "$SERVER" client &&
git init "$SERVER" &&
test_commit -C "$SERVER" foo &&
git -C "$SERVER" update-ref refs/heads/anotherbranch foo &&
git clone $HTTPD_URL/smart/twobranch client &&
test_commit -C "$SERVER" bar &&
git -C client -c protocol.version=0 fetch &&
t55[4-9]*: adjust the references to the default branch name "main" This trick was performed via $ (cd t && sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \ -e 's/Master/Main/g' -e 's/retsam/niam/g' \ -- t55[4-9]*.sh t556x*) This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main` for those tests. Note that t5541 uses the reversed `master` name: `retsam`. We replace it by the equivalent for `main`: `niam`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-19 00:44:34 +01:00
git -C "$SERVER" rev-parse main >expect &&
git -C client rev-parse origin/main >actual &&
fetch-pack: unify ref in and out param When a user fetches: - at least one up-to-date ref and at least one non-up-to-date ref, - using HTTP with protocol v0 (or something else that uses the fetch command of a remote helper) some refs might not be updated after the fetch. This bug was introduced in commit 989b8c4452 ("fetch-pack: put shallow info in output parameter", 2018-06-28) which allowed transports to report the refs that they have fetched in a new out-parameter "fetched_refs". If they do so, transport_fetch_refs() makes this information available to its caller. Users of "fetched_refs" rely on the following 3 properties: (1) it is the complete list of refs that was passed to transport_fetch_refs(), (2) it has shallow information (REF_STATUS_REJECT_SHALLOW set if relevant), and (3) it has updated OIDs if ref-in-want was used (introduced after 989b8c4452). In an effort to satisfy (1), whenever transport_fetch_refs() filters the refs sent to the transport, it re-adds the filtered refs to whatever the transport supplies before returning it to the user. However, the implementation in 989b8c4452 unconditionally re-adds the filtered refs without checking if the transport refrained from reporting anything in "fetched_refs" (which it is allowed to do), resulting in an incomplete list, no longer satisfying (1). An earlier effort to resolve this [1] solved the issue by readding the filtered refs only if the transport did not refrain from reporting in "fetched_refs", but after further discussion, it seems that the better solution is to revert the API change that introduced "fetched_refs". This API change was first suggested as part of a ref-in-want implementation that allowed for ref patterns and, thus, there could be drastic differences between the input refs and the refs actually fetched [2]; we eventually decided to only allow exact ref names, but this API change remained even though its necessity was decreased. Therefore, revert this API change by reverting commit 989b8c4452, and make receive_wanted_refs() update the OIDs in the sought array (like how update_shallow() updates shallow information in the sought array) instead. A test is also included to show that the user-visible bug discussed at the beginning of this commit message no longer exists. [1] https://public-inbox.org/git/20180801171806.GA122458@google.com/ [2] https://public-inbox.org/git/86a128c5fb710a41791e7183207c4d64889f9307.1485381677.git.jonathantanmy@google.com/ Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-01 22:13:20 +02:00
test_cmp expect actual
'
fetch: do not list refs if fetching only hashes If only hash literals are given on a "git fetch" command-line, tag following is not requested, and the fetch is done using protocol v2, a list of refs is not required from the remote. Therefore, optimize by invoking transport_get_remote_refs() only if we need the refs. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-27 21:24:07 +02:00
test_expect_success 'fetch by SHA-1 without tag following' '
SERVER="$HTTPD_DOCUMENT_ROOT_PATH/server" &&
rm -rf "$SERVER" client &&
git init "$SERVER" &&
test_commit -C "$SERVER" foo &&
git clone $HTTPD_URL/smart/server client &&
test_commit -C "$SERVER" bar &&
git -C "$SERVER" rev-parse bar >bar_hash &&
git -C client -c protocol.version=0 fetch \
--no-tags origin $(cat bar_hash)
'
http: redact curl h2h3 headers in info With GIT_TRACE_CURL=1 or GIT_CURL_VERBOSE=1, sensitive headers like "Authorization" and "Cookie" get redacted. However, since [1], curl's h2h3 module (invoked when using HTTP/2) also prints headers in its "info", which don't get redacted. For example, echo 'github.com TRUE / FALSE 1698960413304 o foo=bar' >cookiefile && GIT_TRACE_CURL=1 GIT_TRACE_CURL_NO_DATA=1 git \ -c 'http.cookiefile=cookiefile' \ -c 'http.version=' \ ls-remote https://github.com/git/git refs/heads/main 2>output && grep 'cookie' output produces output like: 23:04:16.920495 http.c:678 == Info: h2h3 [cookie: o=foo=bar] 23:04:16.920562 http.c:637 => Send header: cookie: o=<redacted> Teach http.c to check for h2h3 headers in info and redact them using the existing header redaction logic. This fixes the broken redaction logic that we noted in the previous commit, so mark the redaction tests as passing under HTTP2. [1] https://github.com/curl/curl/commit/f8c3724aa90472c0e617ddbbc420aa199971eb77 Helped-by: Jeff King <peff@peff.net> Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-11 23:35:06 +01:00
test_expect_success 'cookies are redacted by default' '
http: support cookie redaction when tracing When using GIT_TRACE_CURL, Git already redacts the "Authorization:" and "Proxy-Authorization:" HTTP headers. Extend this redaction to a user-specified list of cookies, specified through the "GIT_REDACT_COOKIES" environment variable. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-19 01:28:01 +01:00
rm -rf clone &&
echo "Set-Cookie: Foo=1" >cookies &&
echo "Set-Cookie: Bar=2" >>cookies &&
http: redact all cookies, teach GIT_TRACE_REDACT=0 In trace output (when GIT_TRACE_CURL is true), redact the values of all HTTP cookies by default. Now that auth headers (since the implementation of GIT_TRACE_CURL in 74c682d3c6 ("http.c: implement the GIT_TRACE_CURL environment variable", 2016-05-24)) and cookie values (since this commit) are redacted by default in these traces, also allow the user to inhibit these redactions through an environment variable. Since values of all cookies are now redacted by default, GIT_REDACT_COOKIES (which previously allowed users to select individual cookies to redact) now has no effect. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-06-05 23:21:36 +02:00
GIT_TRACE_CURL=true \
http, imap-send: stop using CURLOPT_VERBOSE Whenever GIT_CURL_VERBOSE is set, teach Git to behave as if GIT_TRACE_CURL=1 and GIT_TRACE_CURL_NO_DATA=1 is set, instead of setting CURLOPT_VERBOSE. This is to prevent inadvertent revelation of sensitive data. In particular, GIT_CURL_VERBOSE redacts neither the "Authorization" header nor any cookies specified by GIT_REDACT_COOKIES. Unifying the tracing mechanism also has the future benefit that any improvements to the tracing mechanism will benefit both users of GIT_CURL_VERBOSE and GIT_TRACE_CURL, and we do not need to remember to implement any improvement twice. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-11 19:43:10 +02:00
git -c "http.cookieFile=$(pwd)/cookies" clone \
$HTTPD_URL/smart/repo.git clone 2>err &&
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-23 00:11:53 +02:00
grep -i "Cookie:.*Foo=<redacted>" err &&
grep -i "Cookie:.*Bar=<redacted>" err &&
! grep -i "Cookie:.*Foo=1" err &&
! grep -i "Cookie:.*Bar=2" err
http, imap-send: stop using CURLOPT_VERBOSE Whenever GIT_CURL_VERBOSE is set, teach Git to behave as if GIT_TRACE_CURL=1 and GIT_TRACE_CURL_NO_DATA=1 is set, instead of setting CURLOPT_VERBOSE. This is to prevent inadvertent revelation of sensitive data. In particular, GIT_CURL_VERBOSE redacts neither the "Authorization" header nor any cookies specified by GIT_REDACT_COOKIES. Unifying the tracing mechanism also has the future benefit that any improvements to the tracing mechanism will benefit both users of GIT_CURL_VERBOSE and GIT_TRACE_CURL, and we do not need to remember to implement any improvement twice. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-11 19:43:10 +02:00
'
http: redact all cookies, teach GIT_TRACE_REDACT=0 In trace output (when GIT_TRACE_CURL is true), redact the values of all HTTP cookies by default. Now that auth headers (since the implementation of GIT_TRACE_CURL in 74c682d3c6 ("http.c: implement the GIT_TRACE_CURL environment variable", 2016-05-24)) and cookie values (since this commit) are redacted by default in these traces, also allow the user to inhibit these redactions through an environment variable. Since values of all cookies are now redacted by default, GIT_REDACT_COOKIES (which previously allowed users to select individual cookies to redact) now has no effect. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-06-05 23:21:36 +02:00
test_expect_success 'empty values of cookies are also redacted' '
http, imap-send: stop using CURLOPT_VERBOSE Whenever GIT_CURL_VERBOSE is set, teach Git to behave as if GIT_TRACE_CURL=1 and GIT_TRACE_CURL_NO_DATA=1 is set, instead of setting CURLOPT_VERBOSE. This is to prevent inadvertent revelation of sensitive data. In particular, GIT_CURL_VERBOSE redacts neither the "Authorization" header nor any cookies specified by GIT_REDACT_COOKIES. Unifying the tracing mechanism also has the future benefit that any improvements to the tracing mechanism will benefit both users of GIT_CURL_VERBOSE and GIT_TRACE_CURL, and we do not need to remember to implement any improvement twice. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-11 19:43:10 +02:00
rm -rf clone &&
http: redact all cookies, teach GIT_TRACE_REDACT=0 In trace output (when GIT_TRACE_CURL is true), redact the values of all HTTP cookies by default. Now that auth headers (since the implementation of GIT_TRACE_CURL in 74c682d3c6 ("http.c: implement the GIT_TRACE_CURL environment variable", 2016-05-24)) and cookie values (since this commit) are redacted by default in these traces, also allow the user to inhibit these redactions through an environment variable. Since values of all cookies are now redacted by default, GIT_REDACT_COOKIES (which previously allowed users to select individual cookies to redact) now has no effect. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-06-05 23:21:36 +02:00
echo "Set-Cookie: Foo=" >cookies &&
GIT_TRACE_CURL=true \
http: support cookie redaction when tracing When using GIT_TRACE_CURL, Git already redacts the "Authorization:" and "Proxy-Authorization:" HTTP headers. Extend this redaction to a user-specified list of cookies, specified through the "GIT_REDACT_COOKIES" environment variable. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-19 01:28:01 +01:00
git -c "http.cookieFile=$(pwd)/cookies" clone \
$HTTPD_URL/smart/repo.git clone 2>err &&
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-23 00:11:53 +02:00
grep -i "Cookie:.*Foo=<redacted>" err
http: support cookie redaction when tracing When using GIT_TRACE_CURL, Git already redacts the "Authorization:" and "Proxy-Authorization:" HTTP headers. Extend this redaction to a user-specified list of cookies, specified through the "GIT_REDACT_COOKIES" environment variable. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-19 01:28:01 +01:00
'
http: redact all cookies, teach GIT_TRACE_REDACT=0 In trace output (when GIT_TRACE_CURL is true), redact the values of all HTTP cookies by default. Now that auth headers (since the implementation of GIT_TRACE_CURL in 74c682d3c6 ("http.c: implement the GIT_TRACE_CURL environment variable", 2016-05-24)) and cookie values (since this commit) are redacted by default in these traces, also allow the user to inhibit these redactions through an environment variable. Since values of all cookies are now redacted by default, GIT_REDACT_COOKIES (which previously allowed users to select individual cookies to redact) now has no effect. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-06-05 23:21:36 +02:00
test_expect_success 'GIT_TRACE_REDACT=0 disables cookie redaction' '
http: support cookie redaction when tracing When using GIT_TRACE_CURL, Git already redacts the "Authorization:" and "Proxy-Authorization:" HTTP headers. Extend this redaction to a user-specified list of cookies, specified through the "GIT_REDACT_COOKIES" environment variable. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-19 01:28:01 +01:00
rm -rf clone &&
http: redact all cookies, teach GIT_TRACE_REDACT=0 In trace output (when GIT_TRACE_CURL is true), redact the values of all HTTP cookies by default. Now that auth headers (since the implementation of GIT_TRACE_CURL in 74c682d3c6 ("http.c: implement the GIT_TRACE_CURL environment variable", 2016-05-24)) and cookie values (since this commit) are redacted by default in these traces, also allow the user to inhibit these redactions through an environment variable. Since values of all cookies are now redacted by default, GIT_REDACT_COOKIES (which previously allowed users to select individual cookies to redact) now has no effect. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-06-05 23:21:36 +02:00
echo "Set-Cookie: Foo=1" >cookies &&
echo "Set-Cookie: Bar=2" >>cookies &&
GIT_TRACE_REDACT=0 GIT_TRACE_CURL=true \
http: support cookie redaction when tracing When using GIT_TRACE_CURL, Git already redacts the "Authorization:" and "Proxy-Authorization:" HTTP headers. Extend this redaction to a user-specified list of cookies, specified through the "GIT_REDACT_COOKIES" environment variable. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-19 01:28:01 +01:00
git -c "http.cookieFile=$(pwd)/cookies" clone \
$HTTPD_URL/smart/repo.git clone 2>err &&
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-23 00:11:53 +02:00
grep -i "Cookie:.*Foo=1" err &&
grep -i "Cookie:.*Bar=2" err
http: support cookie redaction when tracing When using GIT_TRACE_CURL, Git already redacts the "Authorization:" and "Proxy-Authorization:" HTTP headers. Extend this redaction to a user-specified list of cookies, specified through the "GIT_REDACT_COOKIES" environment variable. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-19 01:28:01 +01:00
'
http: support omitting data from traces GIT_TRACE_CURL provides a way to debug what is being sent and received over HTTP, with automatic redaction of sensitive information. But it also logs data transmissions, which significantly increases the log file size, sometimes unnecessarily. Add an option "GIT_TRACE_CURL_NO_DATA" to allow the user to omit such data transmissions. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-19 01:28:02 +01:00
test_expect_success 'GIT_TRACE_CURL_NO_DATA prevents data from being traced' '
rm -rf clone &&
GIT_TRACE_CURL=true \
git clone $HTTPD_URL/smart/repo.git clone 2>err &&
grep "=> Send data" err &&
rm -rf clone &&
GIT_TRACE_CURL=true GIT_TRACE_CURL_NO_DATA=1 \
git clone $HTTPD_URL/smart/repo.git clone 2>err &&
! grep "=> Send data" err
'
t5551: test server-side ERR packet When a smart HTTP server sends an error message via pkt-line, we detect the error due to using PACKET_READ_DIE_ON_ERR_PACKET. This case was added by 2d103c31c2 (pack-protocol.txt: accept error packets in any context, 2018-12-29), but not covered by tests. Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-06 20:19:10 +01:00
test_expect_success 'server-side error detected' '
test_must_fail git clone $HTTPD_URL/error_smart/repo.git 2>actual &&
t5551: use 'test_i18ngrep' to check translated output The two tests 'invalid Content-Type rejected' and 'server-side error detected' in 't5551-http-fetch-smart.sh' use "plain" 'grep' to check that 'git clone' failed with the expected error message, but the messages they are checking are translated, and, consequently, these tests fail when the test script is run with GIT_TEST_GETTEXT_POISON enabled. Use 'test_i18ngrep' instead. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-24 14:44:46 +02:00
test_i18ngrep "server-side error" actual
t5551: test server-side ERR packet When a smart HTTP server sends an error message via pkt-line, we detect the error due to using PACKET_READ_DIE_ON_ERR_PACKET. This case was added by 2d103c31c2 (pack-protocol.txt: accept error packets in any context, 2018-12-29), but not covered by tests. Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-06 20:19:10 +01:00
'
t5551: test http interaction with credential helpers We test authentication with http, and we independently test that credential helpers work, but we don't have any tests that cover the two features working together. Let's add two: 1. Make sure that a successful request asks the helper to save the credential. This works as expected. 2. Make sure that a failed request asks the helper to forget the credential. This is marked as expect_failure, as it was recently regressed by 1b0d9545bb (remote-curl: fall back to basic auth if Negotiate fails, 2021-03-22). The symptom here is that the second request should prompt the user, but doesn't. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-18 08:27:36 +02:00
test_expect_success 'http auth remembers successful credentials' '
rm -f .git-credentials &&
test_config credential.helper store &&
# the first request prompts the user...
set_askpass user@host pass@host &&
git ls-remote "$HTTPD_URL/auth/smart/repo.git" >/dev/null &&
expect_askpass both user@host &&
# ...and the second one uses the stored value rather than
# prompting the user.
set_askpass bogus-user bogus-pass &&
git ls-remote "$HTTPD_URL/auth/smart/repo.git" >/dev/null &&
expect_askpass none
'
Revert "remote-curl: fall back to basic auth if Negotiate fails" This reverts commit 1b0d9545bb85912a16b367229d414f55d140d3be. That commit does fix the situation it intended to (avoiding Negotiate even when the credentials were provided in the URL), but it creates a more serious regression: we now never hit the conditional for "we had a username and password, tried them, but the server still gave us a 401". That has two bad effects: 1. we never call credential_reject(), and thus a bogus credential stored by a helper will live on forever 2. we never return HTTP_NOAUTH, so the error message the user gets is "The requested URL returned error: 401", instead of "Authentication failed". Doing this correctly seems non-trivial, as we don't know whether the Negotiate auth was a problem. Since this is a regression in the upcoming v2.23.0 release (for which we're in -rc0), let's revert for now and work on a fix separately. (Note that this isn't a pure revert; the previous commit added a test showing the regression, so we can now flip it to expect_success). Reported-by: Ben Humphreys <behumphreys@atlassian.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-18 08:27:42 +02:00
test_expect_success 'http auth forgets bogus credentials' '
t5551: test http interaction with credential helpers We test authentication with http, and we independently test that credential helpers work, but we don't have any tests that cover the two features working together. Let's add two: 1. Make sure that a successful request asks the helper to save the credential. This works as expected. 2. Make sure that a failed request asks the helper to forget the credential. This is marked as expect_failure, as it was recently regressed by 1b0d9545bb (remote-curl: fall back to basic auth if Negotiate fails, 2021-03-22). The symptom here is that the second request should prompt the user, but doesn't. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-18 08:27:36 +02:00
# seed credential store with bogus values. In real life,
# this would probably come from a password which worked
# for a previous request.
rm -f .git-credentials &&
test_config credential.helper store &&
{
echo "url=$HTTPD_URL" &&
echo "username=bogus" &&
echo "password=bogus"
} | git credential approve &&
# we expect this to use the bogus values and fail, never even
# prompting the user...
set_askpass user@host pass@host &&
test_must_fail git ls-remote "$HTTPD_URL/auth/smart/repo.git" >/dev/null &&
expect_askpass none &&
# ...but now we should have forgotten the bad value, causing
# us to prompt the user again.
set_askpass user@host pass@host &&
git ls-remote "$HTTPD_URL/auth/smart/repo.git" >/dev/null &&
expect_askpass both user@host
'
t5551: test v2-to-v0 http protocol fallback Since we use the v2 protocol by default, the connection of a v2 client to a v2 server is well covered by the test suite. And with the GIT_TEST_PROTOCOL_VERSION knob, we can easily test a v0 client connecting to a v2-aware server (which will then just speak v0). But we have no regular tests that a v2 client, when encountering a non-v2-aware server, will correctly fall back to using v0. In theory this is a job for the cross-version tests in t/interop, but: - they cover only git:// and file:// clones - they are not part of the usual test suite, so nobody ever runs them anyway Since using v2 over http requires configuring the web server to pass along the Git-Protocol header, we can easily create a situation where the server does not respect the v2 probe, and the conversation falls back to v0. This works just fine. This new test is not about fixing any particular bug, but just making sure that the system works (and continues to work) as expected. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-10 16:04:42 +02:00
test_expect_success 'client falls back from v2 to v0 to match server' '
GIT_TRACE_PACKET=$PWD/trace \
GIT_TEST_PROTOCOL_VERSION=2 \
git clone $HTTPD_URL/smart_v0/repo.git repo-v0 &&
# check for v0; there the HEAD symref is communicated in the capability
# line; v2 uses a different syntax on each ref advertisement line
grep symref=HEAD:refs/heads/ trace
'
http: add custom hostname to IP address resolutions Libcurl has a CURLOPT_RESOLVE easy option that allows the result of hostname resolution in the following format to be passed: [+]HOST:PORT:ADDRESS[,ADDRESS] This way, redirects and everything operating against the HOST+PORT will use the provided ADDRESS(s). The following format is also allowed to stop using hostname resolutions that have already been passed: -HOST:PORT See https://curl.se/libcurl/c/CURLOPT_RESOLVE.html for more details. Let's add a corresponding "http.curloptResolve" config option that takes advantage of CURLOPT_RESOLVE. Each value configured for the "http.curloptResolve" key is passed "as is" to libcurl through CURLOPT_RESOLVE, so it should be in one of the above 2 formats. This keeps the implementation simple and makes us consistent with libcurl's CURLOPT_RESOLVE, and with curl's corresponding `--resolve` command line option. The implementation uses CURLOPT_RESOLVE only in get_active_slot() which is called by all the HTTP request sending functions. Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-05-16 10:38:51 +02:00
test_expect_success 'passing hostname resolution information works' '
BOGUS_HOST=gitbogusexamplehost.invalid &&
BOGUS_HTTPD_URL=$HTTPD_PROTO://$BOGUS_HOST:$LIB_HTTPD_PORT &&
test_must_fail git ls-remote "$BOGUS_HTTPD_URL/smart/repo.git" >/dev/null &&
git -c "http.curloptResolve=$BOGUS_HOST:$LIB_HTTPD_PORT:127.0.0.1" ls-remote "$BOGUS_HTTPD_URL/smart/repo.git" >/dev/null
'
t5516: move plaintext-password tests from t5601 and t5516 Commit 6dcbdc0d66 (remote: create fetch.credentialsInUrl config, 2022-06-06) added tests for our handling of passwords in URLs. Since the obvious URL to be affected is git-over-http, the tests use http. However they don't set up a test server; they just try to access https://localhost, assuming it will fail (because the nothing is listening there). This causes some possible problems: - There might be a web server running on localhost, and we do not actually want to connect to that. - The DNS resolver, or the local firewall, might take a substantial amount of time (or forever, whichever comes first) to fail to connect, slowing down the tests cases unnecessarily. - Since there's no server, our tests for "allow" and "warn" still expect the clone/fetch/push operations to fail, even though in the real world we'd expect these to succeed. We scrape stderr to see what happened, but it's not as robust as a more realistic test. Let's instead move these to t5551, which is all about testing http and where we have a real server. That eliminates any issues with contacting a strange URL, and lets the "allow" and "warn" tests confirm that the operation actually succeeds. It's not quite a verbatim move for a few reasons: - we can drop the LIBCURL dependency; it's already part of lib-httpd.sh - we'll use HTTPD_URL_USER_PASS, etc, instead of our fake URL. To avoid repetition, we'll add a few extra variables. - the "https://username:@localhost" test uses a funny URL that lib-httpd.sh doesn't provide. We'll similarly construct it in a variable. Note that we're hard-coding the lib-httpd username here, but t5551 already does that everywhere. - for the "domain:port" test, the URL provided by lib-httpd is fine, since our test server will always be on an exotic port. But we'll confirm in the test that this is so. - since our message-matching is done via grep, I simplified it to use a regex, rather than trying to massage lib-httpd's variables. Arguably this makes it more readable, too, while retaining the bits we care about: the fatal/warning distinction, the "uses plaintext" message, and the fact that the password was redacted. - we'll use the /auth/ path for the repo, which shows that we are indeed making use of the auth information when needed. - we'll also use /smart/; most of these tests could be done via /dumb/ in t5550, but setting up pushes there requires extra effort and dependencies. The smart protocol is what most everyone is using these days anyway. This patch is my own, but I stole the analysis and a few bits of the commit message from a patch by Johannes Schindelin. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-01 03:26:42 +01:00
# here user%40host is the URL-encoded version of user@host,
# which is our intentionally-odd username to catch parsing errors
url_user=$HTTPD_URL_USER/auth/smart/repo.git
url_userpass=$HTTPD_URL_USER_PASS/auth/smart/repo.git
url_userblank=$HTTPD_PROTO://user%40host:@$HTTPD_DEST/auth/smart/repo.git
message="URL .*:<redacted>@.* uses plaintext credentials"
test_expect_success 'clone warns or fails when using username:password' '
test_when_finished "rm -rf attempt*" &&
git -c transfer.credentialsInUrl=allow \
clone $url_userpass attempt1 2>err &&
! grep "$message" err &&
git -c transfer.credentialsInUrl=warn \
clone $url_userpass attempt2 2>err &&
grep "warning: $message" err >warnings &&
t5516/t5601: be less strict about the number of credential warnings It is unclear as to _why_, but under certain circumstances the warning about credentials being passed as part of the URL seems to be swallowed by the `git remote-https` helper in the Windows jobs of Git's CI builds. Since it is not actually important how many times Git prints the warning/error message, as long as it prints it at least once, let's just make the test a bit more lenient and test for the latter instead of the former, which works around these CI issues. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-01 03:26:48 +01:00
test_line_count -ge 1 warnings &&
t5516: move plaintext-password tests from t5601 and t5516 Commit 6dcbdc0d66 (remote: create fetch.credentialsInUrl config, 2022-06-06) added tests for our handling of passwords in URLs. Since the obvious URL to be affected is git-over-http, the tests use http. However they don't set up a test server; they just try to access https://localhost, assuming it will fail (because the nothing is listening there). This causes some possible problems: - There might be a web server running on localhost, and we do not actually want to connect to that. - The DNS resolver, or the local firewall, might take a substantial amount of time (or forever, whichever comes first) to fail to connect, slowing down the tests cases unnecessarily. - Since there's no server, our tests for "allow" and "warn" still expect the clone/fetch/push operations to fail, even though in the real world we'd expect these to succeed. We scrape stderr to see what happened, but it's not as robust as a more realistic test. Let's instead move these to t5551, which is all about testing http and where we have a real server. That eliminates any issues with contacting a strange URL, and lets the "allow" and "warn" tests confirm that the operation actually succeeds. It's not quite a verbatim move for a few reasons: - we can drop the LIBCURL dependency; it's already part of lib-httpd.sh - we'll use HTTPD_URL_USER_PASS, etc, instead of our fake URL. To avoid repetition, we'll add a few extra variables. - the "https://username:@localhost" test uses a funny URL that lib-httpd.sh doesn't provide. We'll similarly construct it in a variable. Note that we're hard-coding the lib-httpd username here, but t5551 already does that everywhere. - for the "domain:port" test, the URL provided by lib-httpd is fine, since our test server will always be on an exotic port. But we'll confirm in the test that this is so. - since our message-matching is done via grep, I simplified it to use a regex, rather than trying to massage lib-httpd's variables. Arguably this makes it more readable, too, while retaining the bits we care about: the fatal/warning distinction, the "uses plaintext" message, and the fact that the password was redacted. - we'll use the /auth/ path for the repo, which shows that we are indeed making use of the auth information when needed. - we'll also use /smart/; most of these tests could be done via /dumb/ in t5550, but setting up pushes there requires extra effort and dependencies. The smart protocol is what most everyone is using these days anyway. This patch is my own, but I stole the analysis and a few bits of the commit message from a patch by Johannes Schindelin. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-01 03:26:42 +01:00
test_must_fail git -c transfer.credentialsInUrl=die \
clone $url_userpass attempt3 2>err &&
grep "fatal: $message" err >warnings &&
t5516/t5601: be less strict about the number of credential warnings It is unclear as to _why_, but under certain circumstances the warning about credentials being passed as part of the URL seems to be swallowed by the `git remote-https` helper in the Windows jobs of Git's CI builds. Since it is not actually important how many times Git prints the warning/error message, as long as it prints it at least once, let's just make the test a bit more lenient and test for the latter instead of the former, which works around these CI issues. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-01 03:26:48 +01:00
test_line_count -ge 1 warnings &&
t5516: move plaintext-password tests from t5601 and t5516 Commit 6dcbdc0d66 (remote: create fetch.credentialsInUrl config, 2022-06-06) added tests for our handling of passwords in URLs. Since the obvious URL to be affected is git-over-http, the tests use http. However they don't set up a test server; they just try to access https://localhost, assuming it will fail (because the nothing is listening there). This causes some possible problems: - There might be a web server running on localhost, and we do not actually want to connect to that. - The DNS resolver, or the local firewall, might take a substantial amount of time (or forever, whichever comes first) to fail to connect, slowing down the tests cases unnecessarily. - Since there's no server, our tests for "allow" and "warn" still expect the clone/fetch/push operations to fail, even though in the real world we'd expect these to succeed. We scrape stderr to see what happened, but it's not as robust as a more realistic test. Let's instead move these to t5551, which is all about testing http and where we have a real server. That eliminates any issues with contacting a strange URL, and lets the "allow" and "warn" tests confirm that the operation actually succeeds. It's not quite a verbatim move for a few reasons: - we can drop the LIBCURL dependency; it's already part of lib-httpd.sh - we'll use HTTPD_URL_USER_PASS, etc, instead of our fake URL. To avoid repetition, we'll add a few extra variables. - the "https://username:@localhost" test uses a funny URL that lib-httpd.sh doesn't provide. We'll similarly construct it in a variable. Note that we're hard-coding the lib-httpd username here, but t5551 already does that everywhere. - for the "domain:port" test, the URL provided by lib-httpd is fine, since our test server will always be on an exotic port. But we'll confirm in the test that this is so. - since our message-matching is done via grep, I simplified it to use a regex, rather than trying to massage lib-httpd's variables. Arguably this makes it more readable, too, while retaining the bits we care about: the fatal/warning distinction, the "uses plaintext" message, and the fact that the password was redacted. - we'll use the /auth/ path for the repo, which shows that we are indeed making use of the auth information when needed. - we'll also use /smart/; most of these tests could be done via /dumb/ in t5550, but setting up pushes there requires extra effort and dependencies. The smart protocol is what most everyone is using these days anyway. This patch is my own, but I stole the analysis and a few bits of the commit message from a patch by Johannes Schindelin. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-01 03:26:42 +01:00
test_must_fail git -c transfer.credentialsInUrl=die \
clone $url_userblank attempt4 2>err &&
grep "fatal: $message" err >warnings &&
t5516/t5601: be less strict about the number of credential warnings It is unclear as to _why_, but under certain circumstances the warning about credentials being passed as part of the URL seems to be swallowed by the `git remote-https` helper in the Windows jobs of Git's CI builds. Since it is not actually important how many times Git prints the warning/error message, as long as it prints it at least once, let's just make the test a bit more lenient and test for the latter instead of the former, which works around these CI issues. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-01 03:26:48 +01:00
test_line_count -ge 1 warnings
t5516: move plaintext-password tests from t5601 and t5516 Commit 6dcbdc0d66 (remote: create fetch.credentialsInUrl config, 2022-06-06) added tests for our handling of passwords in URLs. Since the obvious URL to be affected is git-over-http, the tests use http. However they don't set up a test server; they just try to access https://localhost, assuming it will fail (because the nothing is listening there). This causes some possible problems: - There might be a web server running on localhost, and we do not actually want to connect to that. - The DNS resolver, or the local firewall, might take a substantial amount of time (or forever, whichever comes first) to fail to connect, slowing down the tests cases unnecessarily. - Since there's no server, our tests for "allow" and "warn" still expect the clone/fetch/push operations to fail, even though in the real world we'd expect these to succeed. We scrape stderr to see what happened, but it's not as robust as a more realistic test. Let's instead move these to t5551, which is all about testing http and where we have a real server. That eliminates any issues with contacting a strange URL, and lets the "allow" and "warn" tests confirm that the operation actually succeeds. It's not quite a verbatim move for a few reasons: - we can drop the LIBCURL dependency; it's already part of lib-httpd.sh - we'll use HTTPD_URL_USER_PASS, etc, instead of our fake URL. To avoid repetition, we'll add a few extra variables. - the "https://username:@localhost" test uses a funny URL that lib-httpd.sh doesn't provide. We'll similarly construct it in a variable. Note that we're hard-coding the lib-httpd username here, but t5551 already does that everywhere. - for the "domain:port" test, the URL provided by lib-httpd is fine, since our test server will always be on an exotic port. But we'll confirm in the test that this is so. - since our message-matching is done via grep, I simplified it to use a regex, rather than trying to massage lib-httpd's variables. Arguably this makes it more readable, too, while retaining the bits we care about: the fatal/warning distinction, the "uses plaintext" message, and the fact that the password was redacted. - we'll use the /auth/ path for the repo, which shows that we are indeed making use of the auth information when needed. - we'll also use /smart/; most of these tests could be done via /dumb/ in t5550, but setting up pushes there requires extra effort and dependencies. The smart protocol is what most everyone is using these days anyway. This patch is my own, but I stole the analysis and a few bits of the commit message from a patch by Johannes Schindelin. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-01 03:26:42 +01:00
'
test_expect_success 'clone does not detect username:password when it is https://username@domain:port/' '
test_when_finished "rm -rf attempt1" &&
# we are relying on lib-httpd for url construction, so document our
# assumptions
case "$HTTPD_URL_USER" in
*:[0-9]*) : ok ;;
*) BUG "httpd url does not have port: $HTTPD_URL_USER"
esac &&
git -c transfer.credentialsInUrl=warn clone $url_user attempt1 2>err &&
! grep "uses plaintext credentials" err
'
test_expect_success 'fetch warns or fails when using username:password' '
git -c transfer.credentialsInUrl=allow fetch $url_userpass 2>err &&
! grep "$message" err &&
git -c transfer.credentialsInUrl=warn fetch $url_userpass 2>err &&
grep "warning: $message" err >warnings &&
t5516/t5601: be less strict about the number of credential warnings It is unclear as to _why_, but under certain circumstances the warning about credentials being passed as part of the URL seems to be swallowed by the `git remote-https` helper in the Windows jobs of Git's CI builds. Since it is not actually important how many times Git prints the warning/error message, as long as it prints it at least once, let's just make the test a bit more lenient and test for the latter instead of the former, which works around these CI issues. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-01 03:26:48 +01:00
test_line_count -ge 1 warnings &&
t5516: move plaintext-password tests from t5601 and t5516 Commit 6dcbdc0d66 (remote: create fetch.credentialsInUrl config, 2022-06-06) added tests for our handling of passwords in URLs. Since the obvious URL to be affected is git-over-http, the tests use http. However they don't set up a test server; they just try to access https://localhost, assuming it will fail (because the nothing is listening there). This causes some possible problems: - There might be a web server running on localhost, and we do not actually want to connect to that. - The DNS resolver, or the local firewall, might take a substantial amount of time (or forever, whichever comes first) to fail to connect, slowing down the tests cases unnecessarily. - Since there's no server, our tests for "allow" and "warn" still expect the clone/fetch/push operations to fail, even though in the real world we'd expect these to succeed. We scrape stderr to see what happened, but it's not as robust as a more realistic test. Let's instead move these to t5551, which is all about testing http and where we have a real server. That eliminates any issues with contacting a strange URL, and lets the "allow" and "warn" tests confirm that the operation actually succeeds. It's not quite a verbatim move for a few reasons: - we can drop the LIBCURL dependency; it's already part of lib-httpd.sh - we'll use HTTPD_URL_USER_PASS, etc, instead of our fake URL. To avoid repetition, we'll add a few extra variables. - the "https://username:@localhost" test uses a funny URL that lib-httpd.sh doesn't provide. We'll similarly construct it in a variable. Note that we're hard-coding the lib-httpd username here, but t5551 already does that everywhere. - for the "domain:port" test, the URL provided by lib-httpd is fine, since our test server will always be on an exotic port. But we'll confirm in the test that this is so. - since our message-matching is done via grep, I simplified it to use a regex, rather than trying to massage lib-httpd's variables. Arguably this makes it more readable, too, while retaining the bits we care about: the fatal/warning distinction, the "uses plaintext" message, and the fact that the password was redacted. - we'll use the /auth/ path for the repo, which shows that we are indeed making use of the auth information when needed. - we'll also use /smart/; most of these tests could be done via /dumb/ in t5550, but setting up pushes there requires extra effort and dependencies. The smart protocol is what most everyone is using these days anyway. This patch is my own, but I stole the analysis and a few bits of the commit message from a patch by Johannes Schindelin. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-01 03:26:42 +01:00
test_must_fail git -c transfer.credentialsInUrl=die \
fetch $url_userpass 2>err &&
grep "fatal: $message" err >warnings &&
t5516/t5601: be less strict about the number of credential warnings It is unclear as to _why_, but under certain circumstances the warning about credentials being passed as part of the URL seems to be swallowed by the `git remote-https` helper in the Windows jobs of Git's CI builds. Since it is not actually important how many times Git prints the warning/error message, as long as it prints it at least once, let's just make the test a bit more lenient and test for the latter instead of the former, which works around these CI issues. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-01 03:26:48 +01:00
test_line_count -ge 1 warnings &&
t5516: move plaintext-password tests from t5601 and t5516 Commit 6dcbdc0d66 (remote: create fetch.credentialsInUrl config, 2022-06-06) added tests for our handling of passwords in URLs. Since the obvious URL to be affected is git-over-http, the tests use http. However they don't set up a test server; they just try to access https://localhost, assuming it will fail (because the nothing is listening there). This causes some possible problems: - There might be a web server running on localhost, and we do not actually want to connect to that. - The DNS resolver, or the local firewall, might take a substantial amount of time (or forever, whichever comes first) to fail to connect, slowing down the tests cases unnecessarily. - Since there's no server, our tests for "allow" and "warn" still expect the clone/fetch/push operations to fail, even though in the real world we'd expect these to succeed. We scrape stderr to see what happened, but it's not as robust as a more realistic test. Let's instead move these to t5551, which is all about testing http and where we have a real server. That eliminates any issues with contacting a strange URL, and lets the "allow" and "warn" tests confirm that the operation actually succeeds. It's not quite a verbatim move for a few reasons: - we can drop the LIBCURL dependency; it's already part of lib-httpd.sh - we'll use HTTPD_URL_USER_PASS, etc, instead of our fake URL. To avoid repetition, we'll add a few extra variables. - the "https://username:@localhost" test uses a funny URL that lib-httpd.sh doesn't provide. We'll similarly construct it in a variable. Note that we're hard-coding the lib-httpd username here, but t5551 already does that everywhere. - for the "domain:port" test, the URL provided by lib-httpd is fine, since our test server will always be on an exotic port. But we'll confirm in the test that this is so. - since our message-matching is done via grep, I simplified it to use a regex, rather than trying to massage lib-httpd's variables. Arguably this makes it more readable, too, while retaining the bits we care about: the fatal/warning distinction, the "uses plaintext" message, and the fact that the password was redacted. - we'll use the /auth/ path for the repo, which shows that we are indeed making use of the auth information when needed. - we'll also use /smart/; most of these tests could be done via /dumb/ in t5550, but setting up pushes there requires extra effort and dependencies. The smart protocol is what most everyone is using these days anyway. This patch is my own, but I stole the analysis and a few bits of the commit message from a patch by Johannes Schindelin. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-01 03:26:42 +01:00
test_must_fail git -c transfer.credentialsInUrl=die \
fetch $url_userblank 2>err &&
grep "fatal: $message" err >warnings &&
t5516/t5601: be less strict about the number of credential warnings It is unclear as to _why_, but under certain circumstances the warning about credentials being passed as part of the URL seems to be swallowed by the `git remote-https` helper in the Windows jobs of Git's CI builds. Since it is not actually important how many times Git prints the warning/error message, as long as it prints it at least once, let's just make the test a bit more lenient and test for the latter instead of the former, which works around these CI issues. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-01 03:26:48 +01:00
test_line_count -ge 1 warnings
t5516: move plaintext-password tests from t5601 and t5516 Commit 6dcbdc0d66 (remote: create fetch.credentialsInUrl config, 2022-06-06) added tests for our handling of passwords in URLs. Since the obvious URL to be affected is git-over-http, the tests use http. However they don't set up a test server; they just try to access https://localhost, assuming it will fail (because the nothing is listening there). This causes some possible problems: - There might be a web server running on localhost, and we do not actually want to connect to that. - The DNS resolver, or the local firewall, might take a substantial amount of time (or forever, whichever comes first) to fail to connect, slowing down the tests cases unnecessarily. - Since there's no server, our tests for "allow" and "warn" still expect the clone/fetch/push operations to fail, even though in the real world we'd expect these to succeed. We scrape stderr to see what happened, but it's not as robust as a more realistic test. Let's instead move these to t5551, which is all about testing http and where we have a real server. That eliminates any issues with contacting a strange URL, and lets the "allow" and "warn" tests confirm that the operation actually succeeds. It's not quite a verbatim move for a few reasons: - we can drop the LIBCURL dependency; it's already part of lib-httpd.sh - we'll use HTTPD_URL_USER_PASS, etc, instead of our fake URL. To avoid repetition, we'll add a few extra variables. - the "https://username:@localhost" test uses a funny URL that lib-httpd.sh doesn't provide. We'll similarly construct it in a variable. Note that we're hard-coding the lib-httpd username here, but t5551 already does that everywhere. - for the "domain:port" test, the URL provided by lib-httpd is fine, since our test server will always be on an exotic port. But we'll confirm in the test that this is so. - since our message-matching is done via grep, I simplified it to use a regex, rather than trying to massage lib-httpd's variables. Arguably this makes it more readable, too, while retaining the bits we care about: the fatal/warning distinction, the "uses plaintext" message, and the fact that the password was redacted. - we'll use the /auth/ path for the repo, which shows that we are indeed making use of the auth information when needed. - we'll also use /smart/; most of these tests could be done via /dumb/ in t5550, but setting up pushes there requires extra effort and dependencies. The smart protocol is what most everyone is using these days anyway. This patch is my own, but I stole the analysis and a few bits of the commit message from a patch by Johannes Schindelin. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-01 03:26:42 +01:00
'
test_expect_success 'push warns or fails when using username:password' '
git -c transfer.credentialsInUrl=allow push $url_userpass 2>err &&
! grep "$message" err &&
git -c transfer.credentialsInUrl=warn push $url_userpass 2>err &&
grep "warning: $message" err >warnings &&
test_must_fail git -c transfer.credentialsInUrl=die \
push $url_userpass 2>err &&
grep "fatal: $message" err >warnings &&
t5516/t5601: be less strict about the number of credential warnings It is unclear as to _why_, but under certain circumstances the warning about credentials being passed as part of the URL seems to be swallowed by the `git remote-https` helper in the Windows jobs of Git's CI builds. Since it is not actually important how many times Git prints the warning/error message, as long as it prints it at least once, let's just make the test a bit more lenient and test for the latter instead of the former, which works around these CI issues. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-01 03:26:48 +01:00
test_line_count -ge 1 warnings
t5516: move plaintext-password tests from t5601 and t5516 Commit 6dcbdc0d66 (remote: create fetch.credentialsInUrl config, 2022-06-06) added tests for our handling of passwords in URLs. Since the obvious URL to be affected is git-over-http, the tests use http. However they don't set up a test server; they just try to access https://localhost, assuming it will fail (because the nothing is listening there). This causes some possible problems: - There might be a web server running on localhost, and we do not actually want to connect to that. - The DNS resolver, or the local firewall, might take a substantial amount of time (or forever, whichever comes first) to fail to connect, slowing down the tests cases unnecessarily. - Since there's no server, our tests for "allow" and "warn" still expect the clone/fetch/push operations to fail, even though in the real world we'd expect these to succeed. We scrape stderr to see what happened, but it's not as robust as a more realistic test. Let's instead move these to t5551, which is all about testing http and where we have a real server. That eliminates any issues with contacting a strange URL, and lets the "allow" and "warn" tests confirm that the operation actually succeeds. It's not quite a verbatim move for a few reasons: - we can drop the LIBCURL dependency; it's already part of lib-httpd.sh - we'll use HTTPD_URL_USER_PASS, etc, instead of our fake URL. To avoid repetition, we'll add a few extra variables. - the "https://username:@localhost" test uses a funny URL that lib-httpd.sh doesn't provide. We'll similarly construct it in a variable. Note that we're hard-coding the lib-httpd username here, but t5551 already does that everywhere. - for the "domain:port" test, the URL provided by lib-httpd is fine, since our test server will always be on an exotic port. But we'll confirm in the test that this is so. - since our message-matching is done via grep, I simplified it to use a regex, rather than trying to massage lib-httpd's variables. Arguably this makes it more readable, too, while retaining the bits we care about: the fatal/warning distinction, the "uses plaintext" message, and the fact that the password was redacted. - we'll use the /auth/ path for the repo, which shows that we are indeed making use of the auth information when needed. - we'll also use /smart/; most of these tests could be done via /dumb/ in t5550, but setting up pushes there requires extra effort and dependencies. The smart protocol is what most everyone is using these days anyway. This patch is my own, but I stole the analysis and a few bits of the commit message from a patch by Johannes Schindelin. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-01 03:26:42 +01:00
'
t5541: simplify and move "no empty path components" test Commit 9ee6bcd398 (t5541-http-push: add test for URLs with trailing slash, 2010-04-08) added a test that clones a URL with a trailing slash, and confirms that we don't send a doubled slash (like "$url//info/refs") to the server. But this test makes no sense in t5541, which is about pushing. It should have been added in t5551. Let's move it there. But putting it at the end is tricky, since it checks the entire contents of the Apache access log. We could get around this by clearing the log before our test. But there's an even simpler solution: just make sure no doubled slashes appear in the log (fortunately, "http://" does not appear in the log itself). As a bonus, this also lets us drop the check for the v0 protocol (which is otherwise necessary since v2 makes multiple requests, and check_access_log insists on exactly matching the number of requests, even though we don't care about that here). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23 11:51:35 +01:00
test_expect_success 'no empty path components' '
# In the URL, add a trailing slash, and see if git appends yet another
# slash.
git clone $HTTPD_URL/smart/repo.git/ clone-with-slash &&
strip_access_log >log &&
! grep "//" log
'
test smart http fetch and push The top level directory "/smart/" of the test Apache server is mapped through our git-http-backend CGI, but uses the same underlying repository space as the server's document root. This is the most simple installation possible. Server logs are checked to verify the client has accessed only the smart URLs during the test. During fetch testing the headers are also logged from libcurl to ensure we are making a reasonably sane HTTP request, and getting back reasonably sane response headers from the CGI. When validating the request headers used during smart fetch we munge away the actual Content-Length and replace it with the placeholder "xxx". This avoids unnecessary varability in the test caused by an unrelated change in the requested capabilities in the first want line of the request. However, we still want to look for and verify that Content-Length was used, because smaller payloads should be using Content-Length and not "Transfer-Encoding: chunked". When validating the server response headers we must discard both Content-Length and Transfer-Encoding, as Apache2 can use either format to return our response. During development of this test I observed Apache returning both forms, depending on when the processes got CPU time. If our CGI returned the pack data quickly, Apache just buffered the whole thing and returned a Content-Length. If our CGI took just a bit too long to complete, Apache flushed its buffer and instead used "Transfer-Encoding: chunked". Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-10-31 01:47:47 +01:00
test_done
Reference in New Issue Copy Permalink