Skip to content

Commit 9cec77a

Browse files
author
Bryan C. Mills
committed
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>
1 parent be0d049 commit 9cec77a

File tree

7 files changed

+198
-76
lines changed

7 files changed

+198
-76
lines changed

api/go1.18.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,8 @@ pkg reflect, method (Value) FieldByIndexErr([]int) (Value, error)
165165
pkg reflect, method (Value) SetIterKey(*MapIter)
166166
pkg reflect, method (Value) SetIterValue(*MapIter)
167167
pkg reflect, method (Value) UnsafePointer() unsafe.Pointer
168-
pkg runtime/debug, method (*BuildInfo) MarshalText() ([]uint8, error)
169-
pkg runtime/debug, method (*BuildInfo) UnmarshalText([]uint8) error
168+
pkg runtime/debug, func ParseBuildInfo(string) (*BuildInfo, error)
169+
pkg runtime/debug, method (*BuildInfo) String() string
170170
pkg runtime/debug, type BuildInfo struct, GoVersion string
171171
pkg runtime/debug, type BuildInfo struct, Settings []BuildSetting
172172
pkg runtime/debug, type BuildSetting struct

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

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2229,13 +2229,17 @@ func (p *Package) setBuildInfo() {
22292229

22302230
var debugModFromModinfo func(*modinfo.ModulePublic) *debug.Module
22312231
debugModFromModinfo = func(mi *modinfo.ModulePublic) *debug.Module {
2232+
version := mi.Version
2233+
if version == "" {
2234+
version = "(devel)"
2235+
}
22322236
dm := &debug.Module{
22332237
Path: mi.Path,
2234-
Version: mi.Version,
2238+
Version: version,
22352239
}
22362240
if mi.Replace != nil {
22372241
dm.Replace = debugModFromModinfo(mi.Replace)
2238-
} else {
2242+
} else if mi.Version != "" {
22392243
dm.Sum = modfetch.Sum(module.Version{Path: mi.Path, Version: mi.Version})
22402244
}
22412245
return dm
@@ -2418,12 +2422,7 @@ func (p *Package) setBuildInfo() {
24182422
appendSetting("vcs.modified", strconv.FormatBool(st.Uncommitted))
24192423
}
24202424

2421-
text, err := info.MarshalText()
2422-
if err != nil {
2423-
setPkgErrorf("error formatting build info: %v", err)
2424-
return
2425-
}
2426-
p.Internal.BuildInfo = string(text)
2425+
p.Internal.BuildInfo = info.String()
24272426
}
24282427

24292428
// SafeArg reports whether arg is a "safe" command-line argument,

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
package version
77

88
import (
9-
"bytes"
109
"context"
1110
"debug/buildinfo"
1211
"errors"
@@ -156,12 +155,8 @@ func scanFile(file string, info fs.FileInfo, mustPrint bool) {
156155

157156
fmt.Printf("%s: %s\n", file, bi.GoVersion)
158157
bi.GoVersion = "" // suppress printing go version again
159-
mod, err := bi.MarshalText()
160-
if err != nil {
161-
fmt.Fprintf(os.Stderr, "%s: formatting build info: %v\n", file, err)
162-
return
163-
}
158+
mod := bi.String()
164159
if *versionM && len(mod) > 0 {
165-
fmt.Printf("\t%s\n", bytes.ReplaceAll(mod[:len(mod)-1], []byte("\n"), []byte("\n\t")))
160+
fmt.Printf("\t%s\n", strings.ReplaceAll(mod[:len(mod)-1], "\n", "\n\t"))
166161
}
167162
}

src/debug/buildinfo/buildinfo.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,8 @@ func Read(r io.ReaderAt) (*BuildInfo, error) {
7575
if err != nil {
7676
return nil, err
7777
}
78-
bi := &BuildInfo{}
79-
if err := bi.UnmarshalText([]byte(mod)); err != nil {
78+
bi, err := debug.ParseBuildInfo(mod)
79+
if err != nil {
8080
return nil, err
8181
}
8282
bi.GoVersion = vers

src/debug/buildinfo/buildinfo_test.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -212,12 +212,10 @@ func TestReadFile(t *testing.T) {
212212
} else {
213213
if tc.wantErr != "" {
214214
t.Fatalf("unexpected success; want error containing %q", tc.wantErr)
215-
} else if got, err := info.MarshalText(); err != nil {
216-
t.Fatalf("unexpected error marshaling BuildInfo: %v", err)
217-
} else if got := cleanOutputForComparison(string(got)); got != tc.want {
218-
if got != tc.want {
219-
t.Fatalf("got:\n%s\nwant:\n%s", got, tc.want)
220-
}
215+
}
216+
got := info.String()
217+
if clean := cleanOutputForComparison(string(got)); got != tc.want && clean != tc.want {
218+
t.Fatalf("got:\n%s\nwant:\n%s", got, tc.want)
221219
}
222220
}
223221
})

src/runtime/debug/mod.go

Lines changed: 106 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55
package debug
66

77
import (
8-
"bytes"
98
"fmt"
109
"runtime"
10+
"strconv"
1111
"strings"
1212
)
1313

@@ -23,8 +23,8 @@ func ReadBuildInfo() (info *BuildInfo, ok bool) {
2323
return nil, false
2424
}
2525
data = data[16 : len(data)-16]
26-
bi := &BuildInfo{}
27-
if err := bi.UnmarshalText([]byte(data)); err != nil {
26+
bi, err := ParseBuildInfo(data)
27+
if err != nil {
2828
return nil, false
2929
}
3030

@@ -63,8 +63,18 @@ type BuildSetting struct {
6363
Key, Value string
6464
}
6565

66-
func (bi *BuildInfo) MarshalText() ([]byte, error) {
67-
buf := &bytes.Buffer{}
66+
// quoteKey reports whether key is required to be quoted.
67+
func quoteKey(key string) bool {
68+
return len(key) == 0 || strings.ContainsAny(key, "= \t\r\n\"`")
69+
}
70+
71+
// quoteValue reports whether value is required to be quoted.
72+
func quoteValue(value string) bool {
73+
return strings.ContainsAny(value, " \t\r\n\"`")
74+
}
75+
76+
func (bi *BuildInfo) String() string {
77+
buf := new(strings.Builder)
6878
if bi.GoVersion != "" {
6979
fmt.Fprintf(buf, "go\t%s\n", bi.GoVersion)
7080
}
@@ -76,12 +86,8 @@ func (bi *BuildInfo) MarshalText() ([]byte, error) {
7686
buf.WriteString(word)
7787
buf.WriteByte('\t')
7888
buf.WriteString(m.Path)
79-
mv := m.Version
80-
if mv == "" {
81-
mv = "(devel)"
82-
}
8389
buf.WriteByte('\t')
84-
buf.WriteString(mv)
90+
buf.WriteString(m.Version)
8591
if m.Replace == nil {
8692
buf.WriteByte('\t')
8793
buf.WriteString(m.Sum)
@@ -91,27 +97,28 @@ func (bi *BuildInfo) MarshalText() ([]byte, error) {
9197
}
9298
buf.WriteByte('\n')
9399
}
94-
if bi.Main.Path != "" {
100+
if bi.Main != (Module{}) {
95101
formatMod("mod", bi.Main)
96102
}
97103
for _, dep := range bi.Deps {
98104
formatMod("dep", *dep)
99105
}
100106
for _, s := range bi.Settings {
101-
if strings.ContainsAny(s.Key, "= \t\n") {
102-
return nil, fmt.Errorf("invalid build setting key %q", s.Key)
107+
key := s.Key
108+
if quoteKey(key) {
109+
key = strconv.Quote(key)
103110
}
104-
if strings.Contains(s.Value, "\n") {
105-
return nil, fmt.Errorf("invalid build setting value for key %q: contains newline", s.Value)
111+
value := s.Value
112+
if quoteValue(value) {
113+
value = strconv.Quote(value)
106114
}
107-
fmt.Fprintf(buf, "build\t%s=%s\n", s.Key, s.Value)
115+
fmt.Fprintf(buf, "build\t%s=%s\n", key, value)
108116
}
109117

110-
return buf.Bytes(), nil
118+
return buf.String()
111119
}
112120

113-
func (bi *BuildInfo) UnmarshalText(data []byte) (err error) {
114-
*bi = BuildInfo{}
121+
func ParseBuildInfo(data string) (bi *BuildInfo, err error) {
115122
lineNum := 1
116123
defer func() {
117124
if err != nil {
@@ -120,85 +127,133 @@ func (bi *BuildInfo) UnmarshalText(data []byte) (err error) {
120127
}()
121128

122129
var (
123-
pathLine = []byte("path\t")
124-
modLine = []byte("mod\t")
125-
depLine = []byte("dep\t")
126-
repLine = []byte("=>\t")
127-
buildLine = []byte("build\t")
128-
newline = []byte("\n")
129-
tab = []byte("\t")
130+
pathLine = "path\t"
131+
modLine = "mod\t"
132+
depLine = "dep\t"
133+
repLine = "=>\t"
134+
buildLine = "build\t"
135+
newline = "\n"
136+
tab = "\t"
130137
)
131138

132-
readModuleLine := func(elem [][]byte) (Module, error) {
139+
readModuleLine := func(elem []string) (Module, error) {
133140
if len(elem) != 2 && len(elem) != 3 {
134141
return Module{}, fmt.Errorf("expected 2 or 3 columns; got %d", len(elem))
135142
}
143+
version := elem[1]
136144
sum := ""
137145
if len(elem) == 3 {
138-
sum = string(elem[2])
146+
sum = elem[2]
139147
}
140148
return Module{
141-
Path: string(elem[0]),
142-
Version: string(elem[1]),
149+
Path: elem[0],
150+
Version: version,
143151
Sum: sum,
144152
}, nil
145153
}
146154

155+
bi = new(BuildInfo)
147156
var (
148157
last *Module
149-
line []byte
158+
line string
150159
ok bool
151160
)
152161
// Reverse of BuildInfo.String(), except for go version.
153162
for len(data) > 0 {
154-
line, data, ok = bytes.Cut(data, newline)
163+
line, data, ok = strings.Cut(data, newline)
155164
if !ok {
156165
break
157166
}
158167
switch {
159-
case bytes.HasPrefix(line, pathLine):
168+
case strings.HasPrefix(line, pathLine):
160169
elem := line[len(pathLine):]
161170
bi.Path = string(elem)
162-
case bytes.HasPrefix(line, modLine):
163-
elem := bytes.Split(line[len(modLine):], tab)
171+
case strings.HasPrefix(line, modLine):
172+
elem := strings.Split(line[len(modLine):], tab)
164173
last = &bi.Main
165174
*last, err = readModuleLine(elem)
166175
if err != nil {
167-
return err
176+
return nil, err
168177
}
169-
case bytes.HasPrefix(line, depLine):
170-
elem := bytes.Split(line[len(depLine):], tab)
178+
case strings.HasPrefix(line, depLine):
179+
elem := strings.Split(line[len(depLine):], tab)
171180
last = new(Module)
172181
bi.Deps = append(bi.Deps, last)
173182
*last, err = readModuleLine(elem)
174183
if err != nil {
175-
return err
184+
return nil, err
176185
}
177-
case bytes.HasPrefix(line, repLine):
178-
elem := bytes.Split(line[len(repLine):], tab)
186+
case strings.HasPrefix(line, repLine):
187+
elem := strings.Split(line[len(repLine):], tab)
179188
if len(elem) != 3 {
180-
return fmt.Errorf("expected 3 columns for replacement; got %d", len(elem))
189+
return nil, fmt.Errorf("expected 3 columns for replacement; got %d", len(elem))
181190
}
182191
if last == nil {
183-
return fmt.Errorf("replacement with no module on previous line")
192+
return nil, fmt.Errorf("replacement with no module on previous line")
184193
}
185194
last.Replace = &Module{
186195
Path: string(elem[0]),
187196
Version: string(elem[1]),
188197
Sum: string(elem[2]),
189198
}
190199
last = nil
191-
case bytes.HasPrefix(line, buildLine):
192-
key, val, ok := strings.Cut(string(line[len(buildLine):]), "=")
193-
if !ok {
194-
return fmt.Errorf("invalid build line")
200+
case strings.HasPrefix(line, buildLine):
201+
kv := line[len(buildLine):]
202+
if len(kv) < 1 {
203+
return nil, fmt.Errorf("build line missing '='")
204+
}
205+
206+
var key, rawValue string
207+
switch kv[0] {
208+
case '=':
209+
return nil, fmt.Errorf("build line with missing key")
210+
211+
case '`', '"':
212+
rawKey, err := strconv.QuotedPrefix(kv)
213+
if err != nil {
214+
return nil, fmt.Errorf("invalid quoted key in build line")
215+
}
216+
if len(kv) == len(rawKey) {
217+
return nil, fmt.Errorf("build line missing '=' after quoted key")
218+
}
219+
if c := kv[len(rawKey)]; c != '=' {
220+
return nil, fmt.Errorf("unexpected character after quoted key: %q", c)
221+
}
222+
key, _ = strconv.Unquote(rawKey)
223+
rawValue = kv[len(rawKey)+1:]
224+
225+
default:
226+
var ok bool
227+
key, rawValue, ok = strings.Cut(kv, "=")
228+
if !ok {
229+
return nil, fmt.Errorf("build line missing '=' after key")
230+
}
231+
if quoteKey(key) {
232+
return nil, fmt.Errorf("unquoted key %q must be quoted", key)
233+
}
195234
}
196-
if key == "" {
197-
return fmt.Errorf("empty key")
235+
236+
var value string
237+
if len(rawValue) > 0 {
238+
switch rawValue[0] {
239+
case '`', '"':
240+
var err error
241+
value, err = strconv.Unquote(rawValue)
242+
if err != nil {
243+
return nil, fmt.Errorf("invalid quoted value in build line")
244+
}
245+
246+
default:
247+
value = rawValue
248+
if quoteValue(value) {
249+
return nil, fmt.Errorf("unquoted value %q must be quoted", value)
250+
}
251+
}
198252
}
199-
bi.Settings = append(bi.Settings, BuildSetting{Key: key, Value: val})
253+
254+
bi.Settings = append(bi.Settings, BuildSetting{Key: key, Value: value})
200255
}
201256
lineNum++
202257
}
203-
return nil
258+
return bi, nil
204259
}

0 commit comments

Comments
 (0)