archive/zip: update extended timestamp field if exists

Fixes golang#61572

src/archive/zip/writer.go

@@ -16,8 +16,9 @@ import (
 )
 
 var (
-	errLongName  = errors.New("zip: FileHeader.Name too long")
-	errLongExtra = errors.New("zip: FileHeader.Extra too long")
+	errLongName     = errors.New("zip: FileHeader.Name too long")
+	errLongExtra    = errors.New("zip: FileHeader.Extra too long")
+	errInvalidExtra = errors.New("zip: invalid FileHeader.Extra")
 )
 
 // Writer implements a zip file writer.
@@ -326,7 +327,16 @@ func (w *Writer) CreateHeader(fh *FileHeader) (io.Writer, error) {
 		eb.uint16(5)  // Size: SizeOf(uint8) + SizeOf(uint32)
 		eb.uint8(1)   // Flags: ModTime
 		eb.uint32(mt) // ModTime
-		fh.Extra = append(fh.Extra, mbuf[:]...)
+
+		offset, err := extendedTimestampOffset(fh.Extra)
+		if err != nil {
+			return nil, err
+		}
+		if offset != -1 {
+			copy(fh.Extra[offset:], mbuf[:])
+		} else {
+			fh.Extra = append(fh.Extra, mbuf[:]...)
+		}
 	}
 
 	var (
@@ -383,6 +393,25 @@ func (w *Writer) CreateHeader(fh *FileHeader) (io.Writer, error) {
 	return ow, nil
 }
 
+func extendedTimestampOffset(extra []byte) (offset int, _ error) {
+	for buf := readBuf(extra); len(buf) >= 4; { // need at least tag and size
+		fieldTag := buf.uint16()
+		fieldSize := int(buf.uint16())
+		if len(buf) < fieldSize {
+			return -1, errInvalidExtra
+		}
+		if fieldTag == extTimeExtraID {
+			if fieldSize != 5 {
+				return -1, errInvalidExtra
+			}
+			return offset, nil
+		}
+		buf.sub(fieldSize)
+		offset += 4 + fieldSize
+	}
+	return -1, nil
+}
+
 func writeHeader(w io.Writer, h *header) error {
 	const maxUint16 = 1<<16 - 1
 	if len(h.Name) > maxUint16 {

src/archive/zip/zip_test.go

@@ -826,3 +826,159 @@ func (zeros) Read(p []byte) (int, error) {
 	}
 	return len(p), nil
 }
+
+// Issue 61572
+func TestWriterModifiedTime(t *testing.T) {
+	modified := time.Date(2023, 07, 27, 00, 41, 57, 0, timeZone(+1*time.Hour))
+	extendedTimestampField := []byte{
+		// tag 0x5455 size 5, extended timestamp, encodes modified
+		0x55, 0x54, 5, 0, 0x01, 0x45, 0xaf, 0xc1, 0x64,
+	}
+
+	for _, tc := range []struct {
+		name        string
+		modified    time.Time
+		extra, want []byte
+	}{
+		{
+			name:     "adds extended timestamp to empty extra",
+			modified: modified,
+			extra:    nil,
+			want:     extendedTimestampField,
+		},
+		{
+			name:     "adds extended timestamp if does not exist",
+			modified: modified,
+			extra: []byte{
+				// tag 0x9999 size 5
+				0x99, 0x99, 5, 0, 0, 1, 2, 3, 4,
+				// tag 0xaaaa size 0
+				0xaa, 0xaa, 0, 0,
+			},
+			want: append([]byte{
+				0x99, 0x99, 5, 0, 0, 1, 2, 3, 4,
+				0xaa, 0xaa, 0, 0},
+				extendedTimestampField...),
+		},
+		{
+			name:     "updates extended timestamp if exists",
+			modified: modified,
+			extra: []byte{
+				// tag 0x9999 size 5
+				0x99, 0x99, 5, 0, 0, 1, 2, 3, 4,
+				// tag 0x5455 size 5, extended timestamp, encodes time.Date(2017, 10, 31, 21, 11, 57, 0, timeZone(-7*time.Hour))
+				0x55, 0x54, 5, 0, 0x01, 0x8d, 0x49, 0xf9, 0x59,
+				// tag 0xaaaa size 0
+				0xaa, 0xaa, 0, 0,
+			},
+			want: append(append([]byte{
+				0x99, 0x99, 5, 0, 0, 1, 2, 3, 4},
+				extendedTimestampField...),
+				0xaa, 0xaa, 0, 0),
+		},
+		{
+			name:     "does not update extended timestamp if not changed",
+			modified: modified,
+			extra: append(append([]byte{
+				0x99, 0x99, 5, 0, 0, 1, 2, 3, 4},
+				extendedTimestampField...),
+				0xaa, 0xaa, 0, 0),
+			want: append(append([]byte{
+				0x99, 0x99, 5, 0, 0, 1, 2, 3, 4},
+				extendedTimestampField...),
+				0xaa, 0xaa, 0, 0),
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			h := &FileHeader{
+				Name:     "issue61572.txt",
+				Modified: tc.modified,
+				Extra:    tc.extra,
+			}
+
+			var buf bytes.Buffer
+			w := NewWriter(&buf)
+			if _, err := w.CreateHeader(h); err != nil {
+				t.Fatalf("unexpected CreateHeader error: %v", err)
+			}
+			if err := w.Close(); err != nil {
+				t.Fatalf("unexpected Close error: %v", err)
+			}
+
+			in := buf.Bytes()
+			zf, err := NewReader(bytes.NewReader(in), int64(len(in)))
+			if err != nil {
+				t.Fatalf("unexpected NewReader error: %v", err)
+			}
+
+			if got := zf.File[0].FileHeader.Extra; !bytes.Equal(tc.want, got) {
+				t.Logf("want: % x", tc.want)
+				t.Logf(" got: % x", got)
+				t.Error("wrong header extra")
+			}
+		})
+	}
+}
+
+func TestWriterModifiedTimeInvalidExtra(t *testing.T) {
+	for _, tc := range []struct {
+		name    string
+		extra   []byte
+		wanterr error
+	}{
+		{
+			name: "truncated non-timestamp field",
+			extra: []byte{
+				// tag 0x9999 size 5, truncated
+				0x99, 0x99, 5, 0, 0, 1, 2, // 3, 4,
+			},
+		},
+		{
+			name: "truncated extended timestamp field",
+			extra: []byte{
+				// tag 0x5455 size 5, extended timestamp, truncated
+				0x55, 0x54, 5, 0, 0x01, 0x45, //0xaf, 0xc1, 0x64,
+			},
+		},
+		{
+			name: "wrong size extended timestamp field",
+			extra: []byte{
+				// tag 0x5455, extended timestamp, wrong size
+				0x55, 0x54, 6, 0, 0x01, 0x45, 0xaf, 0xc1, 0x64, 0xFF,
+			},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			h := &FileHeader{
+				Name:     "issue61572.txt",
+				Modified: time.Date(2023, 07, 27, 00, 41, 57, 0, timeZone(+1*time.Hour)),
+				Extra:    tc.extra,
+			}
+
+			var buf bytes.Buffer
+			w := NewWriter(&buf)
+
+			_, err := w.CreateHeader(h)
+			if err != errInvalidExtra {
+				t.Errorf("unexpected error %v, want %v", err, errInvalidExtra)
+			}
+		})
+	}
+}
+
+func TestWriterShortExtra(t *testing.T) {
+	var buf bytes.Buffer
+	h := &FileHeader{
+		Name:     "test.txt",
+		Modified: time.Date(2017, 10, 31, 21, 11, 57, 0, timeZone(-7*time.Hour)),
+		Extra: []byte{
+			// tag 0x9999 size 5, truncated
+			0x99, 0x99, 5, 0, 0, 1, 2, // 3, 4,
+		},
+	}
+	w := NewWriter(&buf)
+	_, err := w.CreateHeader(h)
+	if err != errInvalidExtra {
+		t.Errorf("error=%v, want %v", err, errInvalidExtra)
+	}
+}