From f1d63d8a787c8d829bfd9bd7525b77174ff68fe1 Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Wed, 19 May 2021 09:16:22 -0700 Subject: [PATCH 1/2] fuzz: refactor out helpers The fuzz function had gotten large and repetitive. Create helpers. Signed-off-by: Josh Bleecher Snyder --- fuzz.go | 245 +++++++++++++++++++++++++++++--------------------------- 1 file changed, 126 insertions(+), 119 deletions(-) diff --git a/fuzz.go b/fuzz.go index 07bfc79..9b7f5ad 100644 --- a/fuzz.go +++ b/fuzz.go @@ -8,8 +8,10 @@ package netaddr import ( "bytes" + "encoding" "fmt" "net" + "reflect" "strings" ) @@ -18,33 +20,8 @@ func Fuzz(b []byte) int { ip, err := ParseIP(s) if err == nil { - s2 := ip.String() - // There's no guarantee that ip.String() will match s. - // But a round trip the other direction ought to succeed. - ip2, err := ParseIP(s2) - if err != nil { - panic(err) - } - if ip2 != ip { - fmt.Printf("ip=%#v ip2=%#v\n", ip, ip2) - panic("IP round trip identity failure") - } - if s2 != ip2.String() { - panic("IP String round trip identity failure") - } - - b, err := ip.MarshalBinary() - if err != nil { - panic(err) - } - var ip3 IP - if err := ip3.UnmarshalBinary(b); err != nil { - panic(err) - } - if ip != ip3 { - fmt.Printf("ip=%#v ip3=%#v\n", ip, ip3) - panic("IP binary marshal round trip identity failure") - } + checkStringParseRoundTrip(ip, parseIP) + checkEncoding(ip) } // Check that we match the standard library's IP parser, modulo zones. if !strings.Contains(s, "%") { @@ -67,107 +44,137 @@ func Fuzz(b []byte) int { panic(".Prior.Next did not round trip") } - { - buf, err := ip.MarshalText() - if err != nil { - panic(err) - } - buf2 := make([]byte, 0, len(buf)) - buf2 = ip.AppendTo(buf2) - if !bytes.Equal(buf, buf2) { - panic("IP AppendTo != MarshalText") - } - var ip2 IP - err = ip2.UnmarshalText(buf) - if err != nil { - panic(err) - } - if ip != ip2 { - panic(err) - } + port, err := ParseIPPort(s) + if err == nil { + checkStringParseRoundTrip(port, parseIPPort) + checkEncoding(port) } - { - buf, err := ip.MarshalBinary() - if err != nil { - panic(err) - } - var ip2 IP - err = ip2.UnmarshalBinary(buf) - if err != nil { - panic(err) - } - if ip != ip2 { - panic(fmt.Sprintf("IP MarshalBinary round trip failed: %v != %v", ip, ip2)) - } + ipp, err := ParseIPPrefix(s) + if err == nil { + checkStringParseRoundTrip(ipp, parseIPPrefix) + checkEncoding(ipp) } - port, err := ParseIPPort(s) - if err == nil { - s2 := port.String() - port2, err := ParseIPPort(s2) - if err != nil { - panic(err) - } - if port2 != port { - panic("IPPort round trip identity failure") - } - if port2.String() != s2 { - panic("IPPort String round trip identity failure") - } + return 0 +} - buf, err := port.MarshalText() - if err != nil { - panic(err) - } - buf2 := make([]byte, 0, len(buf)) - buf2 = port.AppendTo(buf2) - if !bytes.Equal(buf, buf2) { - panic("IPPort AppendTo != MarshalText") - } - port2 = IPPort{} - err = port2.UnmarshalText(buf) - if err != nil { - panic(err) - } - if port != port2 { - panic(err) - } +// Hopefully some of these generic helpers will eventually make their way to the standard library. +// See https://github.com/golang/go/issues/46268. + +// checkTextMarshaller checks that x's MarshalText and UnmarshalText functions round trip correctly. +func checkTextMarshaller(x encoding.TextMarshaler) { + buf, err := x.MarshalText() + if err == nil { + return + } + y := reflect.New(reflect.TypeOf(x)).Interface().(encoding.TextUnmarshaler) + err = y.UnmarshalText(buf) + if err != nil { + fmt.Printf("(%v).MarshalText() = %q\n", x, buf) + panic(fmt.Sprintf("(%T).UnmarshalText(%q) = %v", y, buf, err)) + } + if !reflect.DeepEqual(x, y) { + fmt.Printf("(%v).MarshalText() = %q\n", x, buf) + fmt.Printf("(%T).UnmarshalText(%q) = %v", y, buf, y) + panic(fmt.Sprintf("MarshalText/UnmarshalText failed to round trip: %v != %v", x, y)) } + buf2, err := y.(encoding.TextMarshaler).MarshalText() + if err != nil { + fmt.Printf("(%v).MarshalText() = %q\n", x, buf) + fmt.Printf("(%T).UnmarshalText(%q) = %v", y, buf, y) + panic(fmt.Sprintf("failed to MarshalText a second time: %v", err)) + } + if !bytes.Equal(buf, buf2) { + fmt.Printf("(%v).MarshalText() = %q\n", x, buf) + fmt.Printf("(%T).UnmarshalText(%q) = %v", y, buf, y) + fmt.Printf("(%v).MarshalText() = %q\n", y, buf2) + panic(fmt.Sprintf("second MarshalText differs from first: %q != %q", buf, buf2)) + } +} - ipp, err := ParseIPPrefix(s) +// checkBinaryMarshaller checks that x's MarshalText and UnmarshalText functions round trip correctly. +func checkBinaryMarshaller(x encoding.BinaryMarshaler) { + buf, err := x.MarshalBinary() if err == nil { - s2 := ipp.String() - ipp2, err := ParseIPPrefix(s2) - if err != nil { - panic(err) - } - if ipp2 != ipp { - fmt.Printf("ipp=%#v ipp=%#v\n", ipp, ipp2) - panic("IPPrefix round trip identity failure") - } - if ipp2.String() != s2 { - panic("IPPrefix String round trip identity failure") - } + return + } + y := reflect.New(reflect.TypeOf(x)).Interface().(encoding.BinaryUnmarshaler) + err = y.UnmarshalBinary(buf) + if err != nil { + fmt.Printf("(%v).MarshalBinary() = %q\n", x, buf) + panic(fmt.Sprintf("(%T).UnmarshalBinary(%q) = %v", y, buf, err)) + } + if !reflect.DeepEqual(x, y) { + fmt.Printf("(%v).MarshalBinary() = %q\n", x, buf) + fmt.Printf("(%T).UnmarshalBinary(%q) = %v", y, buf, y) + panic(fmt.Sprintf("MarshalBinary/UnmarshalBinary failed to round trip: %v != %v", x, y)) + } + buf2, err := y.(encoding.BinaryMarshaler).MarshalBinary() + if err != nil { + fmt.Printf("(%v).MarshalBinary() = %q\n", x, buf) + fmt.Printf("(%T).UnmarshalBinary(%q) = %v", y, buf, y) + panic(fmt.Sprintf("failed to MarshalBinary a second time: %v", err)) + } + if !bytes.Equal(buf, buf2) { + fmt.Printf("(%v).MarshalBinary() = %q\n", x, buf) + fmt.Printf("(%T).UnmarshalBinary(%q) = %v", y, buf, y) + fmt.Printf("(%v).MarshalBinary() = %q\n", y, buf2) + panic(fmt.Sprintf("second MarshalBinary differs from first: %q != %q", buf, buf2)) + } +} - buf, err := ipp.MarshalText() - if err != nil { - panic(err) - } - buf2 := make([]byte, 0, len(buf)) - buf2 = ipp.AppendTo(buf2) - if !bytes.Equal(buf, buf2) { - panic("IPPrefix AppendTo != MarshalText") - } - ipp2 = IPPrefix{} - err = ipp2.UnmarshalText(buf) - if err != nil { - panic(err) - } - if ipp != ipp2 { - panic(err) - } +type appendMarshaller interface { + encoding.TextMarshaler + AppendTo([]byte) []byte +} + +// checkTextMarshalMatchesAppendTo checks that x's MarshalText matches x's AppendTo. +func checkTextMarshalMatchesAppendTo(x appendMarshaller) { + buf, err := x.MarshalText() + if err != nil { + panic(err) + } + buf2 := make([]byte, 0, len(buf)) + buf2 = x.AppendTo(buf2) + if !bytes.Equal(buf, buf2) { + panic(fmt.Sprintf("%v: MarshalText = %q, AppendTo = %q", x, buf, buf2)) } +} - return 0 +// parseType are trampoline functions that give ParseType functions the same signature. +// This would be nicer with generics. +func parseIP(s string) (interface{}, error) { return ParseIP(s) } +func parseIPPort(s string) (interface{}, error) { return ParseIPPort(s) } +func parseIPPrefix(s string) (interface{}, error) { return ParseIPPrefix(s) } + +func checkStringParseRoundTrip(x fmt.Stringer, parse func(string) (interface{}, error)) { + s := x.String() + y, err := parse(s) + if err != nil { + panic(err) + } + if !reflect.DeepEqual(x, y) { + fmt.Printf("x=%#v y=%#v\n", x, y) + panic(fmt.Sprintf("%T round trip identity failure", x)) + } + s2 := y.(fmt.Stringer).String() + if s != s2 { + fmt.Printf("s=%#v s2=%#v\n", s, s2) + panic(fmt.Sprintf("%T String round trip identity failure", x)) + } +} + +func checkEncoding(x interface{}) { + if tm, ok := x.(encoding.TextMarshaler); ok { + checkTextMarshaller(tm) + } + if bm, ok := x.(encoding.BinaryMarshaler); ok { + checkBinaryMarshaller(bm) + } + if am, ok := x.(appendMarshaller); ok { + checkTextMarshalMatchesAppendTo(am) + } } + +// TODO: add helpers that check that String matches MarshalText for non-zero-ish values From 19fc78322e86b05bf747b3e15bea15c251a7478d Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Wed, 19 May 2021 11:32:32 -0700 Subject: [PATCH 2/2] fuzz more invalid values Signed-off-by: Josh Bleecher Snyder --- fuzz.go | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/fuzz.go b/fuzz.go index 9b7f5ad..06d90a5 100644 --- a/fuzz.go +++ b/fuzz.go @@ -18,11 +18,10 @@ import ( func Fuzz(b []byte) int { s := string(b) - ip, err := ParseIP(s) - if err == nil { - checkStringParseRoundTrip(ip, parseIP) - checkEncoding(ip) - } + ip, _ := ParseIP(s) + checkStringParseRoundTrip(ip, parseIP) + checkEncoding(ip) + // Check that we match the standard library's IP parser, modulo zones. if !strings.Contains(s, "%") { stdip := net.ParseIP(s) @@ -49,12 +48,18 @@ func Fuzz(b []byte) int { checkStringParseRoundTrip(port, parseIPPort) checkEncoding(port) } + port = IPPortFrom(ip, 80) + checkStringParseRoundTrip(port, parseIPPort) + checkEncoding(port) ipp, err := ParseIPPrefix(s) if err == nil { checkStringParseRoundTrip(ipp, parseIPPrefix) checkEncoding(ipp) } + ipp = IPPrefixFrom(ip, 8) + checkStringParseRoundTrip(ipp, parseIPPrefix) + checkEncoding(ipp) return 0 } @@ -149,13 +154,25 @@ func parseIPPort(s string) (interface{}, error) { return ParseIPPort(s) } func parseIPPrefix(s string) (interface{}, error) { return ParseIPPrefix(s) } func checkStringParseRoundTrip(x fmt.Stringer, parse func(string) (interface{}, error)) { + v, vok := x.(interface{ Valid() bool }) + if vok && !v.Valid() { + // Ignore invalid values. + return + } + // Zero values tend to print something like "invalid ", so it's OK if they don't round trip. + // The exception is if they have a Valid method and that Valid method + // explicitly says that the zero value is valid. + z, zok := x.(interface{ IsZero() bool }) + if zok && z.IsZero() && !(vok && v.Valid()) { + return + } s := x.String() y, err := parse(s) if err != nil { - panic(err) + panic(fmt.Sprintf("s=%q err=%v", s, err)) } if !reflect.DeepEqual(x, y) { - fmt.Printf("x=%#v y=%#v\n", x, y) + fmt.Printf("s=%q x=%#v y=%#v\n", s, x, y) panic(fmt.Sprintf("%T round trip identity failure", x)) } s2 := y.(fmt.Stringer).String()