From patchwork Thu May 19 20:09:16 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 12855991 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C0633C433F5 for ; Thu, 19 May 2022 20:09:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241704AbiESUJj (ORCPT ); Thu, 19 May 2022 16:09:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36730 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S244472AbiESUJf (ORCPT ); Thu, 19 May 2022 16:09:35 -0400 Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CCD00A5AB6 for ; Thu, 19 May 2022 13:09:34 -0700 (PDT) Received: by mail-wr1-x434.google.com with SMTP id r23so8671174wrr.2 for ; Thu, 19 May 2022 13:09:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=cvOawsrHKZ549ig5YIMxqHUCJpvMrWq0Q5eB2cFS3Rw=; b=dn6H5EODg9j2GXSX5RUYvqAZdmCTvDdFxvg8mnX7gQCtC+vKveNSpdIMhxMJuBFuLY 3xC5V+PjtAT+OOq6DHju7Wgd7xJJgSjn/LJlisabYxOG97tYyrdwSEnOBMrcZOuAyGze N62FOyYljJEKwGu1w30ayo+bRvCBYZS9zcIz542tHZj+NdJhNmstlrT7LEruD0dh0nTc TyNGk6taSWb6oV/4u/4bZf8SxwZMGowBfto2URvqfbr5CfUjjRrhkDlXXMZlV+qV1BHD syRXKVWWbuGTJ+tg114JKPdZc4oImxyZUECccDS7huILsnzsCuEQAms2MUJ9285AXkfk NoLA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=cvOawsrHKZ549ig5YIMxqHUCJpvMrWq0Q5eB2cFS3Rw=; b=VR/VKcAsEsUql1mI2SqEOkxPv8MhBltHXtLv5QCa0eZ1fCu2/Fk4RdBlOEE61u9hW8 laQ+P87DK7Bjm7CRMoULYvgOdT+uTcYVNjNkIXX99QdI23l3k7ajWe7U8n2UUTIsD/JF Y1dAnDrYooU/2uziZuoIGkzJKwbpzQS7KI/ms2wWuZGoWOL1lWJW2dHnNULD6ANWKMo1 t60fl7TI6DLuTE36yVp+NsNUmgUywqOVQ9wbngXLgGP7mE/oRxai1gAaxsnKrTN+pLFY jq4RTVnNoE6k9jcDhbLJk2A9lKGnHIIdsk8kZs0Qf3+vPvw30CBeAnxLN+e39TMDuaNc cbMg== X-Gm-Message-State: AOAM5300ZyMxFk3kaX+3vAoR5bIjrNII8PjPuJQy9eFsWUcVgPBvhStK tv9KdSYYeUdQVvTLejeZeclMY27y3E96Tg== X-Google-Smtp-Source: ABdhPJwCyKdttG9ZNayxtEW2aayuX89pF3YRdxRM+XfHYVckiBtlSlK9ORThctR4RhaK6KKV8aKQ2g== X-Received: by 2002:a5d:47c8:0:b0:20c:630f:7df5 with SMTP id o8-20020a5d47c8000000b0020c630f7df5mr5353307wrc.689.1652990973033; Thu, 19 May 2022 13:09:33 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id ay13-20020a05600c1e0d00b003944821105esm428152wmb.2.2022.05.19.13.09.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 May 2022 13:09:32 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , =?utf-8?b?w4Z2YXIgQXJuZmrDtnI=?= =?utf-8?b?w7AgQmphcm1hc29u?= Subject: [RFC PATCH 1/2] object-file API: fix obscure unpack_loose_header() return Date: Thu, 19 May 2022 22:09:16 +0200 Message-Id: X-Mailer: git-send-email 2.36.1.957.g2c13267e09b In-Reply-To: References: <377be0e9-8a0f-4a86-0a66-3b08c0284dae@github.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Fix an obscure unpack_loose_header() return value. In my 5848fb11acd (object-file.c: return ULHR_TOO_LONG on "header too long", 2021-10-01) this API learned to return ULHR_TOO_LONG, but should not have done so in the case of parsing a long header where the terminating \0 cannot be found. We should return a ULHR_BAD in that case. Signed-off-by: Ævar Arnfjörð Bjarmason --- cache.h | 3 +-- object-file.c | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/cache.h b/cache.h index 595582becc8..aa24d5a609f 100644 --- a/cache.h +++ b/cache.h @@ -1342,8 +1342,7 @@ int git_open_cloexec(const char *name, int flags); * "hdrbuf" argument is non-NULL. This is intended for use with * OBJECT_INFO_ALLOW_UNKNOWN_TYPE to extract the bad type for (error) * reporting. The full header will be extracted to "hdrbuf" for use - * with parse_loose_header(), ULHR_TOO_LONG will still be returned - * from this function to indicate that the header was too long. + * with parse_loose_header(). */ enum unpack_loose_header_result { ULHR_OK, diff --git a/object-file.c b/object-file.c index b5d1d12b68a..1babe9791f6 100644 --- a/object-file.c +++ b/object-file.c @@ -1301,7 +1301,7 @@ enum unpack_loose_header_result unpack_loose_header(git_zstream *stream, stream->next_out = buffer; stream->avail_out = bufsiz; } while (status != Z_STREAM_END); - return ULHR_TOO_LONG; + return ULHR_BAD; } static void *unpack_loose_rest(git_zstream *stream, From patchwork Thu May 19 20:09:17 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= X-Patchwork-Id: 12855992 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8803DC433FE for ; Thu, 19 May 2022 20:09:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244645AbiESUJn (ORCPT ); Thu, 19 May 2022 16:09:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36742 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S244628AbiESUJh (ORCPT ); Thu, 19 May 2022 16:09:37 -0400 Received: from mail-wm1-x329.google.com (mail-wm1-x329.google.com [IPv6:2a00:1450:4864:20::329]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 399898E1AD for ; Thu, 19 May 2022 13:09:36 -0700 (PDT) Received: by mail-wm1-x329.google.com with SMTP id ay35so1864138wmb.5 for ; Thu, 19 May 2022 13:09:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=48w8Uwh2KKt4APH5F5gouMQ0Yghho4tBdfBowTt0sjo=; b=iWJC8Qth3+eeykxreUrwiMXy1j1UCkGLkekJuOrH7XRsOtdcWTpLd4d+gfXe3sX5Aq hK2FTTpG6kfSXq07tcUMlPrutfjj7g2eQjD61RRRpFkqyHOpl4939VwcZZ17kalLpqie +pu0MOYdvM60iQw8weW84/WviIqCXrKQs34ofDvIxvOxUsGFvPhYomFlz3DNDaC7YBAD UZ9Kc3elio3PvFqf54yoFUU61A3aQo1RO2xVV+nHKeIfadXydkWg+oe60Mvq8QaEZ4ET p+bGP1d9rJrQspeS8bOb+1sxW9M/LAYq8hTUExqM9J8h26mmfL80WCzCwHkVKED2tELH kTTQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=48w8Uwh2KKt4APH5F5gouMQ0Yghho4tBdfBowTt0sjo=; b=PQnwZGnqnwy6hwNQIT+PFDn2u+q0gjVHRGynN9ReuW0ngwedaDTjz4L/D+1Uf2Jvya 5KKkLGdtpKAfOmJr99N3JC94EjQILs5vJgigYS4vxUuZF9N3z9ApJKsKx55TxFa+YrW1 Ya9/A4U1r8bFG1O5wv4lbzY9oM0HlgYTWC3DwT1ELK+Ehz7bYXhwac3zKw14DaIP+zAb 8baxvNvQDTHYxVr/eQzc4opYwG7NyjJkR8xh0Z39gvl5V9p3KwhJTg8E0qjXZgtoHNFG /kqovL5TdNixHRiObdLWOf06tx7iGukuTMEGuAaAWLZj68vaytdcoXgmRyA5Qf76fNgQ 4sDw== X-Gm-Message-State: AOAM5308y341igCYYxKZGVsecd7Z6cCuKL+jmnOxkKo3jVkcxzh1tDap NeoS0mLreO3fZVBfVLZOkEN4pZnVhsDdYg== X-Google-Smtp-Source: ABdhPJxrrLVDNDwmd4ShdN+QH+Uzd8GKUOh1cMFCmEBis2wHvx4TKl8J7H4RO6MTJ7gWXVDFqBoEwQ== X-Received: by 2002:a7b:cc17:0:b0:38d:af7:3848 with SMTP id f23-20020a7bcc17000000b0038d0af73848mr5389719wmh.41.1652990974535; Thu, 19 May 2022 13:09:34 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id ay13-20020a05600c1e0d00b003944821105esm428152wmb.2.2022.05.19.13.09.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 May 2022 13:09:33 -0700 (PDT) From: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= To: git@vger.kernel.org Cc: Junio C Hamano , Derrick Stolee , =?utf-8?b?w4Z2YXIgQXJuZmrDtnI=?= =?utf-8?b?w7AgQmphcm1hc29u?= Subject: [RFC PATCH 2/2] object-file API: have unpack_loose_header() return "int" again Date: Thu, 19 May 2022 22:09:17 +0200 Message-Id: X-Mailer: git-send-email 2.36.1.957.g2c13267e09b In-Reply-To: References: <377be0e9-8a0f-4a86-0a66-3b08c0284dae@github.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Revert the API change in my 5848fb11acd (object-file.c: return ULHR_TOO_LONG on "header too long", 2021-10-01) in favor of having unpack_loose_header() return a simple negative value on error. The more complex API was needed to give us flexibility that we didn't really need. We only used the ULHR_TOO_LONG return case for headers which "cat-file --allow-unknown-type" is needed for. Let's instead error() on those unconditionally. The slight change in the "error" message is that we won't include the OID in the error in that case, but we will include it in the "unable to unpack" error emitted by loose_object_info(). As noted in the preceding commit we were mishandling the case where we'll now error() with "could not find end of corrupt long header". Signed-off-by: Ævar Arnfjörð Bjarmason --- cache.h | 22 ++++------------------ object-file.c | 46 +++++++++++++++++---------------------------- streaming.c | 11 +++-------- t/t1006-cat-file.sh | 6 ++++-- 4 files changed, 28 insertions(+), 57 deletions(-) diff --git a/cache.h b/cache.h index aa24d5a609f..629c4a84b90 100644 --- a/cache.h +++ b/cache.h @@ -1330,13 +1330,7 @@ int git_open_cloexec(const char *name, int flags); /** * unpack_loose_header() initializes the data stream needed to unpack - * a loose object header. - * - * Returns: - * - * - ULHR_OK on success - * - ULHR_BAD on error - * - ULHR_TOO_LONG if the header was too long + * a loose object header. A negative value indicates an error. * * It will only parse up to MAX_HEADER_LEN bytes unless an optional * "hdrbuf" argument is non-NULL. This is intended for use with @@ -1344,17 +1338,9 @@ int git_open_cloexec(const char *name, int flags); * reporting. The full header will be extracted to "hdrbuf" for use * with parse_loose_header(). */ -enum unpack_loose_header_result { - ULHR_OK, - ULHR_BAD, - ULHR_TOO_LONG, -}; -enum unpack_loose_header_result unpack_loose_header(git_zstream *stream, - unsigned char *map, - unsigned long mapsize, - void *buffer, - unsigned long bufsiz, - struct strbuf *hdrbuf); +int unpack_loose_header(git_zstream *stream, unsigned char *map, + unsigned long mapsize, void *buffer, + unsigned long bufsiz, struct strbuf *header); /** * parse_loose_header() parses the starting " \0" of an diff --git a/object-file.c b/object-file.c index 1babe9791f6..1d2901d0e78 100644 --- a/object-file.c +++ b/object-file.c @@ -1245,12 +1245,9 @@ void *map_loose_object(struct repository *r, return map_loose_object_1(r, NULL, oid, size); } -enum unpack_loose_header_result unpack_loose_header(git_zstream *stream, - unsigned char *map, - unsigned long mapsize, - void *buffer, - unsigned long bufsiz, - struct strbuf *header) +int unpack_loose_header(git_zstream *stream, unsigned char *map, + unsigned long mapsize, void *buffer, + unsigned long bufsiz, struct strbuf *header) { int status; @@ -1266,13 +1263,13 @@ enum unpack_loose_header_result unpack_loose_header(git_zstream *stream, status = git_inflate(stream, 0); obj_read_lock(); if (status < Z_OK) - return ULHR_BAD; + return -1; /* * Check if entire header is unpacked in the first iteration. */ if (memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer)) - return ULHR_OK; + return 0; /* * We have a header longer than MAX_HEADER_LEN. The "header" @@ -1280,7 +1277,8 @@ enum unpack_loose_header_result unpack_loose_header(git_zstream *stream, * --allow-unknown-type". */ if (!header) - return ULHR_TOO_LONG; + return error(_("header too long, exceeds %d bytes"), + MAX_HEADER_LEN); /* * buffer[0..bufsiz] was not large enough. Copy the partial @@ -1301,7 +1299,7 @@ enum unpack_loose_header_result unpack_loose_header(git_zstream *stream, stream->next_out = buffer; stream->avail_out = bufsiz; } while (status != Z_STREAM_END); - return ULHR_BAD; + return error(_("could not find end of corrupt long header")); } static void *unpack_loose_rest(git_zstream *stream, @@ -1466,32 +1464,26 @@ static int loose_object_info(struct repository *r, if (oi->disk_sizep) *oi->disk_sizep = mapsize; - switch (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr), - allow_unknown ? &hdrbuf : NULL)) { - case ULHR_OK: + if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr), + allow_unknown ? &hdrbuf : NULL) < 0) { + status = error(_("unable to unpack %s header"), + oid_to_hex(oid)); + } else { if (parse_loose_header(hdrbuf.len ? hdrbuf.buf : hdr, oi) < 0) status = error(_("unable to parse %s header"), oid_to_hex(oid)); else if (!allow_unknown && *oi->typep < 0) die(_("invalid object type")); if (!oi->contentp) - break; + goto inflate_end; *oi->contentp = unpack_loose_rest(&stream, hdr, *oi->sizep, oid); if (*oi->contentp) goto cleanup; status = -1; - break; - case ULHR_BAD: - status = error(_("unable to unpack %s header"), - oid_to_hex(oid)); - break; - case ULHR_TOO_LONG: - status = error(_("header for %s too long, exceeds %d bytes"), - oid_to_hex(oid), MAX_HEADER_LEN); - break; } +inflate_end: git_inflate_end(&stream); cleanup: munmap(map, mapsize); @@ -2623,12 +2615,8 @@ int read_loose_object(const char *path, goto out; } - switch (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr), - NULL)) { - case ULHR_OK: - break; - case ULHR_BAD: - case ULHR_TOO_LONG: + if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr), + NULL) < 0) { error(_("unable to unpack header of %s"), path); goto out; } diff --git a/streaming.c b/streaming.c index fe54665d86e..4456a60348b 100644 --- a/streaming.c +++ b/streaming.c @@ -230,15 +230,10 @@ static int open_istream_loose(struct git_istream *st, struct repository *r, st->u.loose.mapped = map_loose_object(r, oid, &st->u.loose.mapsize); if (!st->u.loose.mapped) return -1; - switch (unpack_loose_header(&st->z, st->u.loose.mapped, - st->u.loose.mapsize, st->u.loose.hdr, - sizeof(st->u.loose.hdr), NULL)) { - case ULHR_OK: - break; - case ULHR_BAD: - case ULHR_TOO_LONG: + if (unpack_loose_header(&st->z, st->u.loose.mapped, + st->u.loose.mapsize, st->u.loose.hdr, + sizeof(st->u.loose.hdr), NULL) < 0) goto error; - } if (parse_loose_header(st->u.loose.hdr, &oi) < 0 || *type < 0) goto error; diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index dadf3b14583..d742697d3bf 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -536,12 +536,14 @@ do if test "$arg2" = "-p" then cat >expect <<-EOF - error: header for $bogus_long_sha1 too long, exceeds 32 bytes + error: header too long, exceeds 32 bytes + error: unable to unpack $bogus_long_sha1 header fatal: Not a valid object name $bogus_long_sha1 EOF else cat >expect <<-EOF - error: header for $bogus_long_sha1 too long, exceeds 32 bytes + error: header too long, exceeds 32 bytes + error: unable to unpack $bogus_long_sha1 header fatal: git cat-file: could not get object info EOF fi &&