runtime/debug: replace (*BuildInfo).Marshal methods with Parse and String

Since a String method cannot return an error, escape fields that may
contain unsanitized values, and unescape them during parsing.

Add a fuzz test to verify that calling the String method on any
BuildInfo returned by Parse produces a string that parses to the same
BuildInfo. (Note that this doesn't ensure that String always produces
a parseable input: we assume that a user constructing a BuildInfo
provides valid paths and versions, so we don't bother to escape those.
It also doesn't ensure that ParseBuildInfo accepts all inputs that
ought to be valid.)

Fixes #51026

Change-Id: Ida18010ce47622cfedb1494060f32bd7705df014
Reviewed-on: https://go-review.googlesource.com/c/go/+/384154
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>

api/go1.18.txt

@@ -165,8 +165,8 @@ pkg reflect, method (Value) FieldByIndexErr([]int) (Value, error)
 pkg reflect, method (Value) SetIterKey(*MapIter)
 pkg reflect, method (Value) SetIterValue(*MapIter)
 pkg reflect, method (Value) UnsafePointer() unsafe.Pointer
-pkg runtime/debug, method (*BuildInfo) MarshalText() ([]uint8, error)
+pkg runtime/debug, func ParseBuildInfo(string) (*BuildInfo, error)
-pkg runtime/debug, method (*BuildInfo) UnmarshalText([]uint8) error
+pkg runtime/debug, method (*BuildInfo) String() string
 pkg runtime/debug, type BuildInfo struct, GoVersion string
 pkg runtime/debug, type BuildInfo struct, Settings []BuildSetting
 pkg runtime/debug, type BuildSetting struct

src/cmd/go/internal/load/pkg.go

@@ -2229,13 +2229,17 @@ func (p *Package) setBuildInfo() {
 
 	var debugModFromModinfo func(*modinfo.ModulePublic) *debug.Module
 	debugModFromModinfo = func(mi *modinfo.ModulePublic) *debug.Module {
+		version := mi.Version
+		if version == "" {
+			version = "(devel)"
+		}
 		dm := &debug.Module{
 			Path:    mi.Path,
-			Version: mi.Version,
+			Version: version,
 		}
 		if mi.Replace != nil {
 			dm.Replace = debugModFromModinfo(mi.Replace)
-		} else {
+		} else if mi.Version != "" {
 			dm.Sum = modfetch.Sum(module.Version{Path: mi.Path, Version: mi.Version})
 		}
 		return dm
@@ -2418,12 +2422,7 @@ func (p *Package) setBuildInfo() {
 		appendSetting("vcs.modified", strconv.FormatBool(st.Uncommitted))
 	}
 
-	text, err := info.MarshalText()
+	p.Internal.BuildInfo = info.String()
-	if err != nil {
-		setPkgErrorf("error formatting build info: %v", err)
-		return
-	}
-	p.Internal.BuildInfo = string(text)
 }
 
 // SafeArg reports whether arg is a "safe" command-line argument,

src/cmd/go/internal/version/version.go

@@ -6,7 +6,6 @@
 package version
 
 import (
-	"bytes"
 	"context"
 	"debug/buildinfo"
 	"errors"
@@ -156,12 +155,8 @@ func scanFile(file string, info fs.FileInfo, mustPrint bool) {
 
 	fmt.Printf("%s: %s\n", file, bi.GoVersion)
 	bi.GoVersion = "" // suppress printing go version again
-	mod, err := bi.MarshalText()
+	mod := bi.String()
-	if err != nil {
-		fmt.Fprintf(os.Stderr, "%s: formatting build info: %v\n", file, err)
-		return
-	}
 	if *versionM && len(mod) > 0 {
-		fmt.Printf("\t%s\n", bytes.ReplaceAll(mod[:len(mod)-1], []byte("\n"), []byte("\n\t")))
+		fmt.Printf("\t%s\n", strings.ReplaceAll(mod[:len(mod)-1], "\n", "\n\t"))
 	}
 }

src/debug/buildinfo/buildinfo.go

@@ -75,8 +75,8 @@ func Read(r io.ReaderAt) (*BuildInfo, error) {
 	if err != nil {
 		return nil, err
 	}
-	bi := &BuildInfo{}
+	bi, err := debug.ParseBuildInfo(mod)
-	if err := bi.UnmarshalText([]byte(mod)); err != nil {
+	if err != nil {
 		return nil, err
 	}
 	bi.GoVersion = vers

src/debug/buildinfo/buildinfo_test.go

@@ -212,12 +212,10 @@ func TestReadFile(t *testing.T) {
 					} else {
 						if tc.wantErr != "" {
 							t.Fatalf("unexpected success; want error containing %q", tc.wantErr)
-						} else if got, err := info.MarshalText(); err != nil {
+						}
-							t.Fatalf("unexpected error marshaling BuildInfo: %v", err)
+						got := info.String()
-						} else if got := cleanOutputForComparison(string(got)); got != tc.want {
+						if clean := cleanOutputForComparison(string(got)); got != tc.want && clean != tc.want {
-							if got != tc.want {
+							t.Fatalf("got:\n%s\nwant:\n%s", got, tc.want)
-								t.Fatalf("got:\n%s\nwant:\n%s", got, tc.want)
-							}
 						}
 					}
 				})

src/runtime/debug/mod.go

@@ -5,9 +5,9 @@
 package debug
 
 import (
-	"bytes"
 	"fmt"
 	"runtime"
+	"strconv"
 	"strings"
 )
 
@@ -23,8 +23,8 @@ func ReadBuildInfo() (info *BuildInfo, ok bool) {
 		return nil, false
 	}
 	data = data[16 : len(data)-16]
-	bi := &BuildInfo{}
+	bi, err := ParseBuildInfo(data)
-	if err := bi.UnmarshalText([]byte(data)); err != nil {
+	if err != nil {
 		return nil, false
 	}
 
@@ -63,8 +63,18 @@ type BuildSetting struct {
 	Key, Value string
 }
 
-func (bi *BuildInfo) MarshalText() ([]byte, error) {
+// quoteKey reports whether key is required to be quoted.
-	buf := &bytes.Buffer{}
+func quoteKey(key string) bool {
+	return len(key) == 0 || strings.ContainsAny(key, "= \t\r\n\"`")
+}
+
+// quoteValue reports whether value is required to be quoted.
+func quoteValue(value string) bool {
+	return strings.ContainsAny(value, " \t\r\n\"`")
+}
+
+func (bi *BuildInfo) String() string {
+	buf := new(strings.Builder)
 	if bi.GoVersion != "" {
 		fmt.Fprintf(buf, "go\t%s\n", bi.GoVersion)
 	}
@@ -76,12 +86,8 @@ func (bi *BuildInfo) MarshalText() ([]byte, error) {
 		buf.WriteString(word)
 		buf.WriteByte('\t')
 		buf.WriteString(m.Path)
-		mv := m.Version
-		if mv == "" {
-			mv = "(devel)"
-		}
 		buf.WriteByte('\t')
-		buf.WriteString(mv)
+		buf.WriteString(m.Version)
 		if m.Replace == nil {
 			buf.WriteByte('\t')
 			buf.WriteString(m.Sum)
@@ -91,27 +97,28 @@ func (bi *BuildInfo) MarshalText() ([]byte, error) {
 		}
 		buf.WriteByte('\n')
 	}
-	if bi.Main.Path != "" {
+	if bi.Main != (Module{}) {
 		formatMod("mod", bi.Main)
 	}
 	for _, dep := range bi.Deps {
 		formatMod("dep", *dep)
 	}
 	for _, s := range bi.Settings {
-		if strings.ContainsAny(s.Key, "= \t\n") {
+		key := s.Key
-			return nil, fmt.Errorf("invalid build setting key %q", s.Key)
+		if quoteKey(key) {
+			key = strconv.Quote(key)
 		}
-		if strings.Contains(s.Value, "\n") {
+		value := s.Value
-			return nil, fmt.Errorf("invalid build setting value for key %q: contains newline", s.Value)
+		if quoteValue(value) {
+			value = strconv.Quote(value)
 		}
-		fmt.Fprintf(buf, "build\t%s=%s\n", s.Key, s.Value)
+		fmt.Fprintf(buf, "build\t%s=%s\n", key, value)
 	}
 
-	return buf.Bytes(), nil
+	return buf.String()
 }
 
-func (bi *BuildInfo) UnmarshalText(data []byte) (err error) {
+func ParseBuildInfo(data string) (bi *BuildInfo, err error) {
-	*bi = BuildInfo{}
 	lineNum := 1
 	defer func() {
 		if err != nil {
@@ -120,67 +127,69 @@ func (bi *BuildInfo) UnmarshalText(data []byte) (err error) {
 	}()
 
 	var (
-		pathLine  = []byte("path\t")
+		pathLine  = "path\t"
-		modLine   = []byte("mod\t")
+		modLine   = "mod\t"
-		depLine   = []byte("dep\t")
+		depLine   = "dep\t"
-		repLine   = []byte("=>\t")
+		repLine   = "=>\t"
-		buildLine = []byte("build\t")
+		buildLine = "build\t"
-		newline   = []byte("\n")
+		newline   = "\n"
-		tab       = []byte("\t")
+		tab       = "\t"
 	)
 
-	readModuleLine := func(elem [][]byte) (Module, error) {
+	readModuleLine := func(elem []string) (Module, error) {
 		if len(elem) != 2 && len(elem) != 3 {
 			return Module{}, fmt.Errorf("expected 2 or 3 columns; got %d", len(elem))
 		}
+		version := elem[1]
 		sum := ""
 		if len(elem) == 3 {
-			sum = string(elem[2])
+			sum = elem[2]
 		}
 		return Module{
-			Path:    string(elem[0]),
+			Path:    elem[0],
-			Version: string(elem[1]),
+			Version: version,
 			Sum:     sum,
 		}, nil
 	}
 
+	bi = new(BuildInfo)
 	var (
 		last *Module
-		line []byte
+		line string
 		ok   bool
 	)
 	// Reverse of BuildInfo.String(), except for go version.
 	for len(data) > 0 {
-		line, data, ok = bytes.Cut(data, newline)
+		line, data, ok = strings.Cut(data, newline)
 		if !ok {
 			break
 		}
 		switch {
-		case bytes.HasPrefix(line, pathLine):
+		case strings.HasPrefix(line, pathLine):
 			elem := line[len(pathLine):]
 			bi.Path = string(elem)
-		case bytes.HasPrefix(line, modLine):
+		case strings.HasPrefix(line, modLine):
-			elem := bytes.Split(line[len(modLine):], tab)
+			elem := strings.Split(line[len(modLine):], tab)
 			last = &bi.Main
 			*last, err = readModuleLine(elem)
 			if err != nil {
-				return err
+				return nil, err
 			}
-		case bytes.HasPrefix(line, depLine):
+		case strings.HasPrefix(line, depLine):
-			elem := bytes.Split(line[len(depLine):], tab)
+			elem := strings.Split(line[len(depLine):], tab)
 			last = new(Module)
 			bi.Deps = append(bi.Deps, last)
 			*last, err = readModuleLine(elem)
 			if err != nil {
-				return err
+				return nil, err
 			}
-		case bytes.HasPrefix(line, repLine):
+		case strings.HasPrefix(line, repLine):
-			elem := bytes.Split(line[len(repLine):], tab)
+			elem := strings.Split(line[len(repLine):], tab)
 			if len(elem) != 3 {
-				return fmt.Errorf("expected 3 columns for replacement; got %d", len(elem))
+				return nil, fmt.Errorf("expected 3 columns for replacement; got %d", len(elem))
 			}
 			if last == nil {
-				return fmt.Errorf("replacement with no module on previous line")
+				return nil, fmt.Errorf("replacement with no module on previous line")
 			}
 			last.Replace = &Module{
 				Path:    string(elem[0]),
@@ -188,17 +197,63 @@ func (bi *BuildInfo) UnmarshalText(data []byte) (err error) {
 				Sum:     string(elem[2]),
 			}
 			last = nil
-		case bytes.HasPrefix(line, buildLine):
+		case strings.HasPrefix(line, buildLine):
-			key, val, ok := strings.Cut(string(line[len(buildLine):]), "=")
+			kv := line[len(buildLine):]
-			if !ok {
+			if len(kv) < 1 {
-				return fmt.Errorf("invalid build line")
+				return nil, fmt.Errorf("build line missing '='")
 			}
-			if key == "" {
+
-				return fmt.Errorf("empty key")
+			var key, rawValue string
+			switch kv[0] {
+			case '=':
+				return nil, fmt.Errorf("build line with missing key")
+
+			case '`', '"':
+				rawKey, err := strconv.QuotedPrefix(kv)
+				if err != nil {
+					return nil, fmt.Errorf("invalid quoted key in build line")
+				}
+				if len(kv) == len(rawKey) {
+					return nil, fmt.Errorf("build line missing '=' after quoted key")
+				}
+				if c := kv[len(rawKey)]; c != '=' {
+					return nil, fmt.Errorf("unexpected character after quoted key: %q", c)
+				}
+				key, _ = strconv.Unquote(rawKey)
+				rawValue = kv[len(rawKey)+1:]
+
+			default:
+				var ok bool
+				key, rawValue, ok = strings.Cut(kv, "=")
+				if !ok {
+					return nil, fmt.Errorf("build line missing '=' after key")
+				}
+				if quoteKey(key) {
+					return nil, fmt.Errorf("unquoted key %q must be quoted", key)
+				}
 			}
-			bi.Settings = append(bi.Settings, BuildSetting{Key: key, Value: val})
+
+			var value string
+			if len(rawValue) > 0 {
+				switch rawValue[0] {
+				case '`', '"':
+					var err error
+					value, err = strconv.Unquote(rawValue)
+					if err != nil {
+						return nil, fmt.Errorf("invalid quoted value in build line")
+					}
+
+				default:
+					value = rawValue
+					if quoteValue(value) {
+						return nil, fmt.Errorf("unquoted value %q must be quoted", value)
+					}
+				}
+			}
+
+			bi.Settings = append(bi.Settings, BuildSetting{Key: key, Value: value})
 		}
 		lineNum++
 	}
-	return nil
+	return bi, nil
 }

src/runtime/debug/mod_test.go

@@ -0,0 +1,75 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package debug_test
+
+import (
+	"reflect"
+	"runtime/debug"
+	"strings"
+	"testing"
+)
+
+// strip removes two leading tabs after each newline of s.
+func strip(s string) string {
+	replaced := strings.ReplaceAll(s, "\n\t\t", "\n")
+	if len(replaced) > 0 && replaced[0] == '\n' {
+		replaced = replaced[1:]
+	}
+	return replaced
+}
+
+func FuzzParseBuildInfoRoundTrip(f *testing.F) {
+	// Package built from outside a module, missing some fields..
+	f.Add(strip(`
+		path	rsc.io/fortune
+		mod	rsc.io/fortune	v1.0.0
+		`))
+
+	// Package built from the standard library, missing some fields..
+	f.Add(`path	cmd/test2json`)
+
+	// Package built from inside a module.
+	f.Add(strip(`
+		go	1.18
+		path	example.com/m
+		mod	example.com/m	(devel)	
+		build	-compiler=gc
+		`))
+
+	// Package built in GOPATH mode.
+	f.Add(strip(`
+		go	1.18
+		path	example.com/m
+		build	-compiler=gc
+		`))
+
+	// Escaped build info.
+	f.Add(strip(`
+		go 1.18
+		path example.com/m
+		build CRAZY_ENV="requires\nescaping"
+		`))
+
+	f.Fuzz(func(t *testing.T, s string) {
+		bi, err := debug.ParseBuildInfo(s)
+		if err != nil {
+			// Not a round-trippable BuildInfo string.
+			t.Log(err)
+			return
+		}
+
+		// s2 could have different escaping from s.
+		// However, it should parse to exactly the same contents.
+		s2 := bi.String()
+		bi2, err := debug.ParseBuildInfo(s2)
+		if err != nil {
+			t.Fatalf("%v:\n%s", err, s2)
+		}
+
+		if !reflect.DeepEqual(bi2, bi) {
+			t.Fatalf("Parsed representation differs.\ninput:\n%s\noutput:\n%s", s, s2)
+		}
+	})
+}