Skip to content

Commit

Permalink
fsck: it is OK for a tag and a commit to lack the body
Browse files Browse the repository at this point in the history
When fsck validates a commit or a tag, it scans each line in the
header of the object using helper functions such as "start_with()",
etc. that work on a NUL terminated buffer, but before a1e920a
(index-pack: terminate object buffers with NUL, 2014-12-08), the
validation functions were fed the object data in a piece of memory
that is not necessarily terminated with a NUL.

We added a helper function require_end_of_header() to be called at
the beginning of these validation functions to insist that the
object data contains an empty line before its end.  The theory is
that the validating functions will notice and stop when it hits an
empty line as a normal end of header (or a required header line that
is missing) without scanning past the end of potentially not
NUL-terminated buffer.

But the theory forgot that in the older days, Git itself happily
created objects with only the header lines without a body. This
caused Git 2.2 and later to issue an unnecessary warning in some
existing repositories.

With a1e920a, we do not need to require an empty line (or the body)
in these objects to safely parse and validate them.  Drop the
offending "must have an empty line" check from this helper function,
while keeping the other check to make sure that there is no NUL in
the header part of the object, and adjust the name of the helper to
what it does accordingly.

Noticed-by: Wolfgang Denk <wd@denx.de>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Junio C Hamano committed Jun 28, 2015
1 parent a1e920a commit 84d18c0
Showing 1 changed file with 13 additions and 4 deletions.
17 changes: 13 additions & 4 deletions fsck.c
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,8 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)
return retval;
}

static int require_end_of_header(const void *data, unsigned long size,
struct object *obj, fsck_error error_func)
static int verify_headers(const void *data, unsigned long size,
struct object *obj, fsck_error error_func)
{
const char *buffer = (const char *)data;
unsigned long i;
Expand All @@ -255,6 +255,15 @@ static int require_end_of_header(const void *data, unsigned long size,
}
}

/*
* We did not find double-LF that separates the header
* and the body. Not having a body is not a crime but
* we do want to see the terminating LF for the last header
* line.
*/
if (size && buffer[size - 1] == '\n')
return 0;

return error_func(obj, FSCK_ERROR, "unterminated header");
}

Expand Down Expand Up @@ -305,7 +314,7 @@ static int fsck_commit_buffer(struct commit *commit, const char *buffer,
unsigned parent_count, parent_line_count = 0;
int err;

if (require_end_of_header(buffer, size, &commit->object, error_func))
if (verify_headers(buffer, size, &commit->object, error_func))
return -1;

if (!skip_prefix(buffer, "tree ", &buffer))
Expand Down Expand Up @@ -384,7 +393,7 @@ static int fsck_tag_buffer(struct tag *tag, const char *data,
}
}

if (require_end_of_header(buffer, size, &tag->object, error_func))
if (verify_headers(buffer, size, &tag->object, error_func))
goto done;

if (!skip_prefix(buffer, "object ", &buffer)) {
Expand Down

0 comments on commit 84d18c0

Please sign in to comment.