fmt: Scanf accepts invalid boolean values

[andydotxyz]

What version of Go are you using (go version)?

$ go version
1.15.5

Does this issue reproduce with the latest release?

Not checked 1.15.6 yet, sorry

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/andy/Library/Caches/go-build"
GOENV="/Users/andy/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/andy/Code/Go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/andy/Code/Go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.15.5/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.15.5/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/andy/Code/Go/src/fyne.io/fyne/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/rk/wps4hdmx73ldvbq2dbprd_3c0000gn/T/go-build914664370=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Tried to parse certain invalid boolean values using fmt.Sscanf and a "%t" format.
Using the following code to test certain input strings

	err, n := fmt.Sscanf("true", "%t", &val)
	fmt.Println("true:", val, n, err)

What did you expect to see?

true: true 1
tBu: true syntax error scanning boolean 0
trB: true syntax error scanning boolean 0
false: false 1
fRl: false syntax error scanning boolean 0
faR: false syntax error scanning boolean 0
foo: false syntax error scanning boolean 0

What did you see instead?

true: true 1
tBu: true 1
trB: true syntax error scanning boolean 0
false: false 1
fRl: false 1
faR: false syntax error scanning boolean 0
foo: false 1

[andydotxyz]

looking at scan.go I see the function func (s *ss) scanBool(verb rune) bool

ends without an error:

	return false
}

should be

	s.error(boolError)
	return false
}

Additionally, true and false parsers look wrong,

	case 't', 'T':
		if s.accept("rR") && (!s.accept("uU") || !s.accept("eE")) {
			s.error(boolError)
		}
		return true

should be

	case 't', 'T':
		if !s.accept("rR") || !s.accept("uU") || !s.accept("eE") {
			s.error(boolError)
		}
		return true

[seankhliao]

Can you provide a complete, runnable example? your example code and the expected input/output doesn't match

[andydotxyz]

package main

import "fmt"

func main() {
	var val bool
	err, n := fmt.Sscanf("true", "%t", &val)
	fmt.Println("true:", val, n, err)
	err, n = fmt.Sscanf("tBu", "%t", &val)
	fmt.Println("tBu:", val, n, err)
	err, n = fmt.Sscanf("trB", "%t", &val)
	fmt.Println("trB:", val, n, err)

	err, n = fmt.Sscanf("false", "%t", &val)
	fmt.Println("false:", val, n, err)
	err, n = fmt.Sscanf("fRl", "%t", &val)
	fmt.Println("fRl:", val, n, err)
	err, n = fmt.Sscanf("faR", "%t", &val)
	fmt.Println("faR:", val, n, err)

	err, n = fmt.Sscanf("foo", "%t", &val)
	fmt.Println("foo:", val, n, err)
}

[seankhliao]

I think you mixed up expected vs actual in the report. Additionally the value of val is kept between cases where there's a syntax error, a correct test case (with fixed n/err) would be:

func main() {
	ss := []string{"true", "tBu", "trB", "false", "fRl", "faR", "foo"}
	for _, s := range ss {
		var val bool
		n, err := fmt.Sscanf(s, "%t", &val)
		fmt.Println(s, val, n, err)
	}
}

with output

true true 1 <nil>
tBu true 1 <nil>
trB false 0 syntax error scanning boolean
false false 1 <nil>
fRl false 1 <nil>
faR false 0 syntax error scanning boolean
foo false 1 <nil>

The current logic accepts single letter T / F for true/false and only expecting the long form if the second letter also matches, allowing for things such as the following, changing it would be a breaking change

func main() {
	var v bool
	var w int
	fmt.Println(fmt.Sscanf("t1", "%t%d", &v, &w)) // 2 <nil>
	fmt.Println(v, w)                             // true 1
}

[andydotxyz]

(updated the summary of the ticket, the outputs were the wrong way round)

Fixing the single item logic may indeed be breaking, I had not realised.

I guess the bigger issue, however, is that "garbage" will parse as a valid false value.

[robpike]

The Scanf code doesn't try to handle all possible errors, and there is a comment in there to that effect. Scanf isn't a strong input verifier, and isn't meant to be. You should only use it when you want to do something simple like read integers from the user's input command line.

If you're looking for "true", look for "true", don't ask Scanf to verify it. Scanf is slow and tricky to use well for anything much beyond reading an int or float, so if you know the format of your input, you should use and/or write a proper lexer and/or parser.

That said, a clean, simple fix to Scanf that can be more rigorous about parsing booleans would be welcome, but both those adjectives matter. (And you can't add any dependencies to the package.) Assuming of course that being rigorous won't break existing code.

[robpike]

(For those following along at home, as I've said before, Scanf shouldn't even be in the library, being so much below the quality bar we would expect today. But the compatibility guarantee keeps it there.)