| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
httpd tests saw occasional breakage due to the way its access log
gets inspected by the tests, which has been updated to make them
less flaky.
* sg/httpd-test-unflake:
t/lib-httpd: avoid occasional failures when checking access.log
t/lib-httpd: add the strip_access_log() helper function
t5541: clean up truncating access log
|
|
The last test of 't5561-http-backend.sh', 'server request log matches
test results' may fail occasionally, because the order of entries in
Apache's access log doesn't match the order of requests sent in the
previous tests, although all the right requests are there. I saw it
fail on Travis CI five times in the span of about half a year, when
the order of two subsequent requests was flipped, and could trigger
the failure with a modified Git. However, I was unable to trigger it
with stock Git on my machine. Three tests in
't5541-http-push-smart.sh' and 't5551-http-fetch-smart.sh' check
requests in the log the same way, so they might be prone to a similar
occasional failure as well.
When a test sends a HTTP request, it can continue execution after
'git-http-backend' fulfilled that request, but Apache writes the
corresponding access log entry only after 'git-http-backend' exited.
Some time inevitably passes between fulfilling the request and writing
the log entry, and, under unfavourable circumstances, enough time
might pass for the subsequent request to be sent and fulfilled by a
different Apache thread or process, and then Apache writes access log
entries racily.
This effect can be exacerbated by adding a bit of variable delay after
the request is fulfilled but before 'git-http-backend' exits, e.g.
like this:
diff --git a/http-backend.c b/http-backend.c
index f3dc218b2..bbf4c125b 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -709,5 +709,7 @@ int cmd_main(int argc, const char **argv)
max_request_buffer);
cmd->imp(&hdr, cmd_arg);
+ if (getpid() % 2)
+ sleep(1);
return 0;
}
This delay considerably increases the chances of log entries being
written out of order, and in turn makes t5561's last test fail almost
every time. Alas, it doesn't seem to be enough to trigger a similar
failure in t5541 and t5551.
So, since we can't just rely on the order of access log entries always
corresponding the order of requests, make checking the access log more
deterministic by sorting (simply lexicographically) both the stripped
access log entries and the expected entries before the comparison with
'test_cmp'. This way the order of log entries won't matter and
occasional out-of-order entries won't trigger a test failure, but the
comparison will still notice any unexpected or missing log entries.
OTOH, this sorting will make it harder to identify from which test an
unexpected log entry came from or which test's request went missing.
Therefore, in case of an error include the comparison of the unsorted
log enries in the test output as well.
And since all this should be performed in four tests in three test
scripts, put this into a new helper function 'check_access_log' in
't/lib-httpd.sh'.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Four tests in three httpd-related test scripts check the contents of
Apache's 'access.log', and they all do so by running 'sed' with the
exact same script consisting of four s/// commands to strip
uninteresting log fields and to vertically align the requested URLs.
Extract this into a common helper function 'strip_access_log' in
'lib-httpd.sh', and use it in all of those tests.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
In the second test of 't5541-http-push-smart.sh', 'no empty path
components' we truncate Apache's access log by running:
echo >.../access.log
There are two issues with this approach:
- This doesn't leave an empty file behind, like a proper truncation
would, but a file with a lone newline in it. Consequently, a
later test checking the log's contents must consider this improper
truncation and include an empty line in the expected content.
- This truncation is done in the middle of the test, because,
quoting the in-code comment, "we do this [truncation] before the
actual comparison to ensure the log is cleared" even when
subsequent 'test_cmp' fails. Alas, this is not quite robust
enough, as it is conceivable that 'git clone' fails after already
having sent a request, in which case the access log would not be
truncated and would leave stray log entries behind.
Since there is no need for that newline at all, drop the 'echo' from
the truncation and adjust the expected content accordingly.
Furthermore, make sure that the truncation is performed no matter
whether and how 'git clone' fails unexpectedly by specifying it as a
'test_when_finished' command.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
This actually only tests whether the push errors/hints are colored if
the respective color.* config settings are `always`, but in the regular
case they default to `auto` (in which case we color the messages when
stderr is connected to an interactive terminal), therefore these tests
should suffice.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The test 'push --no-progress silences progress but not status' runs
'test_i18ngrep' without specifying a filename parameter. This has
remained unnoticed since its introduction in e304aeba2 (t5541: test
more combinations of --progress, 2012-05-01), because that
'test_i18ngrep' is supposed to check that the given pattern is not
present in its input, and of course it won't find that pattern if its
input is empty (as it comes from /dev/null). This also means that
this test could miss a potential breakage of 'git push --no-progress'.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
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>
|
|
"git fetch http://user:pass@host/repo..." scrubbed the userinfo
part, but "git push" didn't.
* jk/push-scrub-url:
t5541: fix url scrubbing test when GPG is not set
push: anonymize URL in status output
|
|
When the GPG prereq is not set, we do not run test 34. That
test changes the directory of the test script as a side
effect (something we usually frown on, but which matches the
style of the rest of this script). When test 35 (the
url-scrubbing test) runs, it expects to be in the directory
from test 34. If it's not, the test fails; we are in a
different sub-repo, our test-commit is built on a different
history, and the push becomes a non-fast-forward.
We can fix this by unconditionally moving to the directory
we expect (again, against our usual style but matching how
the rest of the script operates).
As an additional protection, let's also switch from "make a
new commit and push to master" to just "push to a new
branch". We don't care about the branch name; we just want
_some_ ref update to trigger the status output. Pushing to a
new branch is less likely to run into problems with
force-updates, changing the checked-out branch, etc.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Commit 47abd85 (fetch: Strip usernames from url's before
storing them, 2009-04-17) taught fetch to anonymize URLs.
The primary purpose there was to avoid sticking passwords in
merge-commit messages, but as a side effect, we also avoid
printing them to stderr.
The push side does not have the merge-commit problem, but it
probably should avoid printing them to stderr. We can reuse
the same anonymizing function.
Note that for this to come up, the credentials would have to
appear either on the command line or in a git config file,
neither of which is particularly secure. So people _should_
be switching to using credential helpers instead, which
makes this problem go away. But that's no excuse not to
improve the situation for people who for whatever reason end
up using credentials embedded in the URL.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Use test_i18n* functions for testing text already marked for
translation.
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Test clean-up.
* jk/skip-http-tests-under-no-curl:
tests: skip dav http-push tests under NO_EXPAT=NoThanks
t/lib-httpd.sh: skip tests if NO_CURL is defined
|
|
Test clean-up.
* jk/skip-http-tests-under-no-curl:
tests: skip dav http-push tests under NO_EXPAT=NoThanks
t/lib-httpd.sh: skip tests if NO_CURL is defined
|
|
If we built git without curl, we can't actually test against
an http server. In fact, all of the test scripts which
include lib-httpd.sh already perform this check, with one
exception: t5540. For those scripts, this is a noop, and for
t5540, this is a bugfix (it used to fail when built with
NO_CURL, though it could go unnoticed if you had a stale
git-remote-https in your build directory).
Noticed-by: Junio C Hamano <junio@pobox.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
People often forget to chain the commands in their test together
with &&, leaving a failure from an earlier command in the test go
unnoticed. The new GIT_TEST_CHAIN_LINT mechanism allows you to
catch such a mistake more easily.
* jk/test-chain-lint: (36 commits)
t9001: drop save_confirm helper
t0020: use test_* helpers instead of hand-rolled messages
t: simplify loop exit-code status variables
t: fix some trivial cases of ignored exit codes in loops
t7701: fix ignored exit code inside loop
t3305: fix ignored exit code inside loop
t0020: fix ignored exit code inside loops
perf-lib: fix ignored exit code inside loop
t6039: fix broken && chain
t9158, t9161: fix broken &&-chain in git-svn tests
t9104: fix test for following larger parents
t4104: drop hand-rolled error reporting
t0005: fix broken &&-chains
t7004: fix embedded single-quotes
t0050: appease --chain-lint
t9001: use test_when_finished
t4117: use modern test_* helpers
t6034: use modern test_* helpers
t1301: use modern test_* helpers
t0020: use modern test_* helpers
...
|
|
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>
|
|
We use this to test http pushing with a restricted
commandline. Other scripts (like t5551, which does http
fetching) will want to use it, too.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Allow "git push" request to be signed, so that it can be verified and
audited, using the GPG signature of the person who pushed, that the
tips of branches at a public repository really point the commits
the pusher wanted to, without having to "trust" the server.
* jc/push-cert: (24 commits)
receive-pack::hmac_sha1(): copy the entire SHA-1 hash out
signed push: allow stale nonce in stateless mode
signed push: teach smart-HTTP to pass "git push --signed" around
signed push: fortify against replay attacks
signed push: add "pushee" header to push certificate
signed push: remove duplicated protocol info
send-pack: send feature request on push-cert packet
receive-pack: GPG-validate push certificates
push: the beginning of "git push --signed"
pack-protocol doc: typofix for PKT-LINE
gpg-interface: move parse_signature() to where it should be
gpg-interface: move parse_gpg_output() to where it should be
send-pack: clarify that cmds_sent is a boolean
send-pack: refactor inspecting and resetting status and sending commands
send-pack: rename "new_refs" to "need_pack_data"
receive-pack: factor out capability string generation
send-pack: factor out capability string generation
send-pack: always send capabilities
send-pack: refactor decision to send update per ref
send-pack: move REF_STATUS_REJECT_NODELETE logic a bit higher
...
|
|
When operating with the stateless RPC mode, we will receive a nonce
issued by another instance of us that advertised our capability and
refs some time ago. Update the logic to check received nonce to
detect this case, compute how much time has passed since the nonce
was issued and report the status with a new environment variable
GIT_PUSH_CERT_NONCE_SLOP to the hooks.
GIT_PUSH_CERT_NONCE_STATUS will report "SLOP" in such a case. The
hooks are free to decide how large a slop it is willing to accept.
Strictly speaking, the "nonce" is not really a "nonce" anymore in
the stateless RPC mode, as it will happily take any "nonce" issued
by it (which is protected by HMAC and its secret key) as long as it
is fresh enough. The degree of this security degradation, relative
to the native protocol, is about the same as the "we make sure that
the 'git push' decided to update our refs with new objects based on
the freshest observation of our refs by making sure the values they
claim the original value of the refs they ask us to update exactly
match the current state" security is loosened to accomodate the
stateless RPC mode in the existing code without this series, so
there is no need for those who are already using smart HTTP to push
to their repositories to be alarmed any more than they already are.
In addition, the server operator can set receive.certnonceslop
configuration variable to specify how stale a nonce can be (in
seconds). When this variable is set, and if the nonce received in
the certificate that passes the HMAC check was less than that many
seconds old, hooks are given "OK" in GIT_PUSH_CERT_NONCE_STATUS
(instead of "SLOP") and the received nonce value is given in
GIT_PUSH_CERT_NONCE, which makes it easier for a simple-minded
hook to check if the certificate we received is recent enough.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The "--signed" option received by "git push" is first passed to the
transport layer, which the native transport directly uses to notice
that a push certificate needs to be sent. When the transport-helper
is involved, however, the option needs to be told to the helper with
set_helper_option(), and the helper needs to take necessary action.
For the smart-HTTP helper, the "necessary action" involves spawning
the "git send-pack" subprocess with the "--signed" option.
Once the above all gets wired in, the smart-HTTP transport now can
use the push certificate mechanism to authenticate its pushes.
Add a test that is modeled after tests for the native transport in
t5534-push-signed.sh to t5541-http-push-smart.sh. Update the test
Apache configuration to pass GNUPGHOME environment variable through.
As PassEnv would trigger warnings for an environment variable that
is not set, export it from test-lib.sh set to a harmless value when
GnuPG is not being used in the tests.
Note that the added test is deliberately loose and does not check
the nonce in this step. This is because the stateless RPC mode is
inevitably flaky and a nonce that comes back in the actual push
processing is one issued by a different process; if the two
interactions with the server crossed a second boundary, the nonces
will not match and such a check will fail. A later patch in the
series will work around this shortcoming.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Pushing a large number of refs works over most transports,
because we implement send-pack as an internal function.
However, it can sometimes fail when pushing over http,
because we have to spawn "git send-pack --stateless-rpc" to
do the heavy lifting, and we pass each refspec on the
command line. This can cause us to overflow the OS limits on
the size of the command line for a large push.
We can solve this by giving send-pack a --stdin option and
using it from remote-curl. We already dealt with this on
the fetch-pack side in 078b895 (fetch-pack: new --stdin
option to read refs from stdin, 2012-04-02). The stdin
option (and in particular, its use of packet-lines for
stateless-rpc input) is modeled after that solution.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Make clear which one is for dumb protocol, which one is for smart from
their file name.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
Junio C Hamano
SZEDER Gábor
Johannes Schindelin
Elia Pinto
Jeff King
Vasco Almeida
Nguyễn Thái Ngọc Duy