-
Notifications
You must be signed in to change notification settings - Fork 18.5k
Description
CL 173362 was a part of Go 1.14 changes and fixed a crash in net.LookupHost on Windows. It added the test TestLookupNullByte. A part of that test is not compatible with non-Windows ports such as darwin/amd64, causing the test to fail:
$ go test net -run=TestLookupNullByte
--- FAIL: TestLookupNullByte (0.00s)
lookup_test.go:1188: unexpected success
FAIL
FAIL net 0.234s
Marking as release-blocker because it's a regression, and the test failure is very reproducible. The fix will be small.
The test's primary goal is was to ensure net.LookupHost("foo\x00bar") doesn't crash on Windows. Additionally, it happens to check that err != nil on Windows:
// Issue 31586: don't crash on null byte in name
func TestLookupNullByte(t *testing.T) {
testenv.MustHaveExternalNetwork(t)
testenv.SkipFlakyNet(t)
_, err := LookupHost("foo\x00bar") // used to crash on Windows
if err == nil {
t.Errorf("unexpected success")
}
}On darwin/amd64, net.LookupHost("foo\x00bar") on my machine/network returns a nil error even with Go 1.13.7:
package main
import (
"fmt"
"net"
"runtime"
)
func main() {
fmt.Println(runtime.Version())
_, err := net.LookupHost("foo\x00bar")
fmt.Println(err)
}
// Output:
// go1.13.7
// <nil>/cc @bradfitz @golang/osp-team I plan to update the test so it either runs only on Windows, or relax the err == nil check. Let me know if there are preferences between those two fixes.
/cc @bradfitz Is it normal that net.LookupHost("foo\x00bar") can succeed on macOS? If it's not expected, I believe a separate issue should be filed for that.