dhcpv6: switch every Option to parse using FromBytes

dhcpv6/dhcpv6message.go

@@ -169,8 +169,8 @@ func (mo MessageOptions) BootFileURL() string {
 	if opt == nil {
 		return ""
 	}
-	if u, ok := opt.(optBootFileURL); ok {
-		return string(u)
+	if u, ok := opt.(*optBootFileURL); ok {
+		return u.url
 	}
 	return ""
 }
@@ -181,8 +181,8 @@ func (mo MessageOptions) BootFileParam() []string {
 	if opt == nil {
 		return nil
 	}
-	if u, ok := opt.(optBootFileParam); ok {
-		return []string(u)
+	if u, ok := opt.(*optBootFileParam); ok {
+		return u.params
 	}
 	return nil
 }

dhcpv6/option_4rd.go

@@ -32,12 +32,10 @@ func (op *Opt4RD) LongString(indentSpace int) string {
 	return fmt.Sprintf("%s: Options=%v", op.Code(), op.Options.LongString(indentSpace))
 }
 
-// ParseOpt4RD builds an Opt4RD structure from a sequence of bytes.
+// FromBytes builds an Opt4RD structure from a sequence of bytes.
 // The input data does not include option code and length bytes
-func ParseOpt4RD(data []byte) (*Opt4RD, error) {
-	var opt Opt4RD
-	err := opt.Options.FromBytes(data)
-	return &opt, err
+func (op *Opt4RD) FromBytes(data []byte) error {
+	return op.Options.FromBytes(data)
 }
 
 // Opt4RDMapRule represents a 4RD Mapping Rule option
@@ -105,18 +103,17 @@ func (op *Opt4RDMapRule) String() string {
 		op.Code(), op.Prefix4.String(), op.Prefix6.String(), op.EABitsLength, op.WKPAuthorized)
 }
 
-// ParseOpt4RDMapRule builds an Opt4RDMapRule structure from a sequence of bytes.
+// FromBytes builds an Opt4RDMapRule structure from a sequence of bytes.
 // The input data does not include option code and length bytes.
-func ParseOpt4RDMapRule(data []byte) (*Opt4RDMapRule, error) {
-	var opt Opt4RDMapRule
+func (op *Opt4RDMapRule) FromBytes(data []byte) error {
 	buf := uio.NewBigEndianBuffer(data)
-	opt.Prefix4.Mask = net.CIDRMask(int(buf.Read8()), 32)
-	opt.Prefix6.Mask = net.CIDRMask(int(buf.Read8()), 128)
-	opt.EABitsLength = buf.Read8()
-	opt.WKPAuthorized = (buf.Read8() & opt4RDWKPAuthorizedMask) != 0
-	opt.Prefix4.IP = net.IP(buf.CopyN(net.IPv4len))
-	opt.Prefix6.IP = net.IP(buf.CopyN(net.IPv6len))
-	return &opt, buf.FinError()
+	op.Prefix4.Mask = net.CIDRMask(int(buf.Read8()), 32)
+	op.Prefix6.Mask = net.CIDRMask(int(buf.Read8()), 128)
+	op.EABitsLength = buf.Read8()
+	op.WKPAuthorized = (buf.Read8() & opt4RDWKPAuthorizedMask) != 0
+	op.Prefix4.IP = net.IP(buf.CopyN(net.IPv4len))
+	op.Prefix6.IP = net.IP(buf.CopyN(net.IPv6len))
+	return buf.FinError()
 }
 
 // Opt4RDNonMapRule represents 4RD parameters other than mapping rules
@@ -165,21 +162,20 @@ func (op *Opt4RDNonMapRule) String() string {
 		op.Code(), op.HubAndSpoke, tClass, op.DomainPMTU)
 }
 
-// ParseOpt4RDNonMapRule builds an Opt4RDNonMapRule structure from a sequence of bytes.
+// FromBytes builds an Opt4RDNonMapRule structure from a sequence of bytes.
 // The input data does not include option code and length bytes
-func ParseOpt4RDNonMapRule(data []byte) (*Opt4RDNonMapRule, error) {
-	var opt Opt4RDNonMapRule
+func (op *Opt4RDNonMapRule) FromBytes(data []byte) error {
 	buf := uio.NewBigEndianBuffer(data)
 	flags := buf.Read8()
 
-	opt.HubAndSpoke = flags&opt4RDHubAndSpokeMask != 0
+	op.HubAndSpoke = flags&opt4RDHubAndSpokeMask != 0
 
 	tClass := buf.Read8()
 	if flags&opt4RDTrafficClassMask != 0 {
-		opt.TrafficClass = &tClass
+		op.TrafficClass = &tClass
 	}
 
-	opt.DomainPMTU = buf.Read16()
+	op.DomainPMTU = buf.Read16()
 
-	return &opt, buf.FinError()
+	return buf.FinError()
 }

dhcpv6/option_4rd_test.go

@@ -2,14 +2,16 @@ package dhcpv6
 
 import (
 	"net"
+	"reflect"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 )
 
 func TestOpt4RDNonMapRuleParse(t *testing.T) {
 	data := []byte{0x81, 0xaa, 0x05, 0xd4}
-	opt, err := ParseOpt4RDNonMapRule(data)
+	var opt Opt4RDNonMapRule
+	err := opt.FromBytes(data)
 	require.NoError(t, err)
 	require.True(t, opt.HubAndSpoke)
 	require.NotNil(t, opt.TrafficClass)
@@ -18,7 +20,8 @@ func TestOpt4RDNonMapRuleParse(t *testing.T) {
 
 	// Remove the TrafficClass flag and check value is ignored
 	data[0] = 0x80
-	opt, err = ParseOpt4RDNonMapRule(data)
+	opt = Opt4RDNonMapRule{}
+	err = opt.FromBytes(data)
 	require.NoError(t, err)
 	require.True(t, opt.HubAndSpoke)
 	require.Nil(t, opt.TrafficClass)
@@ -77,7 +80,8 @@ func TestOpt4RDMapRuleParse(t *testing.T) {
 		append(ip4addr.To4(), ip6addr...)...,
 	)
 
-	opt, err := ParseOpt4RDMapRule(data)
+	var opt Opt4RDMapRule
+	err = opt.FromBytes(data)
 	require.NoError(t, err)
 	require.EqualValues(t, *ip6net, opt.Prefix6)
 	require.EqualValues(t, *ip4net, opt.Prefix4)
@@ -162,9 +166,20 @@ func TestOpt4RDRoundTrip(t *testing.T) {
 		},
 	}
 
-	rtOpt, err := ParseOpt4RD(opt.ToBytes())
+	var rtOpt Opt4RD
+	err := rtOpt.FromBytes(opt.ToBytes())
 
 	require.NoError(t, err)
 	require.NotNil(t, rtOpt)
-	require.Equal(t, opt, *rtOpt)
+	require.Equal(t, opt, rtOpt)
+
+	var mo MessageOptions
+	mo.Options.Add(&opt)
+
+	var got MessageOptions
+	if err := got.FromBytes(mo.ToBytes()); err != nil {
+		t.Errorf("FromBytes = %v", err)
+	} else if !reflect.DeepEqual(mo, got) {
+		t.Errorf("FromBytes = %v, want %v", got, mo)
+	}
 }

dhcpv6/option_archtype.go

@@ -26,10 +26,6 @@ func (op optClientArchType) String() string {
 	return fmt.Sprintf("%s: %s", op.Code(), op.Archs)
 }
 
-// parseOptClientArchType builds an OptClientArchType structure from
-// a sequence of bytes The input data does not include option code and
-// length bytes.
-func parseOptClientArchType(data []byte) (*optClientArchType, error) {
-	var opt optClientArchType
-	return &opt, opt.FromBytes(data)
+func (op *optClientArchType) FromBytes(p []byte) error {
+	return op.Archs.FromBytes(p)
 }

dhcpv6/option_archtype_test.go

@@ -11,22 +11,25 @@ func TestParseOptClientArchType(t *testing.T) {
 	data := []byte{
 		0, 6, // EFI_IA32
 	}
-	opt, err := parseOptClientArchType(data)
+	var opt optClientArchType
+	err := opt.FromBytes(data)
 	require.NoError(t, err)
 	require.Equal(t, iana.EFI_IA32, opt.Archs[0])
 }
 
 func TestParseOptClientArchTypeInvalid(t *testing.T) {
 	data := []byte{42}
-	_, err := parseOptClientArchType(data)
+	var opt optClientArchType
+	err := opt.FromBytes(data)
 	require.Error(t, err)
 }
 
 func TestOptClientArchTypeParseAndToBytes(t *testing.T) {
 	data := []byte{
 		0, 8, // EFI_XSCALE
 	}
-	opt, err := parseOptClientArchType(data)
+	var opt optClientArchType
+	err := opt.FromBytes(data)
 	require.NoError(t, err)
 	require.Equal(t, data, opt.ToBytes())
 }

dhcpv6/option_bootfileparam.go

@@ -9,10 +9,12 @@ import (
 // OptBootFileParam returns a BootfileParam option as defined in RFC 5970
 // Section 3.2.
 func OptBootFileParam(args ...string) Option {
-	return optBootFileParam(args)
+	return &optBootFileParam{args}
 }
 
-type optBootFileParam []string
+type optBootFileParam struct {
+	params []string
+}
 
 // Code returns the option code
 func (optBootFileParam) Code() OptionCode {
@@ -22,7 +24,7 @@ func (optBootFileParam) Code() OptionCode {
 // ToBytes serializes the option and returns it as a sequence of bytes
 func (op optBootFileParam) ToBytes() []byte {
 	buf := uio.NewBigEndianBuffer(nil)
-	for _, param := range op {
+	for _, param := range op.params {
 		if len(param) >= 1<<16 {
 			// TODO: say something here instead of silently ignoring a parameter
 			continue
@@ -42,20 +44,16 @@ func (op optBootFileParam) ToBytes() []byte {
 }
 
 func (op optBootFileParam) String() string {
-	return fmt.Sprintf("%s: %v", op.Code(), ([]string)(op))
+	return fmt.Sprintf("%s: %v", op.Code(), op.params)
 }
 
-// parseOptBootFileParam builds an OptBootFileParam structure from a sequence
+// FromBytes builds an OptBootFileParam structure from a sequence
 // of bytes. The input data does not include option code and length bytes.
-func parseOptBootFileParam(data []byte) (optBootFileParam, error) {
+func (op *optBootFileParam) FromBytes(data []byte) error {
 	buf := uio.NewBigEndianBuffer(data)
-	var result optBootFileParam
 	for buf.Has(2) {
 		length := buf.Read16()
-		result = append(result, string(buf.CopyN(int(length))))
-	}
-	if err := buf.FinError(); err != nil {
-		return nil, err
+		op.params = append(op.params, string(buf.CopyN(int(length))))
 	}
-	return result, nil
+	return buf.FinError()
 }

dhcpv6/option_bootfileparam_test.go

@@ -41,8 +41,8 @@ func compileTestBootfileParams(t *testing.T, params []string) []byte {
 
 func TestOptBootFileParam(t *testing.T) {
 	expected := string(compileTestBootfileParams(t, testBootfileParams1))
-	opt, err := parseOptBootFileParam([]byte(expected))
-	if err != nil {
+	var opt optBootFileParam
+	if err := opt.FromBytes([]byte(expected)); err != nil {
 		t.Fatal(err)
 	}
 	if string(opt.ToBytes()) != expected {
@@ -54,10 +54,10 @@ func TestParsedTypeOptBootFileParam(t *testing.T) {
 	tryParse := func(compiled []byte, expected []string) {
 		opt, err := ParseOption(OptionBootfileParam, compiled)
 		require.NoError(t, err)
-		bootfileParamOpt, ok := opt.(optBootFileParam)
+		bootfileParamOpt, ok := opt.(*optBootFileParam)
 		require.True(t, ok, fmt.Sprintf("invalid type: %T instead of %T", opt, bootfileParamOpt))
 		require.Equal(t, compiled, bootfileParamOpt.ToBytes())
-		require.Equal(t, expected, ([]string)(bootfileParamOpt))
+		require.Equal(t, expected, bootfileParamOpt.params)
 	}
 
 	tryParse(

dhcpv6/option_bootfileurl.go

@@ -6,10 +6,12 @@ import (
 
 // OptBootFileURL returns a OptionBootfileURL as defined by RFC 5970.
 func OptBootFileURL(url string) Option {
-	return optBootFileURL(url)
+	return &optBootFileURL{url}
 }
 
-type optBootFileURL string
+type optBootFileURL struct {
+	url string
+}
 
 // Code returns the option code
 func (op optBootFileURL) Code() OptionCode {
@@ -18,15 +20,16 @@ func (op optBootFileURL) Code() OptionCode {
 
 // ToBytes serializes the option and returns it as a sequence of bytes
 func (op optBootFileURL) ToBytes() []byte {
-	return []byte(op)
+	return []byte(op.url)
 }
 
 func (op optBootFileURL) String() string {
-	return fmt.Sprintf("%s: %s", op.Code(), string(op))
+	return fmt.Sprintf("%s: %s", op.Code(), op.url)
 }
 
-// parseOptBootFileURL builds an optBootFileURL structure from a sequence
+// FromBytes builds an optBootFileURL structure from a sequence
 // of bytes. The input data does not include option code and length bytes.
-func parseOptBootFileURL(data []byte) (optBootFileURL, error) {
-	return optBootFileURL(string(data)), nil
+func (op *optBootFileURL) FromBytes(data []byte) error {
+	op.url = string(data)
+	return nil
 }

dhcpv6/option_bootfileurl_test.go

@@ -9,11 +9,11 @@ import (
 
 func TestOptBootFileURL(t *testing.T) {
 	expected := "https://insomniac.slackware.it"
-	opt, err := parseOptBootFileURL([]byte(expected))
-	if err != nil {
+	var opt optBootFileURL
+	if err := opt.FromBytes([]byte(expected)); err != nil {
 		t.Fatal(err)
 	}
-	if string(opt) != expected {
+	if opt.url != expected {
 		t.Fatalf("Invalid boot file URL. Expected %v, got %v", expected, opt)
 	}
 	require.Contains(t, opt.String(), "https://insomniac.slackware.it", "String() should contain the correct BootFileUrl output")

dhcpv6/option_clientid.go

@@ -22,13 +22,11 @@ func (op *optClientID) String() string {
 	return fmt.Sprintf("%s: %s", op.Code(), op.DUID)
 }
 
-// parseOptClientID builds an OptClientId structure from a sequence
+// FromBytes builds an optClientID structure from a sequence
 // of bytes. The input data does not include option code and length
 // bytes.
-func parseOptClientID(data []byte) (*optClientID, error) {
-	cid, err := DUIDFromBytes(data)
-	if err != nil {
-		return nil, err
-	}
-	return &optClientID{cid}, nil
+func (op *optClientID) FromBytes(data []byte) error {
+	var err error
+	op.DUID, err = DUIDFromBytes(data)
+	return err
 }