cmd/pack: fix export data truncation bug

The binary export data format includes escaping to prevent "\n$$" from
appearing internally, but not "\n!\n". This could result in a false
positive when cmd/pack searched for "\n!\n" as the delimiter between
package definition and linker object.

To address this, this CL changes cmd/pack to also be aware of the
"\n$$" markers, and to ignore "\n!\n" within the export data.

Fixes #21703.

Change-Id: I71ea8ba49dbd066c7afb7717ddc0190e38fe5649
Reviewed-on: https://go-review.googlesource.com/60773
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/cmd/pack/pack.go b/src/cmd/pack/pack.go
index 1c168f946bd04ed4701e5561d6b08ed4611b01c4..3abc83e0900d2029bc91ca0db689b89a9b489204 100644 (file)
--- a/src/cmd/pack/pack.go
+++ b/src/cmd/pack/pack.go
@@ -426,8 +426,15 @@ func readPkgdef(file string) (data []byte, err error) {
        // Read from file, collecting header for __.PKGDEF.
        // The header is from the beginning of the file until a line
        // containing just "!". The first line must begin with "go object ".
+       //
+       // Note: It's possible for "\n!\n" to appear within the binary
+       // package export data format. To avoid truncating the package
+       // definition prematurely (issue 21703), we keep keep track of
+       // how many "$$" delimiters we've seen.
+
        rbuf := bufio.NewReader(f)
        var wbuf bytes.Buffer
+       markers := 0
        for {
                line, err := rbuf.ReadBytes('\n')
                if err != nil {
@@ -436,9 +443,12 @@ func readPkgdef(file string) (data []byte, err error) {
                if wbuf.Len() == 0 && !bytes.HasPrefix(line, []byte("go object ")) {
                        return nil, errors.New("not a Go object file")
                }
-               if bytes.Equal(line, []byte("!\n")) {
+               if markers%2 == 0 && bytes.Equal(line, []byte("!\n")) {
                        break
                }
+               if bytes.HasPrefix(line, []byte("$$")) {
+                       markers++
+               }
                wbuf.Write(line)
        }
        return wbuf.Bytes(), nil

diff --git a/src/cmd/pack/pack_test.go b/src/cmd/pack/pack_test.go
index 79d9cde292a71562b355764a01cac825c83a3f4b..b2217c090fd3989f16040e3a8be38ca1d24b633b 100644 (file)
--- a/src/cmd/pack/pack_test.go
+++ b/src/cmd/pack/pack_test.go
@@ -295,6 +295,37 @@ func TestLargeDefs(t *testing.T) {
        }
 }
 
+// Test that "\n!\n" inside export data doesn't result in a truncated
+// package definition when creating a .a archive from a .o Go object.
+func TestIssue21703(t *testing.T) {
+       testenv.MustHaveGoBuild(t)
+
+       dir := tmpDir(t)
+       defer os.RemoveAll(dir)
+
+       const aSrc = `package a; const X = "\n!\n"`
+       err := ioutil.WriteFile(filepath.Join(dir, "a.go"), []byte(aSrc), 0666)
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       const bSrc = `package b; import _ "a"`
+       err = ioutil.WriteFile(filepath.Join(dir, "b.go"), []byte(bSrc), 0666)
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       run := func(args ...string) string {
+               return doRun(t, dir, args...)
+       }
+
+       goBin := testenv.GoToolPath(t)
+       run(goBin, "build", "cmd/pack") // writes pack binary to dir
+       run(goBin, "tool", "compile", "a.go")
+       run("./pack", "c", "a.a", "a.o")
+       run(goBin, "tool", "compile", "-I", ".", "b.go")
+}
+
 // doRun runs a program in a directory and returns the output.
 func doRun(t *testing.T, dir string, args ...string) string {
        cmd := exec.Command(args[0], args[1:]...)