]> Cypherpunks repositories - gostls13.git/commitdiff
runtime/debug: replace (*BuildInfo).Marshal methods with Parse and String
authorBryan C. Mills <bcmills@google.com>
Tue, 8 Feb 2022 17:23:50 +0000 (12:23 -0500)
committerBryan Mills <bcmills@google.com>
Wed, 9 Feb 2022 19:44:03 +0000 (19:44 +0000)
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
src/cmd/go/internal/load/pkg.go
src/cmd/go/internal/version/version.go
src/debug/buildinfo/buildinfo.go
src/debug/buildinfo/buildinfo_test.go
src/runtime/debug/mod.go
src/runtime/debug/mod_test.go [new file with mode: 0644]

index 7a81ce259e8679c66d7970c1a77442d1a7cb09a0..0f3e26df9d329656578b683eeb4e5bc51f9a8157 100644 (file)
@@ -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, method (*BuildInfo) UnmarshalText([]uint8) error
+pkg runtime/debug, func ParseBuildInfo(string) (*BuildInfo, 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
index fca9d5a0a296c83a4e3c236b0b779adead038134..214502da7cd06cd23fe9c01190eb60b1f436d250 100644 (file)
@@ -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()
-       if err != nil {
-               setPkgErrorf("error formatting build info: %v", err)
-               return
-       }
-       p.Internal.BuildInfo = string(text)
+       p.Internal.BuildInfo = info.String()
 }
 
 // SafeArg reports whether arg is a "safe" command-line argument,
index 52502e95c6d673eed8ec1b1a203ba9b9b55bd268..1c0eb5407d960d78ae725c3371d459d617a64467 100644 (file)
@@ -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()
-       if err != nil {
-               fmt.Fprintf(os.Stderr, "%s: formatting build info: %v\n", file, err)
-               return
-       }
+       mod := bi.String()
        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"))
        }
 }
index 2c0200e8dcb161c19a2bf295a72431a436ebaf5c..8de03ff1063ffb1feb34d09ffd8f774e9fce303d 100644 (file)
@@ -75,8 +75,8 @@ func Read(r io.ReaderAt) (*BuildInfo, error) {
        if err != nil {
                return nil, err
        }
-       bi := &BuildInfo{}
-       if err := bi.UnmarshalText([]byte(mod)); err != nil {
+       bi, err := debug.ParseBuildInfo(mod)
+       if err != nil {
                return nil, err
        }
        bi.GoVersion = vers
index 8346be01095c4ef43267494641882fc5194c57ac..ac71626fda1dfb74be7f869053e548b9b7e9403a 100644 (file)
@@ -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)
-                                               } else if got := cleanOutputForComparison(string(got)); got != tc.want {
-                                                       if got != tc.want {
-                                                               t.Fatalf("got:\n%s\nwant:\n%s", got, tc.want)
-                                                       }
+                                               }
+                                               got := info.String()
+                                               if clean := cleanOutputForComparison(string(got)); got != tc.want && clean != tc.want {
+                                                       t.Fatalf("got:\n%s\nwant:\n%s", got, tc.want)
                                                }
                                        }
                                })
index 14a496a8eb3390de381ad884cd9a049460d7391f..688e2581ed89e66238e7e07ef92134cc193b6315 100644 (file)
@@ -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{}
-       if err := bi.UnmarshalText([]byte(data)); err != nil {
+       bi, err := ParseBuildInfo(data)
+       if err != nil {
                return nil, false
        }
 
@@ -63,8 +63,18 @@ type BuildSetting struct {
        Key, Value string
 }
 
-func (bi *BuildInfo) MarshalText() ([]byte, error) {
-       buf := &bytes.Buffer{}
+// quoteKey reports whether key is required to be quoted.
+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") {
-                       return nil, fmt.Errorf("invalid build setting key %q", s.Key)
+               key := s.Key
+               if quoteKey(key) {
+                       key = strconv.Quote(key)
                }
-               if strings.Contains(s.Value, "\n") {
-                       return nil, fmt.Errorf("invalid build setting value for key %q: contains newline", s.Value)
+               value := 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) {
-       *bi = BuildInfo{}
+func ParseBuildInfo(data string) (bi *BuildInfo, err error) {
        lineNum := 1
        defer func() {
                if err != nil {
@@ -120,67 +127,69 @@ func (bi *BuildInfo) UnmarshalText(data []byte) (err error) {
        }()
 
        var (
-               pathLine  = []byte("path\t")
-               modLine   = []byte("mod\t")
-               depLine   = []byte("dep\t")
-               repLine   = []byte("=>\t")
-               buildLine = []byte("build\t")
-               newline   = []byte("\n")
-               tab       = []byte("\t")
+               pathLine  = "path\t"
+               modLine   = "mod\t"
+               depLine   = "dep\t"
+               repLine   = "=>\t"
+               buildLine = "build\t"
+               newline   = "\n"
+               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]),
-                       Version: string(elem[1]),
+                       Path:    elem[0],
+                       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):
-                       elem := bytes.Split(line[len(modLine):], tab)
+               case strings.HasPrefix(line, modLine):
+                       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):
-                       elem := bytes.Split(line[len(depLine):], tab)
+               case strings.HasPrefix(line, depLine):
+                       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):
-                       elem := bytes.Split(line[len(repLine):], tab)
+               case strings.HasPrefix(line, repLine):
+                       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):
-                       key, val, ok := strings.Cut(string(line[len(buildLine):]), "=")
-                       if !ok {
-                               return fmt.Errorf("invalid build line")
+               case strings.HasPrefix(line, buildLine):
+                       kv := line[len(buildLine):]
+                       if len(kv) < 1 {
+                               return nil, fmt.Errorf("build line missing '='")
+                       }
+
+                       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)
+                               }
                        }
-                       if key == "" {
-                               return fmt.Errorf("empty key")
+
+                       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: val})
+
+                       bi.Settings = append(bi.Settings, BuildSetting{Key: key, Value: value})
                }
                lineNum++
        }
-       return nil
+       return bi, nil
 }
diff --git a/src/runtime/debug/mod_test.go b/src/runtime/debug/mod_test.go
new file mode 100644 (file)
index 0000000..b291769
--- /dev/null
@@ -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)
+               }
+       })
+}