archive/zip: fix zip64 extra headers problems

[philip-firstorder]

The central-directory extra fields can contain either 8, 16 or 24 data bytes for ONLY the fields > uint32max (0xFFFFFFFF).

They should not be all set when either CompressedSize, UncompressedSize or offset > uint32max

Fixes #33116

[gopherbot]

This PR (HEAD: c0c91c9) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/186178 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

This PR (HEAD: 5333827) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/186178 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

Message from Gobot Gobot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
Within the next week or so, a maintainer will review your change and provide
feedback. See https://golang.org/doc/contribute.html#review for more info and
tips to get your patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11, it means that this CL will be reviewed as part of the next development
cycle. See https://golang.org/s/release for more details.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186178.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Emmanuel Odeke:

Patch Set 2: Run-TryBot+1

(1 comment)

Thank you for this change Philip and apologies that we didn't
take a look at this during the easier paced Go1.14 cycle.

Great catch and I have added suggestions but also for such
changes we need to add tests to catch any regressions and failures
that could creep in. Also it would be great if you could add
references in the ZIP specification to why we are making these
changes.

(You'll have to manually edit the Github description for the
commit message changes to reflect here).

I have created for you a test for this change, please expand on it
to exhaustively cover all the cases you made, here it is, please
add it to zip_test.go:

// Test that we don't unnecessarily add extraneous fields like
// CompressedSize to extra. See Issue https://golang.org/issue/33116.
func TestExtraneousHeaders(t *testing.T) {
tests := []struct {
name string
compressedSize uint32
uncompressedSize uint32
compressedSize64 uint64
uncompressedSize64 uint64
want *File
}{
{
name: "greater than 4.9Gb",
compressedSize64: uint32max + 2,
uncompressedSize64: uint32max + 2,
want: &File{
FileHeader: FileHeader{
CompressedSize: 0xffffffff,
UncompressedSize: 0xffffffff,
CompressedSize64: 0x100000001,
UncompressedSize64: 0x100000001,
Modified: time.Date(1979, 11, 30, 0, 0, 0, 0, time.UTC),
Extra: []byte{
// The little endian serialized bytes of extras.
0x01, 0x00, // ZIPExtraID.
0x10, 0x00, // ZIPExtraSize: 8 + 8
0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, // UncompressedSize64
0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, // CompressedSize64
},
},
},
},
}

for _, tt := range tests {
	t.Run(tt.name, func(t *testing.T) {
		buf := new(bytes.Buffer)
		br := bufio.NewWriter(buf)
		w := &Writer{
			cw: &countWriter{count: 0, w: br},
			dir: []*header{
				{
					FileHeader: &FileHeader{
						CompressedSize:     tt.compressedSize,
						UncompressedSize:   tt.uncompressedSize,
						CompressedSize64:   tt.compressedSize64,
						UncompressedSize64: tt.uncompressedSize64,
					},
				},
			},
		}

		if err := w.Close(); err != nil {
			t.Fatal(err)
		}

		// read it back
		zr, err := NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len()))
		if err != nil {
			t.Fatal(err)
		}
		if g, w := len(zr.File), 1; g != w {
			t.Fatalf("Invalid files count: got %d, want %d", g, w)
		}
		want := tt.want
		// Some fields are impossible to predict beforehand
		// yet we need them for equivalence checks.
		got := zr.File[0]
		want.zip = got.zip
		want.zipr = got.zipr
		want.zipsize = int64(buf.Len())
		if !reflect.DeepEqual(got, tt.want) {
			gotBlob, _ := json.MarshalIndent(got, "", "  ")
			wantBlob, _ := json.MarshalIndent(tt.want, "", "  ")
			t.Fatalf("FileHeader mismatch\nGot:\n%s\n%+v\n\n\nWant:\n%s\n%+v\n\n", gotBlob, got, wantBlob, tt.want)
		}
	})
}

}

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186178.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Gobot Gobot:

Patch Set 2:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=85ec2dbb

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186178.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Gobot Gobot:

Patch Set 2: TryBot-Result+1

TryBots are happy.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186178.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Emmanuel Odeke:

Patch Set 2:

Here are some relevant parts of the ZIP specification
that we can reference in your commit:

Document: https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT

• CompressedSize and UncompressedSize being stored in zip64
  format iff file size > 0xFFFFFFFF (uint32max): Section 4.3.9.2

but note that that same section 4.3.9.2 says:
"However ZIP64 format MAY be used regardless of the size of a file."

which is the current behavior. With that bit from the specification, we
know that this new behavior is a nice-to-have but since other uncompressors
are tripping out, we'll try to conform to them.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186178.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Go Bot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
Within the next week or so, a maintainer will review your change and provide
feedback. See https://golang.org/doc/contribute.html#review for more info and
tips to get your patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11, it means that this CL will be reviewed as part of the next development
cycle. See https://golang.org/s/release for more details.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186178.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Go Bot:

Patch Set 2:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=85ec2dbb

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186178.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Go Bot:

Patch Set 2: TryBot-Result+1

TryBots are happy.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186178.
After addressing review feedback, remember to publish your drafts!