refs: write packed_refs file using stdio

We write each line of a new packed-refs file individually
using a write() syscall (and sometimes 2, if the ref is
peeled). Since each line is only about 50-100 bytes long,
this creates a lot of system call overhead.

We can instead open a stdio handle around our descriptor and
use fprintf to write to it. The extra buffering is not a
problem for us, because nobody will read our new packed-refs
file until we call commit_lock_file (by which point we have
flushed everything).

On a pathological repository with 8.5 million refs, this
dropped the time to run `git pack-refs` from 20s to 6s.

Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Refactor type_from_string() to allow continuing after detecting an error

In the next commits, we will enhance the fsck_tag() function to check
tag objects more thoroughly. To this end, we need a function to verify
that a given string is a valid object type, but that does not die() in
the negative case.

While at it, prepare type_from_string() for counted strings, i.e. strings
with an explicitly specified length rather than a NUL termination.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
fsck_object(): allow passing object data separately from the object itself

When fsck'ing an incoming pack, we need to fsck objects that cannot be
read via read_sha1_file() because they are not local yet (and might even
be rejected if transfer.fsckobjects is set to 'true').

For commits, there is a hack in place: we basically cache commit
objects' buffers anyway, but the same is not true, say, for tag objects.

By refactoring fsck_object() to take the object buffer and size as
optional arguments -- optional, because we still fall back to the
previous method to look at the cached commit objects if the caller
passes NULL -- we prepare the machinery for the upcoming handling of tag
objects.

The assumption that such buffers are inherently NUL terminated is now
wrong, of course, hence we pass the size of the buffer so that we can
add a sanity check later, to prevent running past the end of the buffer.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make sure fsck_commit_buffer() does not run out of the buffer

So far, we assumed that the buffer is NUL terminated, but this is not
a safe assumption, now that we opened the fsck_object() API to pass a
buffer directly.

So let's make sure that there is at least an empty line in the buffer.
That way, our checks would fail if the empty line was encountered
prematurely, and consequently we can get away with the current string
comparisons even with non-NUL-terminated buffers are passed to
fsck_object().

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
fsck: check tag objects' headers

We inspect commit objects pretty much in detail in git-fsck, but we just
glanced over the tag objects. Let's be stricter.

Since we do not want to limit 'tag' lines unduly, values that would fail
the refname check only result in warnings, not errors.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
hash-object: reduce file-scope statics

Most of the knobs that affect helper functions called from
cmd_hash_object() were passed to them as parameters already, and the
only effect of having them as file-scope statics was to make the
reader wonder if the parameters are hiding the file-scope global
values by accident.  Adjust their initialisation and make them
function-local variables.

The only exception was no_filters hash_stdin_paths() peeked from the
file-scope global, which was converted to a parameter to the helper
function.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
hash-object: pass 'write_object' as a flag

Instead of forcing callers of lower level functions write
(write_object ? HASH_WRITE_OBJECT : 0), prepare the flag to be
passed down in the callchain from the command line parser.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add regression tests for stricter tag fsck'ing

The intent of the new test case is to catch general breakages in
the fsck_tag() function, not so much to test it extensively, trying to
strike the proper balance between thoroughness and speed.

While it *would* have been nice to test the code path where fsck_object()
encounters an invalid tag object, this is not possible using git fsck: tag
objects are parsed already before fsck'ing (and the parser already fails
upon such objects).

Even worse: we would not even be able write out invalid tag objects
because git hash-object parses those objects, too, unless we resorted to
really ugly hacks such as using something like this in the unit tests
(essentially depending on Perl *and* Compress::Zlib):

	hash_invalid_object () {
		contents="$(printf '%s %d\0%s' "$1" ${#2} "$2")" &&
		sha1=$(echo "$contents" | test-sha1) &&
		suffix=${sha1#??} &&
		mkdir -p .git/objects/${sha1%$suffix} &&
		echo "$contents" |
		perl -MCompress::Zlib -e 'undef $/; print compress(<>)' \
			> .git/objects/${sha1%$suffix}/$suffix &&
		echo $sha1
	}

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
hash-object: add --literally option

This allows "hash-object --stdin" to just hash any garbage into a
"loose object" that may not pass the standard object parsing check
or fsck, so that different kind of corrupt objects we may encounter
in the field can be imitated in our test suite.  That would in turn
allow us to test features that catch these corrupt objects.

Note that "cat-file" may need to learn "--literally" option to allow
us peek into a truly broken object.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make sure that index-pack --strict checks tag objects

One of the most important use cases for the strict tag object checking
is when transfer.fsckobjects is set to true to catch invalid objects
early on. This new regression test essentially tests the same code path
by directly calling 'index-pack --strict' on a pack containing an
tag object without a 'tagger' line.

Technically, this test is not enough: it only exercises a code path that
*warns*, not one that *fails*. The reason is that hash-object and
pack-objects both insist on parsing the tag objects and would fail on
invalid tag objects at this time.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Merge branch 'js/fsck-tag-validation' into HEAD

* js/fsck-tag-validation:
  Make sure that index-pack --strict checks tag objects
  Add regression tests for stricter tag fsck'ing
  fsck: check tag objects' headers
  Make sure fsck_commit_buffer() does not run out of the buffer
  fsck_object(): allow passing object data separately from the object itself
  Refactor type_from_string() to allow continuing after detecting an error
t1450: make sure fsck detects a malformed tagger line

With "hash-object --literally", write a tag object that is not
supposed to pass one of the new checks added to "fsck", and make
sure that the new check catches the breakage.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
refs: speed up is_refname_available

Our filesystem ref storage does not allow D/F conflicts; so
if "refs/heads/a/b" exists, we do not allow "refs/heads/a"
to exist (and vice versa). This falls out naturally for
loose refs, where the filesystem enforces the condition. But
for packed-refs, we have to make the check ourselves.

We do so by iterating over the entire packed-refs namespace
and checking whether each name creates a conflict. If you
have a very large number of refs, this is quite inefficient,
as you end up doing a large number of comparisons with
uninteresting bits of the ref tree (e.g., we know that all
of "refs/tags" is uninteresting in the example above, yet we
check each entry in it).

Instead, let's take advantage of the fact that we have the
packed refs stored as a trie of ref_entry structs. We can
find each component of the proposed refname as we walk
through the trie, checking for D/F conflicts as we go. For a
refname of depth N (i.e., 4 in the above example), we only
have to visit N nodes. And at each visit, we can binary
search the M names at that level, for a total complexity of
O(N lg M). ("M" is different at each level, of course, but
we can take the worst-case "M" as a bound).

In a pathological case of fetching 30,000 fresh refs into a
repository with 8.5 million refs, this dropped the time to
run "git fetch" from tens of minutes to ~30s.

This may also help smaller cases in which we check against
loose refs (which we do when renaming a ref), as we may
avoid a disk access for unrelated loose directories.

Note that the tests we add appear at first glance to be
redundant with what is already in t3210. However, the early
tests are not robust; they are run with reflogs turned on,
meaning that we are not actually testing
is_refname_available at all! The operations will still fail
because the reflogs will hit D/F conflicts in the
filesystem. To get a true test, we must turn off reflogs
(but we don't want to do so for the entire script, because
the point of turning them on was to cover some other cases).

Reviewed-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
prune-packed: fix minor memory leak

We form all of our directories in a strbuf, but never release it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
make update-server-info more robust

Since "git update-server-info" may be called automatically
as part of a push or a "gc --auto", we should be robust
against two processes trying to update it simultaneously.
However, we currently use a fixed tempfile, which means that
two simultaneous writers may step on each other's toes and
end up renaming junk into place.

Let's instead switch to using a unique tempfile via mkstemp.
We do not want to use a lockfile here, because it's OK for
two writers to simultaneously update (one will "win" the
rename race, but that's OK; they should be writing the same
information).

While we're there, let's clean up a few other things:

  1. Detect write errors. Report them and abort the update
     if any are found.

  2. Free path memory rather than leaking it (and clean up
     the tempfile when necessary).

  3. Use the pathdup functions consistently rather than
     static buffers or manually calculated lengths.

This last one fixes a potential overflow of "infofile" in
update_info_packs (e.g., by putting large junk into
$GIT_OBJECT_DIRECTORY). However, this overflow was probably
not an interesting attack vector for two reasons:

  a. The attacker would need to control the environment to
     do this, in which case it was already game-over.

  b. During its setup phase, git checks that the directory
     actually exists, which means it is probably shorter
     than PATH_MAX anyway.

Because both update_info_refs and update_info_packs share
these same failings (and largely duplicate each other), this
patch factors out the improved error-checking version into a
helper function.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
server-info: clean up after writing info/packs

We allocate pack information in a static global list but
never clean it up. This leaks memory, and means that calling
update_server_info twice will generate a buggy file (it will
have duplicate entries).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
repack: call prune_packed_objects() and update_server_info() directly

Call the functions behind git prune-packed and git update-server-info
directly instead of using run_command().  This is shorter, easier and
quicker.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
mailinfo: make ">From" in-body header check more robust

Since commit 81c5cf7 (mailinfo: skip bogus UNIX From line inside
body, 2006-05-21), we have treated lines like ">From" in the body as
headers. This makes "git am" work for people who erroneously paste
the whole output from format-patch:

  From 12345abcd...fedcba543210 Mon Sep 17 00:00:00 2001
  From: them
  Subject: [PATCH] whatever

into their email body (assuming that an mbox writer then quotes
"From" as ">From", as otherwise we would actually mailsplit on the
in-body line).

However, this has false positives if somebody actually has a commit
body that starts with "From "; in this case we erroneously remove
the line entirely from the commit message. We can make this check
more robust by making sure the line actually looks like a real mbox
"From" line.

Inspect the line that begins with ">From " a more carefully to only
skip lines that match the expected pattern (note that the datestamp
part of the format-patch output is designed to be kept constant to
help those who write magic(5) entries).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
credential-cache: close stderr in daemon process

If the stderr of "git credential-cache" is redirected to a
pipe, the reader on the other end of a pipe may be surprised
that the pipe remains open long after the process exits.
This happens because we may auto-spawn a daemon which is
long-lived, and which keeps stderr open.

We can solve this by redirecting the daemon's stderr to
/dev/null once we are ready to go into our event loop. We
would not want to do so before then, because we may want to
report errors about the setup (e.g., failure to establish
the listening socket).

This does mean that we will not report errors we encounter
for specific clients. That's acceptable, as such errors
should be rare (e.g., clients sending buggy requests).
However, we also provide an escape hatch: if you want to see
these later messages, you can provide the "--debug" option
to keep stderr open.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
receive-pack: allow hooks to ignore its standard input stream

The pre-receive and post-receive hooks were designed to be an
improvement over old style update and post-update hooks, which take
the update information on their command line and are limited by the
command line length limit.  The same information is fed from the
standard input to pre/post-receive hooks instead to lift this
limitation.  It has been mandatory for these new style hooks to
consume the update information fully from the standard input stream.
Otherwise, they would risk killing the receive-pack process via
SIGPIPE.

If a hook does not want to look at all the information, it is easy
to send its standard input to /dev/null (perhaps a niche use of hook
might need to know only the fact that a push was made, without
having to know what objects have been pushed to update which refs),
and this has already been done by existing hooks that are written
carefully.

However, because there is no good way to consistently fail hooks
that do not consume the input fully (a small push may result in a
short update record that may fit within the pipe buffer, to which
the receive-pack process may manage to write before the hook has a
chance to exit without reading anything, which will not result in a
death-by-SIGPIPE of receive-pack), it can lead to a hard to diagnose
"once in a blue moon" phantom failure.

Lift this "hooks must consume their input fully" mandate.  A mandate
that is not enforced strictly is not helping us to catch mistakes in
hooks.  If a hook has a good reason to decide the outcome of its
operation without reading the information we feed it, let it do so
as it pleases.

Signed-off-by: Junio C Hamano <gitster@pobox.com>