On Mon, Nov 25, 2013 at 10:35:19AM -0800, Junio C Hamano wrote:
if (type == OBJ_BLOB) {
if (stream_blob_to_fd(fd, sha1, NULL, 0) 0)
die(unable to stream %s to stdout, sha1_to_hex(sha1));
+ if (check_sha1_signature(sha1, NULL, 0, NULL) 0)
+
Jeff King p...@peff.net writes:
The vast majority of blobs in git.git will be stored as packed deltas.
That means the streaming code will fall back to doing the regular
in-core access. We _could_ therefore use that in-core copy to do our
sha1 check rather than streaming; but of course we
On Wed, Nov 27, 2013 at 10:57:14AM -0800, Junio C Hamano wrote:
Yes, I think it is a reasonable addition to the streaming API. However,
I do not think there are any callsites which would currently want it.
All of the current users of stream_blob_to_fd use read_sha1_file as
their
Jeff King p...@peff.net writes:
On Sun, Nov 24, 2013 at 03:44:44AM -0500, Jeff King wrote:
In any code path where we call parse_object, we double-check that the
result matches the sha1 we asked for. But low-level commands like
cat-file just call read_sha1_file directly, and do not have such
On Fri, Nov 22, 2013 at 01:28:59PM -0400, Joey Hess wrote:
Hrm. For --batch, I'd think we would open the whole object and notice
the corruption, even with the current code. But for --batch-check, we
use sha1_object_info, and for an experimental object, we do not need
to de-zlib the object
On Sun, Nov 24, 2013 at 03:44:44AM -0500, Jeff King wrote:
In any code path where we call parse_object, we double-check that the
result matches the sha1 we asked for. But low-level commands like
cat-file just call read_sha1_file directly, and do not have such a
check. We could add it, but I
On Thu, Nov 21, 2013 at 09:19:25PM +0100, Christian Couder wrote:
Yeah, I think it might report wrong size in case of replaced objects
for example.
I looked at that following Junio's comment about the
sha1_object_info() API, which,
unlike read_sha1_file() API, does not interact with the
On Fri, Nov 22, 2013 at 10:58 AM, Jeff King p...@peff.net wrote:
On Thu, Nov 21, 2013 at 09:19:25PM +0100, Christian Couder wrote:
Yeah, I think it might report wrong size in case of replaced objects
for example.
I looked at that following Junio's comment about the
sha1_object_info() API,
On Fri, Nov 22, 2013 at 12:04:01PM +0100, Christian Couder wrote:
In sha1_file.c, there is:
void *read_sha1_file_extended(const unsigned char *sha1,
enum object_type *type,
unsigned long *size,
On Fri, Nov 22, 2013 at 12:24 PM, Jeff King p...@peff.net wrote:
On Fri, Nov 22, 2013 at 12:04:01PM +0100, Christian Couder wrote:
In sha1_file.c, there is:
void *read_sha1_file_extended(const unsigned char *sha1,
enum object_type *type,
On Fri, Nov 22, 2013 at 03:23:31PM +0100, Christian Couder wrote:
The only site which calls read_sha1_file_extended directly and does not
pass the REPLACE flag is in streaming.c. And that looks to be a case of
(2), since we resolve the replacement at the start in open_istream().
Yeah,
Jeff King p...@peff.net writes:
I guess we would need to audit all the sha1_object_info callers.
Yup; I agree that was the conclusion of Christian's thread.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo
Jeff King wrote:
BTW, I've also seen git cat-file --batch report wrong sizes for objects,
Hrm. For --batch, I'd think we would open the whole object and notice
the corruption, even with the current code. But for --batch-check, we
use sha1_object_info, and for an experimental object, we do
Jeff King wrote:
sha1_file.c| 74
-
Yay!
t/t1013-loose-object-format.sh | 66 --
Hmm, not all of these tests are about the experimental format. Do
we really want to remove them all?
Thanks,
On Fri, Nov 22, 2013 at 04:24:05PM -0800, Jonathan Nieder wrote:
t/t1013-loose-object-format.sh | 66 --
Hmm, not all of these tests are about the experimental format. Do
we really want to remove them all?
I think so. They were not all testing the
Jeff King wrote:
On Fri, Nov 22, 2013 at 04:24:05PM -0800, Jonathan Nieder wrote:
t/t1013-loose-object-format.sh | 66 --
Hmm, not all of these tests are about the experimental format. Do
we really want to remove them all?
I think so. They were not all
On Wed, Nov 20, 2013 at 06:28:06PM -0400, Joey Hess wrote:
Jeff King wrote:
As for your specific corruption, I can't make heads or tails of it. It
is not a single-bit error.
Oh, I should have mentioned that I am generating corrupt git
repositories mechanically for testing. I think in
On Thu, Nov 21, 2013 at 06:41:58AM -0500, Jeff King wrote:
The test objects removed are all binary. Git seems to guess a few as
non-binary, though, because they don't contain any NULs, and includes
gross binary bytes in the patch below. In theory the mail's transfer
encoding will take care of
On Thu, Nov 21, 2013 at 07:43:03PM +0700, Duy Nguyen wrote:
- if (experimental_loose_object(map)) {
Perhaps keep this..
- /*
-* The old experimental format we no longer produce;
-* we can still read it.
-*/
-
Jeff King p...@peff.net writes:
We could try to improve the heuristic to err on the side of
normal objects in the face of corruption, but there is
really little point. The experimental format is long-dead,
and was never enabled by default to begin with. We can
instead simply remove it. The
On Thu, Nov 21, 2013 at 5:04 PM, Joey Hess j...@kitenet.net wrote:
BTW, I've also seen git cat-file --batch report wrong sizes for objects,
sometimes without crashing. This is particularly problimatic because if
the object size is wrong, it's very hard to detect the actual end of the
object
On Thu, Nov 21, 2013 at 6:48 PM, Jeff King p...@peff.net wrote:
@@ -1514,14 +1469,6 @@ unsigned long unpack_object_header_buffer(const
unsigned char *buf,
int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned
long mapsize, void *buffer, unsigned long bufsiz)
{
-
Duy Nguyen pclo...@gmail.com writes:
On Thu, Nov 21, 2013 at 6:48 PM, Jeff King p...@peff.net wrote:
@@ -1514,14 +1469,6 @@ unsigned long unpack_object_header_buffer(const
unsigned char *buf,
int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned
long mapsize, void
On Thu, Nov 21, 2013 at 12:04:26PM -0400, Joey Hess wrote:
Jeff King wrote:
This latter behavior is much worse for two reasons. One,
git reports an allocation error when the real error is
corruption. And two, the program dies unconditionally, so
you cannot even run fsck (which would
Jeff King wrote:
This latter behavior is much worse for two reasons. One,
git reports an allocation error when the real error is
corruption. And two, the program dies unconditionally, so
you cannot even run fsck (which would otherwise ignore the
broken object and keep going).
BTW, I've also
25 matches
Mail list logo