[release-branch.go1.4] cmd/go: avoid use of bufio.Scanner in generate

««« CL 182970043 / 573a7b5178c4
cmd/go: avoid use of bufio.Scanner in generate

Scanner can't handle stupid long lines and there are
reports of stupid long lines in production.

Note the issue isn't long "//go:generate" lines, but
any long line in any Go source file.

To be fair, if you're going to have a stupid long line
it's not a bad bet you'll want to run it through go
generate, because it's some embeddable asset that
has been machine generated. (One could ask why
that generation process didn't add a newline or two,
but we should cope anyway.)

Rewrite the file scanner in "go generate" so it can
handle arbitrarily long lines, and only stores in memory
those lines that start "//go:generate".

Also: Adjust the documentation to make clear that it
does not parse the file.

Fixes #9143.
Fixes #9196.

LGTM=rsc, dominik.honnef
R=rsc, cespare, minux, dominik.honnef
CC=golang-codereviews
https://golang.org/cl/182970043
»»»

TBR=r
CC=golang-codereviews
https://golang.org/cl/183060044

diff --git a/src/cmd/go/doc.go b/src/cmd/go/doc.go
index 879fc7f8ba6907c499b1c2e28b98e8654684fc4c..65640fb483bcbef4940e1bff575fb77382ca2188 100644 (file)
--- a/src/cmd/go/doc.go
+++ b/src/cmd/go/doc.go
@@ -234,17 +234,24 @@ create or update Go source files, for instance by running yacc.
 Go generate is never run automatically by go build, go get, go test,
 and so on. It must be run explicitly.
 
-Directives are written as a whole-line comment of the form
+Go generate scans the file for directives, which are lines of
+the form,
 
        //go:generate command argument...
 
-(note: no space in "//go") where command is the generator to be
-run, corresponding to an executable file that can be run locally.
-It must either be in the shell path (gofmt), a fully qualified path
-(/usr/you/bin/mytool), or a command alias, described below.
+(note: no leading spaces and no space in "//go") where command
+is the generator to be run, corresponding to an executable file
+that can be run locally. It must either be in the shell path
+(gofmt), a fully qualified path (/usr/you/bin/mytool), or a
+command alias, described below.
 
-The arguments are space-separated tokens or double-quoted strings
-passed to the generator as individual arguments when it is run.
+Note that go generate does not parse the file, so lines that look
+like directives in comments or multiline strings will be treated
+as directives.
+
+The arguments to the directive are space-separated tokens or
+double-quoted strings passed to the generator as individual
+arguments when it is run.
 
 Quoted strings use Go syntax and are evaluated before execution; a
 quoted string appears as a single argument to the generator.

diff --git a/src/cmd/go/generate.go b/src/cmd/go/generate.go
index 2772452dd52ab7a9ee636eb402a71bf1d4f9e42e..88f7efa0f34224462cd1c8cc3bbaaca8d9897c3b 100644 (file)
--- a/src/cmd/go/generate.go
+++ b/src/cmd/go/generate.go
@@ -32,17 +32,24 @@ create or update Go source files, for instance by running yacc.
 Go generate is never run automatically by go build, go get, go test,
 and so on. It must be run explicitly.
 
-Directives are written as a whole-line comment of the form
+Go generate scans the file for directives, which are lines of
+the form,
 
        //go:generate command argument...
 
-(note: no space in "//go") where command is the generator to be
-run, corresponding to an executable file that can be run locally.
-It must either be in the shell path (gofmt), a fully qualified path
-(/usr/you/bin/mytool), or a command alias, described below.
+(note: no leading spaces and no space in "//go") where command
+is the generator to be run, corresponding to an executable file
+that can be run locally. It must either be in the shell path
+(gofmt), a fully qualified path (/usr/you/bin/mytool), or a
+command alias, described below.
 
-The arguments are space-separated tokens or double-quoted strings
-passed to the generator as individual arguments when it is run.
+Note that go generate does not parse the file, so lines that look
+like directives in comments or multiline strings will be treated
+as directives.
+
+The arguments to the directive are space-separated tokens or
+double-quoted strings passed to the generator as individual
+arguments when it is run.
 
 Quoted strings use Go syntax and are evaluated before execution; a
 quoted string appears as a single argument to the generator.
@@ -178,13 +185,43 @@ func (g *Generator) run() (ok bool) {
                fmt.Fprintf(os.Stderr, "%s\n", shortPath(g.path))
        }
 
-       s := bufio.NewScanner(g.r)
-       for s.Scan() {
-               g.lineNum++
-               if !bytes.HasPrefix(s.Bytes(), []byte("//go:generate ")) && !bytes.HasPrefix(s.Bytes(), []byte("//go:generate\t")) {
+       // Scan for lines that start "//go:generate".
+       // Can't use bufio.Scanner because it can't handle long lines,
+       // which are likely to appear when using generate.
+       input := bufio.NewReader(g.r)
+       var err error
+       // One line per loop.
+       for {
+               g.lineNum++ // 1-indexed.
+               var buf []byte
+               buf, err = input.ReadSlice('\n')
+               if err == bufio.ErrBufferFull {
+                       // Line too long - consume and ignore.
+                       if isGoGenerate(buf) {
+                               g.errorf("directive too long")
+                       }
+                       for err == bufio.ErrBufferFull {
+                               _, err = input.ReadSlice('\n')
+                       }
+                       if err != nil {
+                               break
+                       }
+                       continue
+               }
+
+               if err != nil {
+                       // Check for marker at EOF without final \n.
+                       if err == io.EOF && isGoGenerate(buf) {
+                               err = io.ErrUnexpectedEOF
+                       }
+                       break
+               }
+
+               if !isGoGenerate(buf) {
                        continue
                }
-               words := g.split(s.Text())
+
+               words := g.split(string(buf))
                if len(words) == 0 {
                        g.errorf("no arguments to directive")
                }
@@ -201,19 +238,23 @@ func (g *Generator) run() (ok bool) {
                }
                g.exec(words)
        }
-       if s.Err() != nil {
-               g.errorf("error reading %s: %s", shortPath(g.path), s.Err())
+       if err != nil && err != io.EOF {
+               g.errorf("error reading %s: %s", shortPath(g.path), err)
        }
        return true
 }
 
+func isGoGenerate(buf []byte) bool {
+       return bytes.HasPrefix(buf, []byte("//go:generate ")) || bytes.HasPrefix(buf, []byte("//go:generate\t"))
+}
+
 // split breaks the line into words, evaluating quoted
 // strings and evaluating environment variables.
 // The initial //go:generate element is dropped.
 func (g *Generator) split(line string) []string {
        // Parse line, obeying quoted strings.
        var words []string
-       line = line[len("//go:generate "):]
+       line = line[len("//go:generate ") : len(line)-1] // Drop preamble and final newline.
        // One (possibly quoted) word per iteration.
 Words:
        for {