FAQ
Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
archive/zip: handle extra data headers with no body

Fixes issue 4393.

Please review this at http://codereview.appspot.com/6854058/

Affected files:
M src/pkg/archive/zip/reader.go
M src/pkg/archive/zip/zip_test.go


Index: src/pkg/archive/zip/reader.go
===================================================================
--- a/src/pkg/archive/zip/reader.go
+++ b/src/pkg/archive/zip/reader.go
@@ -238,7 +238,7 @@

if len(f.Extra) > 0 {
b := readBuf(f.Extra)
- for len(b) > 4 { // need at least tag and size
+ for len(b) > 3 { // need at least tag and size
tag := b.uint16()
size := b.uint16()
if int(size) > len(b) {
Index: src/pkg/archive/zip/zip_test.go
===================================================================
--- a/src/pkg/archive/zip/zip_test.go
+++ b/src/pkg/archive/zip/zip_test.go
@@ -195,6 +195,27 @@
}
}

+func testValidHeader(h *FileHeader, t *testing.T) {
+ var buf bytes.Buffer
+ z := NewWriter(&buf)
+
+ f, err := z.CreateHeader(h)
+ if err != nil {
+ t.Fatalf("error creating header: %v", err)
+ }
+ if _, err := f.Write([]byte("hi")); err != nil {
+ t.Fatalf("error writing content: %v", err)
+ }
+ if err := z.Close(); err != nil {
+ t.Fatal("error closing zip writer: %v", err)
+ }
+
+ b := buf.Bytes()
+ if _, err = NewReader(bytes.NewReader(b), int64(len(b))); err != nil {
+ t.Fatal("got %v, expected nil", err)
+ }
+}
+
// Issue 4302.
func TestHeaderInvalidTagAndSize(t *testing.T) {
const timeFormat = "20060102T150405.000.txt"
@@ -220,3 +241,17 @@
}
testInvalidHeader(&h, t)
}
+
+// Issue 4393. It is valid to have an extra data header
+// which contains no body.
+func TestZeroLengthHeader(t *testing.T) {
+ h := FileHeader{
+ Name: "extadata.txt",
+ Method: Deflate,
+ Extra: []byte{
+ 85, 84, 5, 0, 3, 154, 144, 195, 77, // tag 21589 size 5
+ 85, 120, 0, 0, // tag 30805 size 0
+ },
+ }
+ testValidHeader(&h, t)
+}

Search Discussions

  • Dave at Nov 17, 2012 at 2:45 am
    Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

    Please take another look.


    http://codereview.appspot.com/6854058/
  • Brad Fitzpatrick at Nov 17, 2012 at 6:35 am
    LGTM

    I'd write >= 4 when about to read at least 4 bytes, but > 3 works too.

    On Thu, Nov 15, 2012 at 4:54 PM, wrote:

    Reviewers: golang-dev_googlegroups.com,

    Message:
    Hello golang-dev@googlegroups.com,

    I'd like you to review this change to
    https://go.googlecode.com/hg/


    Description:
    archive/zip: handle extra data headers with no body

    Fixes issue 4393.

    Please review this at http://codereview.appspot.com/**6854058/<http://codereview.appspot.com/6854058/>

    Affected files:
    M src/pkg/archive/zip/reader.go
    M src/pkg/archive/zip/zip_test.**go


    Index: src/pkg/archive/zip/reader.go
    ==============================**==============================**=======
    --- a/src/pkg/archive/zip/reader.**go
    +++ b/src/pkg/archive/zip/reader.**go
    @@ -238,7 +238,7 @@

    if len(f.Extra) > 0 {
    b := readBuf(f.Extra)
    - for len(b) > 4 { // need at least tag and size
    + for len(b) > 3 { // need at least tag and size
    tag := b.uint16()
    size := b.uint16()
    if int(size) > len(b) {
    Index: src/pkg/archive/zip/zip_test.**go
    ==============================**==============================**=======
    --- a/src/pkg/archive/zip/zip_**test.go
    +++ b/src/pkg/archive/zip/zip_**test.go
    @@ -195,6 +195,27 @@
    }
    }

    +func testValidHeader(h *FileHeader, t *testing.T) {
    + var buf bytes.Buffer
    + z := NewWriter(&buf)
    +
    + f, err := z.CreateHeader(h)
    + if err != nil {
    + t.Fatalf("error creating header: %v", err)
    + }
    + if _, err := f.Write([]byte("hi")); err != nil {
    + t.Fatalf("error writing content: %v", err)
    + }
    + if err := z.Close(); err != nil {
    + t.Fatal("error closing zip writer: %v", err)
    + }
    +
    + b := buf.Bytes()
    + if _, err = NewReader(bytes.NewReader(b), int64(len(b))); err !=
    nil {
    + t.Fatal("got %v, expected nil", err)
    + }
    +}
    +
    // Issue 4302.
    func TestHeaderInvalidTagAndSize(t *testing.T) {
    const timeFormat = "20060102T150405.000.txt"
    @@ -220,3 +241,17 @@
    }
    testInvalidHeader(&h, t)
    }
    +
    +// Issue 4393. It is valid to have an extra data header
    +// which contains no body.
    +func TestZeroLengthHeader(t *testing.T) {
    + h := FileHeader{
    + Name: "extadata.txt",
    + Method: Deflate,
    + Extra: []byte{
    + 85, 84, 5, 0, 3, 154, 144, 195, 77, // tag 21589
    size 5
    + 85, 120, 0, 0, // tag 30805 size 0
    + },
    + }
    + testValidHeader(&h, t)
    +}

  • Dave at Nov 17, 2012 at 1:47 pm
    *** Submitted as
    http://code.google.com/p/go/source/detail?r=a26a8ada8f6e ***

    archive/zip: handle extra data headers with no body

    Fixes issue 4393.

    R=golang-dev, bradfitz
    CC=golang-dev
    http://codereview.appspot.com/6854058


    http://codereview.appspot.com/6854058/

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedNov 16, '12 at 12:54a
activeNov 17, '12 at 1:47p
posts4
users2
websitegolang.org

2 users in discussion

Dave: 3 posts Brad Fitzpatrick: 1 post

People

Translate

site design / logo © 2021 Grokbase