From 8b655f3de91be943e73fc1efce02e18dac089e59 Mon Sep 17 00:00:00 2001 From: Kurits Nusbaum Date: Sat, 21 Sep 2019 15:06:32 -0700 Subject: [PATCH 1/5] net: add text marshalling and unmarshalling for HardwareAddr The HardwareAddr type has the ability to be transformed to and from a string. However, this capability is not exposed in a manner suitable for use with various forms of marshalling and unmarshaling of text (e.g. JSON or YAML). Let's add the proper functions so that HardwareAddr implements the TextMarshaller and TextUnmarshaler interfaces from the encoding package. Fixes #29678 --- src/net/mac.go | 20 +++++++++++++++ src/net/mac_test.go | 62 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/src/net/mac.go b/src/net/mac.go index 373ac3d7e2018e..1cf2bf85b7107a 100644 --- a/src/net/mac.go +++ b/src/net/mac.go @@ -24,6 +24,26 @@ func (a HardwareAddr) String() string { return string(buf) } +// MarshalText implements encoding.TextMarshaler using the +// standard string representation of a HardwareAddr. +func (a HardwareAddr) MarshalText() ([]byte, error) { + return []byte(a.String()), nil +} + +// UnmarshalText implements encoding.TextUnmarshaler. +func (a *HardwareAddr) UnmarshalText(text []byte) error { + if len(text) == 0 { + *a = nil + return nil + } + v, err := ParseMAC(string(text)) + if err != nil { + return err + } + *a = v + return nil +} + // ParseMAC parses s as an IEEE 802 MAC-48, EUI-48, EUI-64, or a 20-octet // IP over InfiniBand link-layer address using one of the following formats: // 00:00:5e:00:53:01 diff --git a/src/net/mac_test.go b/src/net/mac_test.go index cad884fcf5d42d..30d8d033c46837 100644 --- a/src/net/mac_test.go +++ b/src/net/mac_test.go @@ -107,3 +107,65 @@ func TestParseMAC(t *testing.T) { } } } + +func TestHardwareAddr_UnmarshalText(t *testing.T) { + tests := []struct { + msg string + text string + wantStr string + wantErr string + }{ + { + msg: "valid mac", + text: "aa:bb:cc:dd:ee:ff", + wantStr: "aa:bb:cc:dd:ee:ff", + wantErr: "", + }, { + msg: "empty text", + text: "", + wantStr: "", + wantErr: "", + }, { + msg: "invalid text", + text: "foo-bar-baz", + wantStr: "", + wantErr: "address foo-bar-baz: invalid MAC address", + }, + } + for _, tt := range tests { + t.Run(tt.msg, func(t *testing.T) { + var a HardwareAddr + err := a.UnmarshalText([]byte(tt.text)) + if tt.wantErr != "" && err.Error() != tt.wantErr { + t.Errorf("Wanted err %q got %q", tt.wantErr, err.Error()) + } else if tt.wantErr == "" && err != nil { + t.Errorf("Unexpected error %q", err) + } + + if tt.wantStr != a.String() { + t.Errorf("Wanted address string %q but got %q", tt.wantStr, a.String()) + } + }) + } +} + +func TestHardwareAddr_MarshalText(t *testing.T) { + input := "aa:bb:cc:dd:ee:ff" + + var a HardwareAddr + if err := a.UnmarshalText([]byte(input)); err != nil { + t.Fatalf("Unexpected error while unmarashaling: %q", err) + } + + output, err := a.MarshalText() + if err != nil { + t.Fatalf("Error marshaling: %s", err) + } + + if err := a.UnmarshalText([]byte(input)); err != nil { + t.Fatalf("Unexpected error while marshaling: %q", err) + } + if input != string(output) { + t.Errorf("Input/Output mismatch: %q and %q", input, string(output)) + } +} From 084018df077b3bcca664a4431eaac2479a471121 Mon Sep 17 00:00:00 2001 From: Kurits Nusbaum Date: Mon, 23 Sep 2019 20:06:58 -0700 Subject: [PATCH 2/5] add ability to decode strings repsenting a byte slice for backwards compatibility reasons --- src/net/mac.go | 11 +++++++++++ src/net/mac_test.go | 17 +++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/net/mac.go b/src/net/mac.go index 1cf2bf85b7107a..01844a1168cc02 100644 --- a/src/net/mac.go +++ b/src/net/mac.go @@ -4,6 +4,8 @@ package net +import "encoding/base64" + const hexDigit = "0123456789abcdef" // A HardwareAddr represents a physical hardware address. @@ -36,6 +38,15 @@ func (a *HardwareAddr) UnmarshalText(text []byte) error { *a = nil return nil } + + // first attempt to decode raw bytes in case the mac address + // was encoded as a raw byte slice in an older verison of golang. + b64buf := make([]byte, 6) + if _, err := base64.StdEncoding.Decode(b64buf, text); err == nil { + *a = b64buf + return nil + } + v, err := ParseMAC(string(text)) if err != nil { return err diff --git a/src/net/mac_test.go b/src/net/mac_test.go index 30d8d033c46837..687d61bb466489 100644 --- a/src/net/mac_test.go +++ b/src/net/mac_test.go @@ -116,11 +116,24 @@ func TestHardwareAddr_UnmarshalText(t *testing.T) { wantErr string }{ { - msg: "valid mac", + msg: "valid mac1", text: "aa:bb:cc:dd:ee:ff", wantStr: "aa:bb:cc:dd:ee:ff", wantErr: "", - }, { + }, + { + msg: "valid mac2", + text: "00-00-5e-00-63-01", + wantStr: "00:00:5e:00:63:01", + wantErr: "", + }, + { + msg: "binary mac", + text: "VCKpL053", + wantStr: "54:22:a9:2f:4e:77", + wantErr: "", + }, + { msg: "empty text", text: "", wantStr: "", From df49700d7134cef359240e83f7c7c25cc9d7d58e Mon Sep 17 00:00:00 2001 From: Kurtis Nusbaum Date: Mon, 30 Sep 2019 06:51:26 -0700 Subject: [PATCH 3/5] - Add some more docstring to `UnmarshalText` - Create `matchErr` function and simplify result testing in `TestHardwareAddr_UnmarshalText` - Fix Lbrace in `TestHardwareAddr_UnmarshalText` - Change `Fatalf`s to just `Fatal`s in `TestHardwareAddr_MarshalText` --- src/net/mac.go | 3 ++- src/net/mac_test.go | 30 +++++++++++++++++------------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/net/mac.go b/src/net/mac.go index 01844a1168cc02..e817f965359fad 100644 --- a/src/net/mac.go +++ b/src/net/mac.go @@ -39,7 +39,7 @@ func (a *HardwareAddr) UnmarshalText(text []byte) error { return nil } - // first attempt to decode raw bytes in case the mac address + // First, attempt to decode raw bytes in case the mac address // was encoded as a raw byte slice in an older verison of golang. b64buf := make([]byte, 6) if _, err := base64.StdEncoding.Decode(b64buf, text); err == nil { @@ -47,6 +47,7 @@ func (a *HardwareAddr) UnmarshalText(text []byte) error { return nil } + // Otherwise, fallback to normal MAC address parsing. v, err := ParseMAC(string(text)) if err != nil { return err diff --git a/src/net/mac_test.go b/src/net/mac_test.go index 687d61bb466489..0eb0f2c9b57e87 100644 --- a/src/net/mac_test.go +++ b/src/net/mac_test.go @@ -138,7 +138,8 @@ func TestHardwareAddr_UnmarshalText(t *testing.T) { text: "", wantStr: "", wantErr: "", - }, { + }, + { msg: "invalid text", text: "foo-bar-baz", wantStr: "", @@ -149,36 +150,39 @@ func TestHardwareAddr_UnmarshalText(t *testing.T) { t.Run(tt.msg, func(t *testing.T) { var a HardwareAddr err := a.UnmarshalText([]byte(tt.text)) - if tt.wantErr != "" && err.Error() != tt.wantErr { - t.Errorf("Wanted err %q got %q", tt.wantErr, err.Error()) - } else if tt.wantErr == "" && err != nil { - t.Errorf("Unexpected error %q", err) - } - - if tt.wantStr != a.String() { - t.Errorf("Wanted address string %q but got %q", tt.wantStr, a.String()) + gotStr := a.String() + if tt.wantStr != gotStr || !matchErr(tt.wantErr, err) { + t.Errorf("want: addr = %q, err = %q, got: addr = %q, err = %q", tt.wantStr, tt.wantErr, gotStr, err) } }) } } +func matchErr(s string, err error) bool { + if s == "" { + return err == nil + } + return err != nil && strings.Contains(err.Error(), s) +} + func TestHardwareAddr_MarshalText(t *testing.T) { input := "aa:bb:cc:dd:ee:ff" var a HardwareAddr if err := a.UnmarshalText([]byte(input)); err != nil { - t.Fatalf("Unexpected error while unmarashaling: %q", err) + t.Fatal(err) } output, err := a.MarshalText() if err != nil { - t.Fatalf("Error marshaling: %s", err) + t.Fatal(err) } if err := a.UnmarshalText([]byte(input)); err != nil { - t.Fatalf("Unexpected error while marshaling: %q", err) + t.Fatal(err) } + if input != string(output) { - t.Errorf("Input/Output mismatch: %q and %q", input, string(output)) + t.Errorf("want %q, got %q", input, string(output)) } } From 9e0759203190dce7b778b4ddf1b07117314cd7d5 Mon Sep 17 00:00:00 2001 From: Kurtis Nusbaum Date: Mon, 30 Sep 2019 07:01:27 -0700 Subject: [PATCH 4/5] Standardize casing of MAC in doc strings --- src/net/mac.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net/mac.go b/src/net/mac.go index e817f965359fad..69d15a69b1440e 100644 --- a/src/net/mac.go +++ b/src/net/mac.go @@ -39,7 +39,7 @@ func (a *HardwareAddr) UnmarshalText(text []byte) error { return nil } - // First, attempt to decode raw bytes in case the mac address + // First, attempt to decode raw bytes in case the MAC address // was encoded as a raw byte slice in an older verison of golang. b64buf := make([]byte, 6) if _, err := base64.StdEncoding.Decode(b64buf, text); err == nil { From 566064296335016a8cbeb8c7aac4d239ce3b7476 Mon Sep 17 00:00:00 2001 From: Kurtis Nusbaum Date: Wed, 2 Oct 2019 11:47:03 -0700 Subject: [PATCH 5/5] move matchErr to be a closure within the test --- src/net/mac_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/net/mac_test.go b/src/net/mac_test.go index 0eb0f2c9b57e87..0b709a4ec36c1b 100644 --- a/src/net/mac_test.go +++ b/src/net/mac_test.go @@ -109,6 +109,13 @@ func TestParseMAC(t *testing.T) { } func TestHardwareAddr_UnmarshalText(t *testing.T) { + matchErr := func(s string, err error) bool { + if s == "" { + return err == nil + } + return err != nil && strings.Contains(err.Error(), s) + } + tests := []struct { msg string text string @@ -158,13 +165,6 @@ func TestHardwareAddr_UnmarshalText(t *testing.T) { } } -func matchErr(s string, err error) bool { - if s == "" { - return err == nil - } - return err != nil && strings.Contains(err.Error(), s) -} - func TestHardwareAddr_MarshalText(t *testing.T) { input := "aa:bb:cc:dd:ee:ff"