[BUG] ZIP timestamp conversion and strict fast-import date validation

[BUG] ZIP timestamp conversion and strict fast-import date validation

[Matthew E. Luallen]

Hello Git community,

I'm reporting three date-handling bugs reproduced on Apple Git 2.50.1
and upstream Git 2.56.0:

1. Exporting a 1972-dated commit with git archive --format=zip produces
   a legacy DOS date interpreted as 2100, while the extended Unix
   timestamp retains 1972.
2. Exporting a commit at epoch 4294967296 (2106-02-07 06:28:16 UTC)
   wraps ZIP's four-byte extended timestamp to zero (1970). Exporting
   the same commit as TAR preserves the original value.
3. git fast-import --date-format=raw accepts -32184000 +0000, but
   git fsck --strict then reports badDate and ISO rendering returns
   literal placeholders. This occurs in strict raw mode, not just
   the deliberately permissive import mode.

For the archive cases, export the dated commit with:

    git archive --format=zip <commit> > test.zip
    git archive --format=tar <commit> > test.tar

Compare the ZIP DOS and extended timestamp fields with the TAR mtime.
The overflow affects consumer behavior: in macOS tests, UnZip update
mode retained different existing 2025 content because the 2106 ZIP
appeared older. An in-range 2038 ZIP replaced that content.

What range-handling policy should ZIP use, and should strict raw import
reject timestamps that fsck considers invalid?

Thank you,
Matthew E. Luallen (@meluallen)
With research, reproduction, and drafting assistance from OpenAI Codex.

Re: [BUG] ZIP timestamp conversion and strict fast-import date validation

[René Scharfe]

On 10/5/26 3:48 PM, Matthew E. Luallen wrote:
> Hello Git community,
> 
> I'm reporting three date-handling bugs reproduced on Apple Git 2.50.1
> and upstream Git 2.56.0:
> 
> 1. Exporting a 1972-dated commit with git archive --format=zip produces
>    a legacy DOS date interpreted as 2100, while the extended Unix
>    timestamp retains 1972.

DOS timestamps can express years starting from 1980.  Unix timestamps
start at 1970.  We can't provide a DOS timestamp for 1972, but can we do
better?  zip(1) clamps DOS timestamps to zero = the DOS epoch =
1980-01-01 00:00:00.

Is anything using the DOS timestamp even though a Unix timestamp is
present?

> 2. Exporting a commit at epoch 4294967296 (2106-02-07 06:28:16 UTC)
>    wraps ZIP's four-byte extended timestamp to zero (1970). Exporting
>    the same commit as TAR preserves the original value.

tar's timestamp can range from 1970 to 2242 with standard headers and
beyond indefinitely with extended headers.

We cannot put a higher value than 4294967295 into the four-byte field
provided by the Unix time extension for ZIP, but we could clamp to that
value.  zip(1) wraps around as well, though..

(I'm using "Zip 3.0 (July 5th 2008), by Info-ZIP, with modifications by
Apple Inc.")

> 3. git fast-import --date-format=raw accepts -32184000 +0000, but
>    git fsck --strict then reports badDate and ISO rendering returns
>    literal placeholders. This occurs in strict raw mode, not just
>    the deliberately permissive import mode.

Well, raw mode passes on the timestamp value with only little checks.
strtoul(3) used in builtin/fast-import.c::validate_raw_date() happily
accepts negative numbers.  The latter function contains two NEEDSWORK
comments about perhaps adding more checks, though.

René

Re: [BUG] ZIP timestamp conversion and strict fast-import date validation

[Matthew E. Luallen]

[-- Attachment #1: Type: text/plain, Size: 943 bytes --]

Hi René,

Thank you for the thoughtful response. I appreciate your help.

- Reader example: Python 3.14.7's ZipInfo.date_time reports 2100 for
  the 1972 ZIP despite its correct Unix timestamp. Our tested UnZip
  and bsdtar restored 1972 correctly; this is a metadata-reading example.

- Separate consequence: the 2106-to-1970 wrap caused UnZip update mode
  to retain different existing 2025 content. The 2038 control updated
  correctly. No deployed security bypass has been demonstrated.

- Fix direction: would you prefer clamping or rejecting ZIP timestamps
  beyond the Unix field's range? Our candidate rejects them. Would
  rejecting negative dates in strict raw import be a useful first step?

Attached is brief guidance on misinterpretation risks, clearer date
labels, and safer downstream decisions. These suggestions complement
the ordinary bug fixes without asserting a security classification.

Best,
Matt

[-- Attachment #2: git-timestamp-guidance.txt --]
[-- Type: text/plain, Size: 3504 bytes --]

GIT TIMESTAMP GUIDANCE: INTERPRETATION, INTEGRITY AND REMEDIATION
5 October 2026 | Companion to the public Git date-handling discussion

WHY IT MATTERS

- Interpretation: dates can be mistaken for creation, publication or
  approval times. Conversion errors can mislead without malicious intent.
- Scale: repeated reliance on dates across update, review and audit
  pipelines could multiply errors. Prevalence has not been measured.
- Security impact requires attacker-influenced metadata to affect a
  security decision without independent checks. A local update decision
  changed; a production security bypass has not been demonstrated.

OBSERVED, WITH CONTROLS

- Python 3.14.7 reads the 1972 ZIP's DOS date as 2100 despite the correct
  Unix extra field. UnZip 6.00 and bsdtar 3.5.3 restored 1972; Python's
  ordinary extraction used a current local modification time.
- A 2106 commit's ZIP Unix timestamp wrapped to 1970; UnZip update mode
  kept different existing 2025 bytes. A 2038 control replaced them;
  TAR preserved 2106. Strict raw import also accepted a negative date
  subsequently rejected by strict fsck. Reader behavior varies by platform.

PRACTICAL OPTIONS

- Exporters: validate ranges before encoding. Clamp pre-1980 DOS dates
  while preserving representable Unix dates. For Unix-field overflow,
  choose rejection or documented clamping; clamping loses the original
  value. Our candidate rejects overflow. These are proposed policies.
- Importers: align strict parsing with validation; check negative values,
  complete numeric input and overflow. Keep permissive modes explicit.
  Test boundaries across timezones and archive readers.
- Consumers: decide updates using expected content/version information
  from a trusted source, not modification time alone. For change review,
  retain a trusted baseline commit and compare reachable changes; handle
  missing or rewritten history explicitly. Date windows do not establish
  complete coverage of newly received content.
- Displays: distinguish 'committer-supplied date', 'archive modification
  time' and 'server observed at'. Explain clamping and missing evidence.
  Server receipt is not original creation or first publication elsewhere.
- Evidence: preserve original bytes and independent receipt records.
  Commit signatures bind signed content to a key under a trust policy;
  they do not independently prove its claimed creation time. A validated
  trusted timestamp can support existence by a time, not exact creation.

RISK TAXONOMY, NOT A VULNERABILITY ASSIGNMENT

- Numeric representation/truncation: CWE-197; wraparound: CWE-190.
- Validation: CWE-20 (broad category). Downstream trust: CWE-807 only
  where a security decision relies on untrusted input. Display confusion
  alone does not establish it. These labels do not establish severity.

REFERENCES AND EVIDENCE

Git date controls: https://git-scm.com/docs/git-commit#_commit_information
Archive behavior: https://git-scm.com/docs/git-archive#_description
CWE taxonomy: https://cwe.mitre.org/data/definitions/197.html
https://cwe.mitre.org/data/definitions/190.html
https://cwe.mitre.org/data/definitions/20.html
https://cwe.mitre.org/data/definitions/807.html
Trusted timestamp semantics: https://www.rfc-editor.org/rfc/rfc3161
Evidence: recorded 2026-10-01 consumer-date-impact results and Git matrix;
no new experiment is claimed by this guidance.

Matthew E. Luallen (@meluallen), with research, reproduction and drafting
assistance from OpenAI Codex.

Re: [BUG] ZIP timestamp conversion and strict fast-import date validation

[m]

Hi René,

Thanks for the explanation. Python 3.14.7’s ZipInfo.date_time is one
example: it reads the DOS date as 2100 for the 1972 archive, even
though the correct Unix timestamp is present. This affects metadata
reading; the tested UnZip and bsdtar restored 1972 correctly.

The 2106 case is separate: the Unix timestamp wraps to 1970, causing
UnZip update mode to retain different existing 2025 content. The 2038
control replaced that content as expected.

Best,
Matthew

On Mon, 05 Oct 2026 21:49:33 +0200, René Scharfe <l.s.r@web.de> wrote:
> On 10/5/26 3:48 PM, Matthew E. Luallen wrote:
> > Hello Git community,
> >
> > I'm reporting three date-handling bugs reproduced on Apple Git 2.50.1
> > and upstream Git 2.56.0:
> >
> > 1. Exporting a 1972-dated commit with git archive --format=zip produces
> >    a legacy DOS date interpreted as 2100, while the extended Unix
> >    timestamp retains 1972.
>
> DOS timestamps can express years starting from 1980.  Unix timestamps
> start at 1970.  We can't provide a DOS timestamp for 1972, but can we do
> better?  zip(1) clamps DOS timestamps to zero = the DOS epoch =
> 1980-01-01 00:00:00.
>
> Is anything using the DOS timestamp even though a Unix timestamp is
> present?
>
> > 2. Exporting a commit at epoch 4294967296 (2106-02-07 06:28:16 UTC)
> >    wraps ZIP's four-byte extended timestamp to zero (1970). Exporting
> >    the same commit as TAR preserves the original value.
>
> tar's timestamp can range from 1970 to 2242 with standard headers and
> beyond indefinitely with extended headers.
>
> We cannot put a higher value than 4294967295 into the four-byte field
> provided by the Unix time extension for ZIP, but we could clamp to that
> value.  zip(1) wraps around as well, though..
>
> (I'm using "Zip 3.0 (July 5th 2008), by Info-ZIP, with modifications by
> Apple Inc.")
>
> > 3. git fast-import --date-format=raw accepts -32184000 +0000, but
> >    git fsck --strict then reports badDate and ISO rendering returns
> >    literal placeholders. This occurs in strict raw mode, not just
> >    the deliberately permissive import mode.
>
> Well, raw mode passes on the timestamp value with only little checks.
> strtoul(3) used in builtin/fast-import.c::validate_raw_date() happily
> accepts negative numbers.  The latter function contains two NEEDSWORK
> comments about perhaps adding more checks, though.
>
> René

Re: [BUG] ZIP timestamp conversion and strict fast-import date validation

[René Scharfe]

On 10/5/26 10:32 PM, Matthew E. Luallen wrote:
> 
> - Reader example: Python 3.14.7's ZipInfo.date_time reports 2100 for
>   the 1972 ZIP despite its correct Unix timestamp.

Makes sense.  Its source code,
https://github.com/python/cpython/blob/3.14/Lib/zipfile/__init__.py,
contains a decode function for two extra fields, Zip64 (0x0001) and
Info-ZIP Unicode Path (0x7075), but no support for UNIX (0x000d).
And why should it?

> - Separate consequence: the 2106-to-1970 wrap caused UnZip update mode
>   to retain different existing 2025 content. The 2038 control updated
>   correctly. No deployed security bypass has been demonstrated.

I'm not aware of a ZIP extension that allows storing arbitrary dates.
I see three options:

- Don't color outside the lines, only emit ZIP files with valid DOS
  and UNIX timestamps and refuse to write earlier or later ones,

- wrap consistently, which can be confusing, but allows users to
  recover the original timestamp when they supply the higher bits
  (like we can say twenties now to mean 202x and 30 years ago we
  meant 192x), or

- map all earlier timestamps to the epoch and later ones to the maximum
  value, effectively stopping time at the borders -- all mtimes will be
  0xffffffff forever after that point is crossed, making them useless,
  except as a marker that a yet to be invented extension needs to be
  consulted to get the real mtime value.

> - Fix direction: would you prefer clamping or rejecting ZIP timestamps
>   beyond the Unix field's range? Our candidate rejects them. Would
>   rejecting negative dates in strict raw import be a useful first step?

Rejecting non-representable timestamps when creating ZIP files seems
like the most honest option.  Perhaps it's annoying enough to motivate
people to find a proper solution?  Which could be "use the tar format".

It would be nice if there was an example to follow.  Info-ZIP zip(1)
clamping at the low end and rolling over at the high end seems odd,
though.

Rejecting negative timestamps on fast-import or at all seems bad for
people who want to import ancient records.  Git commit objects store
timestamps as decimal numeric strings, so they can support arbitrarily
high and low values.  Importing mainframe file versions from the
sixties or versions of legal documents from the last few hundred years
don't seem too outlandish.

timestamp_t, Git's in-memory representation, is unsigned for
historical reasons, but that is not necessarily fixed in stone.

René

[PATCH] archive-zip: omit UT extra field if mtime is out of range

[René Scharfe]

We write an Extended Timestamp Extra Field (ID 0x5455 = "UT") for each
zip entry, like Info-ZIP zip(1) does.  Its time values are signed 32-bit
integers (see e.g. https://libzip.org/specifications/extrafld.txt).
Stop emitting them when the commit time doesn't fit in.

Use a strbuf to prepare the extra field up front.  Keep it empty if the
time value is out of range.  Like for the central zip directory we only
care about the length-limited buffer aspect of the strbuf, not about
NUL-termination.  This simplifies the conditional output by avoiding to
repeat the condition.

Reported-by: Matthew E. Luallen <m@sph3r3.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
 archive-zip.c                   | 37 +++++++++++++--------------------
 t/t5004-archive-corner-cases.sh | 28 +++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/archive-zip.c b/archive-zip.c
index 6f73ca6d58..785689584c 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -24,6 +24,7 @@ static int zip_time;
 
 /* We only care about the "buf" part here. */
 static struct strbuf zip_dir;
+static struct strbuf zip_extra_mtime;
 
 static uintmax_t zip_offset;
 static uint64_t zip_dir_entries;
@@ -81,14 +82,6 @@ struct zip_dir_trailer {
 	unsigned char _end[1];
 };
 
-struct zip_extra_mtime {
-	unsigned char magic[2];
-	unsigned char extra_size[2];
-	unsigned char flags[1];
-	unsigned char mtime[4];
-	unsigned char _end[1];
-};
-
 struct zip64_extra {
 	unsigned char magic[2];
 	unsigned char extra_size[2];
@@ -129,9 +122,6 @@ struct zip64_dir_trailer_locator {
 #define ZIP64_DATA_DESC_SIZE	offsetof(struct zip64_data_desc, _end)
 #define ZIP_DIR_HEADER_SIZE	offsetof(struct zip_dir_header, _end)
 #define ZIP_DIR_TRAILER_SIZE	offsetof(struct zip_dir_trailer, _end)
-#define ZIP_EXTRA_MTIME_SIZE	offsetof(struct zip_extra_mtime, _end)
-#define ZIP_EXTRA_MTIME_PAYLOAD_SIZE \
-	(ZIP_EXTRA_MTIME_SIZE - offsetof(struct zip_extra_mtime, flags))
 #define ZIP64_EXTRA_SIZE	offsetof(struct zip64_extra, _end)
 #define ZIP64_EXTRA_PAYLOAD_SIZE \
 	(ZIP64_EXTRA_SIZE - offsetof(struct zip64_extra, size))
@@ -299,9 +289,8 @@ static int write_zip_entry(struct archiver_args *args,
 {
 	struct zip_local_header header;
 	uintmax_t offset = zip_offset;
-	struct zip_extra_mtime extra;
 	struct zip64_extra extra64;
-	size_t header_extra_size = ZIP_EXTRA_MTIME_SIZE;
+	size_t header_extra_size = zip_extra_mtime.len;
 	int need_zip64_extra = 0;
 	unsigned long attr2;
 	unsigned long compressed_size;
@@ -315,7 +304,7 @@ static int write_zip_entry(struct archiver_args *args,
 	const char *path_without_prefix = path + args->baselen;
 	unsigned int creator_version = 0;
 	unsigned int version_needed = 10;
-	size_t zip_dir_extra_size = ZIP_EXTRA_MTIME_SIZE;
+	size_t zip_dir_extra_size = zip_extra_mtime.len;
 	size_t zip64_dir_extra_payload_size = 0;
 
 	crc = crc32(0, NULL, 0);
@@ -381,11 +370,6 @@ static int write_zip_entry(struct archiver_args *args,
 		}
 	}
 
-	copy_le16(extra.magic, 0x5455);
-	copy_le16(extra.extra_size, ZIP_EXTRA_MTIME_PAYLOAD_SIZE);
-	extra.flags[0] = 1;	/* just mtime */
-	copy_le32(extra.mtime, args->time);
-
 	if (size > 0xffffffff || compressed_size > 0xffffffff)
 		need_zip64_extra = 1;
 	if (stream && size > 0x7fffffff)
@@ -412,8 +396,8 @@ static int write_zip_entry(struct archiver_args *args,
 	zip_offset += ZIP_LOCAL_HEADER_SIZE;
 	write_or_die(1, path, pathlen);
 	zip_offset += pathlen;
-	write_or_die(1, &extra, ZIP_EXTRA_MTIME_SIZE);
-	zip_offset += ZIP_EXTRA_MTIME_SIZE;
+	write_or_die(1, zip_extra_mtime.buf, zip_extra_mtime.len);
+	zip_offset += zip_extra_mtime.len;
 	if (need_zip64_extra) {
 		copy_le16(extra64.magic, 0x0001);
 		copy_le16(extra64.extra_size, ZIP64_EXTRA_PAYLOAD_SIZE);
@@ -546,7 +530,7 @@ static int write_zip_entry(struct archiver_args *args,
 	strbuf_add_le(&zip_dir, 4, attr2);
 	strbuf_add_le(&zip_dir, 4, clamp32(offset));
 	strbuf_add(&zip_dir, path, pathlen);
-	strbuf_add(&zip_dir, &extra, ZIP_EXTRA_MTIME_SIZE);
+	strbuf_addbuf(&zip_dir, &zip_extra_mtime);
 	if (zip64_dir_extra_payload_size) {
 		strbuf_add_le(&zip_dir, 2, 0x0001);	/* magic */
 		strbuf_add_le(&zip_dir, 2, zip64_dir_extra_payload_size);
@@ -644,12 +628,21 @@ static int write_zip_archive(const struct archiver *ar UNUSED,
 	dos_time(&args->time, &zip_date, &zip_time);
 
 	strbuf_init(&zip_dir, 0);
+	strbuf_init(&zip_extra_mtime, 0);
+
+	if (args->time <= 0x7fffffff) {
+		strbuf_add_le(&zip_extra_mtime, 2, 0x5455); /* magic */
+		strbuf_add_le(&zip_extra_mtime, 2, 1 + 4); /* payload size */
+		strbuf_add_le(&zip_extra_mtime, 1, 1); /* flags; just mtime */
+		strbuf_add_le(&zip_extra_mtime, 4, args->time);
+	}
 
 	err = write_archive_entries(args, write_zip_entry);
 	if (!err)
 		write_zip_trailer(args->commit_oid);
 
 	strbuf_release(&zip_dir);
+	strbuf_release(&zip_extra_mtime);
 
 	return err;
 }
diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh
index 768b0ff85d..20bab21048 100755
--- a/t/t5004-archive-corner-cases.sh
+++ b/t/t5004-archive-corner-cases.sh
@@ -227,4 +227,32 @@ test_expect_success 'tar archive with long paths' '
 	git archive -o long_paths.tar $tree
 '
 
+test_lazy_prereq ZIPINFO_UT_EXTRA '
+	"$ZIPINFO" -v "$TEST_DIRECTORY"/t5004/big-pack.zip |
+	grep "UT extra field modtime"
+'
+
+test_expect_success TIME_IS_64BIT 'zip archive with 32-bit mtime' '
+	blob=$(echo foo | git hash-object -w --stdin) &&
+	tree=$(printf "100644 blob $blob\tfile" | git mktree) &&
+	git archive --mtime="@2147483647 +0000" -o with_ut_extra.zip $tree
+'
+
+test_expect_success TIME_IS_64BIT 'zip archive with mtime exceeding 32-bit' '
+	blob=$(echo foo | git hash-object -w --stdin) &&
+	tree=$(printf "100644 blob $blob\tfile" | git mktree) &&
+	git archive --mtime="@2147483648 +0000" -o without_ut_extra.zip $tree
+'
+
+test_expect_success TIME_IS_64BIT,ZIPINFO_UT_EXTRA 'UT extra field' '
+	"$ZIPINFO" -v with_ut_extra.zip >actual &&
+	test_grep "UT extra field modtime" actual
+'
+
+test_expect_success TIME_IS_64BIT,ZIPINFO_UT_EXTRA \
+	'UT extra field omitted when mtime exceeds 32-bit' '
+	"$ZIPINFO" -v without_ut_extra.zip >actual &&
+	test_grep ! "UT extra field modtime" actual
+'
+
 test_done
-- 
2.56.0

Re: [PATCH] archive-zip: omit UT extra field if mtime is out of range

[Junio C Hamano]

René Scharfe <l.s.r@web.de> writes:

> diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh
> index 768b0ff85d..20bab21048 100755
> --- a/t/t5004-archive-corner-cases.sh
> +++ b/t/t5004-archive-corner-cases.sh
> @@ -227,4 +227,32 @@ test_expect_success 'tar archive with long paths' '
>  	git archive -o long_paths.tar $tree
>  '
>  
> +test_lazy_prereq ZIPINFO_UT_EXTRA '
> +	"$ZIPINFO" -v "$TEST_DIRECTORY"/t5004/big-pack.zip |
> +	grep "UT extra field modtime"
> +'
> +
> +test_expect_success TIME_IS_64BIT 'zip archive with 32-bit mtime' '
> +	blob=$(echo foo | git hash-object -w --stdin) &&
> +	tree=$(printf "100644 blob $blob\tfile" | git mktree) &&
> +	git archive --mtime="@2147483647 +0000" -o with_ut_extra.zip $tree
> +'
> +
> +test_expect_success TIME_IS_64BIT 'zip archive with mtime exceeding 32-bit' '
> +	blob=$(echo foo | git hash-object -w --stdin) &&
> +	tree=$(printf "100644 blob $blob\tfile" | git mktree) &&
> +	git archive --mtime="@2147483648 +0000" -o without_ut_extra.zip $tree
> +'

Both Ubuntu/20.02 i386 (which is going away so you do not have to
deeply care) and Debian/latest i386 (which we plan to use to replace
the Ubuntu/20.02 i386, so it would be superb to get it work) jobs
seem to fail this one.

https://github.com/git/git/actions/runs/38189684665/job/114619579067#step:10:321
https://github.com/git/git/actions/runs/38189685137/job/114619583658#step:10:337

Thanks.

> +
> +test_expect_success TIME_IS_64BIT,ZIPINFO_UT_EXTRA 'UT extra field' '
> +	"$ZIPINFO" -v with_ut_extra.zip >actual &&
> +	test_grep "UT extra field modtime" actual
> +'
> +
> +test_expect_success TIME_IS_64BIT,ZIPINFO_UT_EXTRA \
> +	'UT extra field omitted when mtime exceeds 32-bit' '
> +	"$ZIPINFO" -v without_ut_extra.zip >actual &&
> +	test_grep ! "UT extra field modtime" actual
> +'
> +
>  test_done

Re: [PATCH] archive-zip: omit UT extra field if mtime is out of range

[m]

Hi René,

Thanks for the patch. I confirmed that it fixes the 2106 case: before
the change, UT wrapped to zero and unzip -u retained the existing 2025
file; afterward, UT is omitted and the file is replaced correctly.

For the i386 failure Junio reported, the beyond-2038 archive test and
its dependent omission check should use TIME_T_IS_64BIT. TIME_IS_64BIT
checks Git's timestamp_t, which can be 64-bit while time_t is 32-bit.

With that two-line adjustment, all 18 t5004 tests pass on x86-64
Linux. Forcing TIME_T_IS_64BIT off skips only those two tests; the
other 16 pass, including both INT32_MAX checks. I haven't run a native
32-bit build.

Best,
Matthew

On Sun, 11 Oct 2026 18:06:04 +0200, René Scharfe <l.s.r@web.de> wrote:
> We write an Extended Timestamp Extra Field (ID 0x5455 = "UT") for each
> zip entry, like Info-ZIP zip(1) does.  Its time values are signed 32-bit
> integers (see e.g. https://libzip.org/specifications/extrafld.txt).
> Stop emitting them when the commit time doesn't fit in.
>
> Use a strbuf to prepare the extra field up front.  Keep it empty if the
> time value is out of range.  Like for the central zip directory we only
> care about the length-limited buffer aspect of the strbuf, not about
> NUL-termination.  This simplifies the conditional output by avoiding to
> repeat the condition.
>
> Reported-by: Matthew E. Luallen <m@sph3r3.com>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>  archive-zip.c                   | 37 +++++++++++++--------------------
>  t/t5004-archive-corner-cases.sh | 28 +++++++++++++++++++++++++
>  2 files changed, 43 insertions(+), 22 deletions(-)
>
> diff --git a/archive-zip.c b/archive-zip.c
> index 6f73ca6d58..785689584c 100644
> --- a/archive-zip.c
> +++ b/archive-zip.c
> @@ -24,6 +24,7 @@ static int zip_time;
>
>  /* We only care about the "buf" part here. */
>  static struct strbuf zip_dir;
> +static struct strbuf zip_extra_mtime;
>
>  static uintmax_t zip_offset;
>  static uint64_t zip_dir_entries;
> @@ -81,14 +82,6 @@ struct zip_dir_trailer {
>  	unsigned char _end[1];
>  };
>
> -struct zip_extra_mtime {
> -	unsigned char magic[2];
> -	unsigned char extra_size[2];
> -	unsigned char flags[1];
> -	unsigned char mtime[4];
> -	unsigned char _end[1];
> -};
> -
>  struct zip64_extra {
>  	unsigned char magic[2];
>  	unsigned char extra_size[2];
> @@ -129,9 +122,6 @@ struct zip64_dir_trailer_locator {
>  #define ZIP64_DATA_DESC_SIZE	offsetof(struct zip64_data_desc, _end)
>  #define ZIP_DIR_HEADER_SIZE	offsetof(struct zip_dir_header, _end)
>  #define ZIP_DIR_TRAILER_SIZE	offsetof(struct zip_dir_trailer, _end)
> -#define ZIP_EXTRA_MTIME_SIZE	offsetof(struct zip_extra_mtime, _end)
> -#define ZIP_EXTRA_MTIME_PAYLOAD_SIZE \
> -	(ZIP_EXTRA_MTIME_SIZE - offsetof(struct zip_extra_mtime, flags))
>  #define ZIP64_EXTRA_SIZE	offsetof(struct zip64_extra, _end)
>  #define ZIP64_EXTRA_PAYLOAD_SIZE \
>  	(ZIP64_EXTRA_SIZE - offsetof(struct zip64_extra, size))
> @@ -299,9 +289,8 @@ static int write_zip_entry(struct archiver_args *args,
>  {
>  	struct zip_local_header header;
>  	uintmax_t offset = zip_offset;
> -	struct zip_extra_mtime extra;
>  	struct zip64_extra extra64;
> -	size_t header_extra_size = ZIP_EXTRA_MTIME_SIZE;
> +	size_t header_extra_size = zip_extra_mtime.len;
>  	int need_zip64_extra = 0;
>  	unsigned long attr2;
>  	unsigned long compressed_size;
> @@ -315,7 +304,7 @@ static int write_zip_entry(struct archiver_args *args,
>  	const char *path_without_prefix = path + args->baselen;
>  	unsigned int creator_version = 0;
>  	unsigned int version_needed = 10;
> -	size_t zip_dir_extra_size = ZIP_EXTRA_MTIME_SIZE;
> +	size_t zip_dir_extra_size = zip_extra_mtime.len;
>  	size_t zip64_dir_extra_payload_size = 0;
>
>  	crc = crc32(0, NULL, 0);
> @@ -381,11 +370,6 @@ static int write_zip_entry(struct archiver_args *args,
>  		}
>  	}
>
> -	copy_le16(extra.magic, 0x5455);
> -	copy_le16(extra.extra_size, ZIP_EXTRA_MTIME_PAYLOAD_SIZE);
> -	extra.flags[0] = 1;	/* just mtime */
> -	copy_le32(extra.mtime, args->time);
> -
>  	if (size > 0xffffffff || compressed_size > 0xffffffff)
>  		need_zip64_extra = 1;
>  	if (stream && size > 0x7fffffff)
> @@ -412,8 +396,8 @@ static int write_zip_entry(struct archiver_args *args,
>  	zip_offset += ZIP_LOCAL_HEADER_SIZE;
>  	write_or_die(1, path, pathlen);
>  	zip_offset += pathlen;
> -	write_or_die(1, &extra, ZIP_EXTRA_MTIME_SIZE);
> -	zip_offset += ZIP_EXTRA_MTIME_SIZE;
> +	write_or_die(1, zip_extra_mtime.buf, zip_extra_mtime.len);
> +	zip_offset += zip_extra_mtime.len;
>  	if (need_zip64_extra) {
>  		copy_le16(extra64.magic, 0x0001);
>  		copy_le16(extra64.extra_size, ZIP64_EXTRA_PAYLOAD_SIZE);
> @@ -546,7 +530,7 @@ static int write_zip_entry(struct archiver_args *args,
>  	strbuf_add_le(&zip_dir, 4, attr2);
>  	strbuf_add_le(&zip_dir, 4, clamp32(offset));
>  	strbuf_add(&zip_dir, path, pathlen);
> -	strbuf_add(&zip_dir, &extra, ZIP_EXTRA_MTIME_SIZE);
> +	strbuf_addbuf(&zip_dir, &zip_extra_mtime);
>  	if (zip64_dir_extra_payload_size) {
>  		strbuf_add_le(&zip_dir, 2, 0x0001);	/* magic */
>  		strbuf_add_le(&zip_dir, 2, zip64_dir_extra_payload_size);
> @@ -644,12 +628,21 @@ static int write_zip_archive(const struct archiver *ar UNUSED,
>  	dos_time(&args->time, &zip_date, &zip_time);
>
>  	strbuf_init(&zip_dir, 0);
> +	strbuf_init(&zip_extra_mtime, 0);
> +
> +	if (args->time <= 0x7fffffff) {
> +		strbuf_add_le(&zip_extra_mtime, 2, 0x5455); /* magic */
> +		strbuf_add_le(&zip_extra_mtime, 2, 1 + 4); /* payload size */
> +		strbuf_add_le(&zip_extra_mtime, 1, 1); /* flags; just mtime */
> +		strbuf_add_le(&zip_extra_mtime, 4, args->time);
> +	}
>
>  	err = write_archive_entries(args, write_zip_entry);
>  	if (!err)
>  		write_zip_trailer(args->commit_oid);
>
>  	strbuf_release(&zip_dir);
> +	strbuf_release(&zip_extra_mtime);
>
>  	return err;
>  }
> diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh
> index 768b0ff85d..20bab21048 100755
> --- a/t/t5004-archive-corner-cases.sh
> +++ b/t/t5004-archive-corner-cases.sh
> @@ -227,4 +227,32 @@ test_expect_success 'tar archive with long paths' '
>  	git archive -o long_paths.tar $tree
>  '
>
> +test_lazy_prereq ZIPINFO_UT_EXTRA '
> +	"$ZIPINFO" -v "$TEST_DIRECTORY"/t5004/big-pack.zip |
> +	grep "UT extra field modtime"
> +'
> +
> +test_expect_success TIME_IS_64BIT 'zip archive with 32-bit mtime' '
> +	blob=$(echo foo | git hash-object -w --stdin) &&
> +	tree=$(printf "100644 blob $blob\tfile" | git mktree) &&
> +	git archive --mtime="@2147483647 +0000" -o with_ut_extra.zip $tree
> +'
> +
> +test_expect_success TIME_IS_64BIT 'zip archive with mtime exceeding 32-bit' '
> +	blob=$(echo foo | git hash-object -w --stdin) &&
> +	tree=$(printf "100644 blob $blob\tfile" | git mktree) &&
> +	git archive --mtime="@2147483648 +0000" -o without_ut_extra.zip $tree
> +'
> +
> +test_expect_success TIME_IS_64BIT,ZIPINFO_UT_EXTRA 'UT extra field' '
> +	"$ZIPINFO" -v with_ut_extra.zip >actual &&
> +	test_grep "UT extra field modtime" actual
> +'
> +
> +test_expect_success TIME_IS_64BIT,ZIPINFO_UT_EXTRA \
> +	'UT extra field omitted when mtime exceeds 32-bit' '
> +	"$ZIPINFO" -v without_ut_extra.zip >actual &&
> +	test_grep ! "UT extra field modtime" actual
> +'
> +
>  test_done
> --
> 2.56.0

Re: [PATCH] archive-zip: omit UT extra field if mtime is out of range

[René Scharfe]

On 10/12/26 4:08 AM, Junio C Hamano wrote:
> René Scharfe <l.s.r@web.de> writes:
> 
>> diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh
>> index 768b0ff85d..20bab21048 100755
>> --- a/t/t5004-archive-corner-cases.sh
>> +++ b/t/t5004-archive-corner-cases.sh
>> @@ -227,4 +227,32 @@ test_expect_success 'tar archive with long paths' '
>>  	git archive -o long_paths.tar $tree
>>  '
>>  
>> +test_lazy_prereq ZIPINFO_UT_EXTRA '
>> +	"$ZIPINFO" -v "$TEST_DIRECTORY"/t5004/big-pack.zip |
>> +	grep "UT extra field modtime"
>> +'
>> +
>> +test_expect_success TIME_IS_64BIT 'zip archive with 32-bit mtime' '
>> +	blob=$(echo foo | git hash-object -w --stdin) &&
>> +	tree=$(printf "100644 blob $blob\tfile" | git mktree) &&
>> +	git archive --mtime="@2147483647 +0000" -o with_ut_extra.zip $tree
>> +'
>> +
>> +test_expect_success TIME_IS_64BIT 'zip archive with mtime exceeding 32-bit' '
>> +	blob=$(echo foo | git hash-object -w --stdin) &&
>> +	tree=$(printf "100644 blob $blob\tfile" | git mktree) &&
>> +	git archive --mtime="@2147483648 +0000" -o without_ut_extra.zip $tree
>> +'
> 
> Both Ubuntu/20.02 i386 (which is going away so you do not have to
> deeply care) and Debian/latest i386 (which we plan to use to replace
> the Ubuntu/20.02 i386, so it would be superb to get it work) jobs
> seem to fail this one.
> 
> https://github.com/git/git/actions/runs/38189684665/job/114619579067#step:10:321
> https://github.com/git/git/actions/runs/38189685137/job/114619583658#step:10:337
git archive --mtime="@2147483648 +0000" fails in these runs with "fatal:
timestamp too large for this system: 18446744071562067968, which looks
like a preexisting bug.  I suspect the time_t conversion in dos_time().
Will take a deeper look later this week.

René

Re: [PATCH] archive-zip: omit UT extra field if mtime is out of range

[René Scharfe]

On 10/12/26 2:56 PM, m@sph3r3.com wrote:
> Thanks for the patch. I confirmed that it fixes the 2106 case: before
> the change, UT wrapped to zero and unzip -u retained the existing 2025
> file; afterward, UT is omitted and the file is replaced correctly.

Thanks for checking!

> For the i386 failure Junio reported, the beyond-2038 archive test and
> its dependent omission check should use TIME_T_IS_64BIT. TIME_IS_64BIT
> checks Git's timestamp_t, which can be 64-bit while time_t is 32-bit.
> 
> With that two-line adjustment, all 18 t5004 tests pass on x86-64
> Linux. Forcing TIME_T_IS_64BIT off skips only those two tests; the
> other 16 pass, including both INT32_MAX checks. I haven't run a native
> 32-bit build.
That would be an easy way out, nice!  I suspect git archive should not
even use time_t in this context, though.

René

Re: [PATCH] archive-zip: omit UT extra field if mtime is out of range

[Junio C Hamano]

m@sph3r3.com writes:

> For the i386 failure Junio reported, the beyond-2038 archive test and
> its dependent omission check should use TIME_T_IS_64BIT. TIME_IS_64BIT
> checks Git's timestamp_t, which can be 64-bit while time_t is 32-bit.

;-)

Thanks.  It is an obvious thing, only once it is pointed out X-<.

These two prerequisites sound too similar, and the latter probably
is no longer very relevant in the modern world, but that was what
was used in the test.

TIME_T_IS_64BIT measures how wide sizeof(time_t) is, which makes
sense as a portability test across platforms.

But TIME_IS_64BIT?  It measures how wide sizeof(timestamp_t) is, but
the thing is, it is uintmax_t everywhere as far as "git grep" can see.

        $ git grep -n -e 'typedef .*timestamp_t' :/
        compat/posix.h:267:typedef uintmax_t timestamp_t;
        $ git grep -n -e timestamp_t\; :/\*.h
        compat/posix.h:267:typedef uintmax_t timestamp_t;

So TIME_IS_64BIT can be true even if TIME_T_IS_64BIT is not.  Unless
we are on a platform where uintmax_t is 32-bit wide, TIME_IS_64BIT
would be true.  And the time value from the system that is delibered
to us initially in time_t would be limited by that even if timestamp_t
is wider.  TIME_IS_64BIT would not have much value as TIME_T_IS_64BIT
as a portability guard to tests that require wider time values across
the callstack.

Perhaps we should audit places where timestamp_t value is read from
SCM data and coerced into time_t to expose it in a form that outside
world (like ZIP archive format) would understand and warn when we
truncate values?  I will leave it as #leftoverbits for others to
think about it through ;-).

Thanks.