dhcpv4: conform to RFC 2131 with respect to options.

Removes AddOption and GetOption.

RFC 2131 specifies that options may only appear once (Section 4.1). If
an option does appear more than once, its byte values must be
concatenated.

RFC 3396 further specifies that to send options longer than 255 bytes,
one option may be split into multiple option codes, which must be
concatenated back together by the receiver.

Both of these are concerned with the byte representation of options.
Fact is, based on both RFCs one can say that an option may only appear
once, but may be composed of multiple values.

Because an option may appear only once logically in any case, we remove
the AddOption and GetOption functions and leave only UpdateOption and
GetOneOption.

Also remove all additions & checks of the End option - the marshaling
and unmarshaling code is exclusively responsible for that now.

11 files changed, 115 insertions, 230 deletions

diff --git a/dhcpv4/bsdp/bsdp.go b/dhcpv4/bsdp/bsdp.go
index 43adcc9..21cc71a 100644
--- a/dhcpv4/bsdp/bsdp.go
+++ b/dhcpv4/bsdp/bsdp.go
@@ -31,7 +31,6 @@ type ReplyConfig struct {
 // ParseBootImageListFromAck parses the list of boot images presented in the
 // ACK[LIST] packet and returns them as a list of BootImages.
 func ParseBootImageListFromAck(ack dhcpv4.DHCPv4) ([]BootImage, error) {
-	var images []BootImage
 	opt := ack.GetOneOption(dhcpv4.OptionVendorSpecificInformation)
 	if opt == nil {
 		return nil, errors.New("ParseBootImageListFromAck: could not find vendor-specific option")
@@ -40,11 +39,11 @@ func ParseBootImageListFromAck(ack dhcpv4.DHCPv4) ([]BootImage, error) {
 	if err != nil {
 		return nil, err
 	}
-	bootImageOpts := vendorOpt.GetOption(OptionBootImageList)
-	for _, opt := range bootImageOpts {
-		images = append(images, opt.(*OptBootImageList).Images...)
+	bootImageOpts := vendorOpt.GetOneOption(OptionBootImageList)
+	if bootImageOpts == nil {
+		return nil, fmt.Errorf("boot image option not found")
 	}
-	return images, nil
+	return bootImageOpts.(*OptBootImageList).Images, nil
 }
 
 func needsReplyPort(replyPort uint16) bool {
@@ -59,12 +58,14 @@ func MessageTypeFromPacket(packet *dhcpv4.DHCPv4) *MessageType {
 		vendorOpts *OptVendorSpecificInformation
 		err        error
 	)
-	for _, opt := range packet.GetOption(dhcpv4.OptionVendorSpecificInformation) {
-		if vendorOpts, err = ParseOptVendorSpecificInformation(opt.ToBytes()); err == nil {
-			if o := vendorOpts.GetOneOption(OptionMessageType); o != nil {
-				if optMessageType, ok := o.(*OptMessageType); ok {
-					return &optMessageType.Type
-				}
+	opt := packet.GetOneOption(dhcpv4.OptionVendorSpecificInformation)
+	if opt == nil {
+		return nil
+	}
+	if vendorOpts, err = ParseOptVendorSpecificInformation(opt.ToBytes()); err == nil {
+		if o := vendorOpts.GetOneOption(OptionMessageType); o != nil {
+			if optMessageType, ok := o.(*OptMessageType); ok {
+				return &optMessageType.Type
 			}
 		}
 	}
@@ -114,21 +115,21 @@ func NewInformList(hwaddr net.HardwareAddr, localIP net.IP, replyPort uint16) (*
 	if needsReplyPort(replyPort) {
 		vendorOpts = append(vendorOpts, &OptReplyPort{replyPort})
 	}
-	d.AddOption(&OptVendorSpecificInformation{vendorOpts})
+	d.UpdateOption(&OptVendorSpecificInformation{vendorOpts})
 
-	d.AddOption(&dhcpv4.OptParameterRequestList{
+	d.UpdateOption(&dhcpv4.OptParameterRequestList{
 		RequestedOpts: []dhcpv4.OptionCode{
 			dhcpv4.OptionVendorSpecificInformation,
 			dhcpv4.OptionClassIdentifier,
 		},
 	})
-	d.AddOption(&dhcpv4.OptMaximumDHCPMessageSize{Size: MaxDHCPMessageSize})
+	d.UpdateOption(&dhcpv4.OptMaximumDHCPMessageSize{Size: MaxDHCPMessageSize})
 
 	vendorClassID, err := MakeVendorClassIdentifier()
 	if err != nil {
 		return nil, err
 	}
-	d.AddOption(&dhcpv4.OptClassIdentifier{Identifier: vendorClassID})
+	d.UpdateOption(&dhcpv4.OptClassIdentifier{Identifier: vendorClassID})
 	return d, nil
 }
 
@@ -179,8 +180,8 @@ func InformSelectForAck(ack dhcpv4.DHCPv4, replyPort uint16, selectedImage BootI
 	if err != nil {
 		return nil, err
 	}
-	d.AddOption(&dhcpv4.OptClassIdentifier{Identifier: vendorClassID})
-	d.AddOption(&dhcpv4.OptParameterRequestList{
+	d.UpdateOption(&dhcpv4.OptClassIdentifier{Identifier: vendorClassID})
+	d.UpdateOption(&dhcpv4.OptParameterRequestList{
 		RequestedOpts: []dhcpv4.OptionCode{
 			dhcpv4.OptionSubnetMask,
 			dhcpv4.OptionRouter,
@@ -189,8 +190,8 @@ func InformSelectForAck(ack dhcpv4.DHCPv4, replyPort uint16, selectedImage BootI
 			dhcpv4.OptionClassIdentifier,
 		},
 	})
-	d.AddOption(&dhcpv4.OptMessageType{MessageType: dhcpv4.MessageTypeInform})
-	d.AddOption(&OptVendorSpecificInformation{vendorOpts})
+	d.UpdateOption(&dhcpv4.OptMessageType{MessageType: dhcpv4.MessageTypeInform})
+	d.UpdateOption(&OptVendorSpecificInformation{vendorOpts})
 	return d, nil
 }
 
@@ -212,9 +213,9 @@ func NewReplyForInformList(inform *dhcpv4.DHCPv4, config ReplyConfig) (*dhcpv4.D
 	reply.ServerIPAddr = config.ServerIP
 	reply.ServerHostName = config.ServerHostname
 
-	reply.AddOption(&dhcpv4.OptMessageType{MessageType: dhcpv4.MessageTypeAck})
-	reply.AddOption(&dhcpv4.OptServerIdentifier{ServerID: config.ServerIP})
-	reply.AddOption(&dhcpv4.OptClassIdentifier{Identifier: AppleVendorID})
+	reply.UpdateOption(&dhcpv4.OptMessageType{MessageType: dhcpv4.MessageTypeAck})
+	reply.UpdateOption(&dhcpv4.OptServerIdentifier{ServerID: config.ServerIP})
+	reply.UpdateOption(&dhcpv4.OptClassIdentifier{Identifier: AppleVendorID})
 
 	// BSDP opts.
 	vendorOpts := []dhcpv4.Option{
@@ -226,9 +227,7 @@ func NewReplyForInformList(inform *dhcpv4.DHCPv4, config ReplyConfig) (*dhcpv4.D
 	if config.SelectedImage != nil {
 		vendorOpts = append(vendorOpts, &OptSelectedBootImageID{ID: config.SelectedImage.ID})
 	}
-	reply.AddOption(&OptVendorSpecificInformation{Options: vendorOpts})
-
-	reply.AddOption(&dhcpv4.OptionGeneric{OptionCode: dhcpv4.OptionEnd})
+	reply.UpdateOption(&OptVendorSpecificInformation{Options: vendorOpts})
 	return reply, nil
 }
 
@@ -252,18 +251,16 @@ func NewReplyForInformSelect(inform *dhcpv4.DHCPv4, config ReplyConfig) (*dhcpv4
 	reply.ServerHostName = config.ServerHostname
 	reply.BootFileName = config.BootFileName
 
-	reply.AddOption(&dhcpv4.OptMessageType{MessageType: dhcpv4.MessageTypeAck})
-	reply.AddOption(&dhcpv4.OptServerIdentifier{ServerID: config.ServerIP})
-	reply.AddOption(&dhcpv4.OptClassIdentifier{Identifier: AppleVendorID})
+	reply.UpdateOption(&dhcpv4.OptMessageType{MessageType: dhcpv4.MessageTypeAck})
+	reply.UpdateOption(&dhcpv4.OptServerIdentifier{ServerID: config.ServerIP})
+	reply.UpdateOption(&dhcpv4.OptClassIdentifier{Identifier: AppleVendorID})
 
 	// BSDP opts.
-	reply.AddOption(&OptVendorSpecificInformation{
+	reply.UpdateOption(&OptVendorSpecificInformation{
 		Options: []dhcpv4.Option{
 			&OptMessageType{Type: MessageTypeSelect},
 			&OptSelectedBootImageID{ID: config.SelectedImage.ID},
 		},
 	})
-
-	reply.AddOption(&dhcpv4.OptionGeneric{OptionCode: dhcpv4.OptionEnd})
 	return reply, nil
 }
diff --git a/dhcpv4/bsdp/bsdp_test.go b/dhcpv4/bsdp/bsdp_test.go
index cb1a456..2caa6e5 100644
--- a/dhcpv4/bsdp/bsdp_test.go
+++ b/dhcpv4/bsdp/bsdp_test.go
@@ -37,7 +37,7 @@ func TestParseBootImageListFromAck(t *testing.T) {
 		},
 	}
 	ack, _ := dhcpv4.New()
-	ack.AddOption(&OptVendorSpecificInformation{
+	ack.UpdateOption(&OptVendorSpecificInformation{
 		[]dhcpv4.Option{&OptBootImageList{expectedBootImages}},
 	})
 
@@ -69,7 +69,6 @@ func TestNewInformList_NoReplyPort(t *testing.T) {
 	require.True(t, m.Options.Has(dhcpv4.OptionVendorSpecificInformation))
 	require.True(t, m.Options.Has(dhcpv4.OptionParameterRequestList))
 	require.True(t, m.Options.Has(dhcpv4.OptionMaximumDHCPMessageSize))
-	require.True(t, m.Options.Has(dhcpv4.OptionEnd))
 
 	opt := m.GetOneOption(dhcpv4.OptionVendorSpecificInformation)
 	require.NotNil(t, opt, "vendor opts not present")
@@ -108,8 +107,7 @@ func newAck(hwAddr net.HardwareAddr, transactionID [4]byte) *dhcpv4.DHCPv4 {
 	ack.TransactionID = transactionID
 	ack.HWType = iana.HWTypeEthernet
 	ack.ClientHWAddr = hwAddr
-	ack.AddOption(&dhcpv4.OptMessageType{MessageType: dhcpv4.MessageTypeAck})
-	ack.AddOption(&dhcpv4.OptionGeneric{OptionCode: dhcpv4.OptionEnd})
+	ack.UpdateOption(&dhcpv4.OptMessageType{MessageType: dhcpv4.MessageTypeAck})
 	return ack
 }
 
@@ -127,7 +125,7 @@ func TestInformSelectForAck_Broadcast(t *testing.T) {
 	}
 	ack := newAck(hwAddr, tid)
 	ack.SetBroadcast()
-	ack.AddOption(&dhcpv4.OptServerIdentifier{ServerID: serverID})
+	ack.UpdateOption(&dhcpv4.OptServerIdentifier{ServerID: serverID})
 
 	m, err := InformSelectForAck(*ack, 0, bootImage)
 	require.NoError(t, err)
@@ -143,7 +141,6 @@ func TestInformSelectForAck_Broadcast(t *testing.T) {
 	require.True(t, m.Options.Has(dhcpv4.OptionDHCPMessageType))
 	opt := m.GetOneOption(dhcpv4.OptionDHCPMessageType)
 	require.Equal(t, dhcpv4.MessageTypeInform, opt.(*dhcpv4.OptMessageType).MessageType)
-	require.True(t, m.Options.Has(dhcpv4.OptionEnd))
 
 	// Validate vendor opts.
 	require.True(t, m.Options.Has(dhcpv4.OptionVendorSpecificInformation))
@@ -186,7 +183,7 @@ func TestInformSelectForAck_BadReplyPort(t *testing.T) {
 	}
 	ack := newAck(hwAddr, tid)
 	ack.SetBroadcast()
-	ack.AddOption(&dhcpv4.OptServerIdentifier{ServerID: serverID})
+	ack.UpdateOption(&dhcpv4.OptServerIdentifier{ServerID: serverID})
 
 	_, err := InformSelectForAck(*ack, 11223, bootImage)
 	require.Error(t, err, "expect error for > 1024 replyPort")
@@ -206,7 +203,7 @@ func TestInformSelectForAck_ReplyPort(t *testing.T) {
 	}
 	ack := newAck(hwAddr, tid)
 	ack.SetBroadcast()
-	ack.AddOption(&dhcpv4.OptServerIdentifier{ServerID: serverID})
+	ack.UpdateOption(&dhcpv4.OptServerIdentifier{ServerID: serverID})
 
 	replyPort := uint16(999)
 	m, err := InformSelectForAck(*ack, replyPort, bootImage)
@@ -281,9 +278,6 @@ func TestNewReplyForInformList(t *testing.T) {
 	RequireHasOption(t, ack.Options, &dhcpv4.OptClassIdentifier{Identifier: AppleVendorID})
 	require.NotNil(t, ack.GetOneOption(dhcpv4.OptionVendorSpecificInformation))
 
-	// Ensure options terminated with End option.
-	require.Equal(t, &dhcpv4.OptionGeneric{OptionCode: dhcpv4.OptionEnd}, ack.Options[len(ack.Options)-1])
-
 	// Vendor-specific options.
 	vendorOpts := ack.GetOneOption(dhcpv4.OptionVendorSpecificInformation).(*OptVendorSpecificInformation)
 	RequireHasOption(t, vendorOpts.Options, &OptMessageType{Type: MessageTypeList})
@@ -363,9 +357,6 @@ func TestNewReplyForInformSelect(t *testing.T) {
 	RequireHasOption(t, ack.Options, &dhcpv4.OptClassIdentifier{Identifier: AppleVendorID})
 	require.NotNil(t, ack.GetOneOption(dhcpv4.OptionVendorSpecificInformation))
 
-	// Ensure options are terminated with End option.
-	require.Equal(t, &dhcpv4.OptionGeneric{OptionCode: dhcpv4.OptionEnd}, ack.Options[len(ack.Options)-1])
-
 	vendorOpts := ack.GetOneOption(dhcpv4.OptionVendorSpecificInformation).(*OptVendorSpecificInformation)
 	RequireHasOption(t, vendorOpts.Options, &OptMessageType{Type: MessageTypeSelect})
 	RequireHasOption(t, vendorOpts.Options, &OptSelectedBootImageID{ID: images[0].ID})
@@ -424,7 +415,7 @@ func TestMessageTypeForPacket(t *testing.T) {
 		t.Run(tt.tcName, func(t *testing.T) {
 			pkt, _ = dhcpv4.New()
 			for _, opt := range tt.opts {
-				pkt.AddOption(opt)
+				pkt.UpdateOption(opt)
 			}
 			gotMessageType = MessageTypeFromPacket(pkt)
 			require.Equal(t, tt.wantMessageType, gotMessageType)
diff --git a/dhcpv4/bsdp/option_vendor_specific_information.go b/dhcpv4/bsdp/option_vendor_specific_information.go
index de144b3..a87135f 100644
--- a/dhcpv4/bsdp/option_vendor_specific_information.go
+++ b/dhcpv4/bsdp/option_vendor_specific_information.go
@@ -87,11 +87,6 @@ func (o *OptVendorSpecificInformation) String() string {
 	return s
 }
 
-// GetOption returns all suboptions that match the given OptionCode code.
-func (o *OptVendorSpecificInformation) GetOption(code dhcpv4.OptionCode) []dhcpv4.Option {
-	return o.Options.Get(code)
-}
-
 // GetOneOption returns the first suboption that matches the OptionCode code.
 func (o *OptVendorSpecificInformation) GetOneOption(code dhcpv4.OptionCode) dhcpv4.Option {
 	return o.Options.GetOne(code)
diff --git a/dhcpv4/bsdp/option_vendor_specific_information_test.go b/dhcpv4/bsdp/option_vendor_specific_information_test.go
index 64977c4..ede8a0b 100644
--- a/dhcpv4/bsdp/option_vendor_specific_information_test.go
+++ b/dhcpv4/bsdp/option_vendor_specific_information_test.go
@@ -166,52 +166,6 @@ func TestOptVendorSpecificInformationString(t *testing.T) {
 	require.Equal(t, expectedString, o.String())
 }
 
-func TestOptVendorSpecificInformationGetOptions(t *testing.T) {
-	// No option
-	o := &OptVendorSpecificInformation{
-		[]dhcpv4.Option{
-			&OptMessageType{MessageTypeList},
-			Version1_1,
-		},
-	}
-	foundOpts := o.GetOption(OptionBootImageList)
-	require.Empty(t, foundOpts, "should not get any options")
-
-	// One option
-	o = &OptVendorSpecificInformation{
-		[]dhcpv4.Option{
-			&OptMessageType{MessageTypeList},
-			Version1_1,
-		},
-	}
-	foundOpts = o.GetOption(OptionMessageType)
-	require.Equal(t, 1, len(foundOpts), "should only get one option")
-	require.Equal(t, MessageTypeList, foundOpts[0].(*OptMessageType).Type)
-
-	// Multiple options
-	//
-	// TODO: Remove this test when RFC 3396 is properly implemented. This
-	// isn't a valid packet. RFC 2131, Section 4.1: "Options may appear
-	// only once." RFC 3396 clarifies this to say that options will be
-	// concatenated. I.e., in this case, the bytes would be:
-	//
-	// <versioncode> 4 1 1 0 1
-	//
-	// Which would obviously not be parsed by the Version parser, as it
-	// only accepts two bytes.
-	o = &OptVendorSpecificInformation{
-		[]dhcpv4.Option{
-			&OptMessageType{MessageTypeList},
-			Version1_1,
-			Version1_0,
-		},
-	}
-	foundOpts = o.GetOption(OptionVersion)
-	require.Equal(t, 2, len(foundOpts), "should get two options")
-	require.Equal(t, Version1_1, foundOpts[0].(Version))
-	require.Equal(t, Version1_0, foundOpts[1].(Version))
-}
-
 func TestOptVendorSpecificInformationGetOneOption(t *testing.T) {
 	// No option
 	o := &OptVendorSpecificInformation{
@@ -232,15 +186,4 @@ func TestOptVendorSpecificInformationGetOneOption(t *testing.T) {
 	}
 	foundOpt = o.GetOneOption(OptionMessageType)
 	require.Equal(t, MessageTypeList, foundOpt.(*OptMessageType).Type)
-
-	// Multiple options
-	o = &OptVendorSpecificInformation{
-		[]dhcpv4.Option{
-			&OptMessageType{MessageTypeList},
-			Version1_1,
-			Version1_0,
-		},
-	}
-	foundOpt = o.GetOneOption(OptionVersion)
-	require.Equal(t, Version1_1, foundOpt.(Version))
 }
diff --git a/dhcpv4/dhcpv4.go b/dhcpv4/dhcpv4.go
index f4e3596..429d404 100644
--- a/dhcpv4/dhcpv4.go
+++ b/dhcpv4/dhcpv4.go
@@ -124,8 +124,6 @@ func New() (*DHCPv4, error) {
 		GatewayIPAddr: net.IPv4zero,
 		Options:       make([]Option, 0, 10),
 	}
-	// the End option has to be added explicitly
-	d.AddOption(&OptionGeneric{OptionCode: OptionEnd})
 	return &d, nil
 }
 
@@ -152,8 +150,8 @@ func NewDiscovery(hwaddr net.HardwareAddr) (*DHCPv4, error) {
 	d.HWType = iana.HWTypeEthernet
 	d.ClientHWAddr = hwaddr
 	d.SetBroadcast()
-	d.AddOption(&OptMessageType{MessageType: MessageTypeDiscover})
-	d.AddOption(&OptParameterRequestList{
+	d.UpdateOption(&OptMessageType{MessageType: MessageTypeDiscover})
+	d.UpdateOption(&OptParameterRequestList{
 		RequestedOpts: []OptionCode{
 			OptionSubnetMask,
 			OptionRouter,
@@ -205,7 +203,7 @@ func NewInform(hwaddr net.HardwareAddr, localIP net.IP) (*DHCPv4, error) {
 	d.HWType = iana.HWTypeEthernet
 	d.ClientHWAddr = hwaddr
 	d.ClientIPAddr = localIP
-	d.AddOption(&OptMessageType{MessageType: MessageTypeInform})
+	d.UpdateOption(&OptMessageType{MessageType: MessageTypeInform})
 	return d, nil
 }
 
@@ -235,9 +233,9 @@ func NewRequestFromOffer(offer *DHCPv4, modifiers ...Modifier) (*DHCPv4, error)
 		return nil, errors.New("Missing Server IP Address in DHCP Offer")
 	}
 	d.ServerIPAddr = serverIP
-	d.AddOption(&OptMessageType{MessageType: MessageTypeRequest})
-	d.AddOption(&OptRequestedIPAddress{RequestedAddr: offer.YourIPAddr})
-	d.AddOption(&OptServerIdentifier{ServerID: serverIP})
+	d.UpdateOption(&OptMessageType{MessageType: MessageTypeRequest})
+	d.UpdateOption(&OptRequestedIPAddress{RequestedAddr: offer.YourIPAddr})
+	d.UpdateOption(&OptServerIdentifier{ServerID: serverIP})
 	for _, mod := range modifiers {
 		d = mod(d)
 	}
@@ -359,48 +357,17 @@ func (d *DHCPv4) SetUnicast() {
 	d.Flags &= ^uint16(0x8000)
 }
 
-// GetOption will attempt to get all options that match a DHCPv4 option
-// from its OptionCode.  If the option was not found it will return an
-// empty list.
+// GetOneOption returns the option that matches the given option code.
 //
-// According to RFC 3396, options that are specified more than once are
-// concatenated, and hence this should always just return one option.
-func (d *DHCPv4) GetOption(code OptionCode) []Option {
-	return d.Options.Get(code)
-}
-
-// GetOneOption will attempt to get an  option that match a Option code.
-// If there are multiple options with the same OptionCode it will only return
-// the first one found.  If no matching option is found nil will be returned.
+// If no matching option is found, nil is returned.
 func (d *DHCPv4) GetOneOption(code OptionCode) Option {
 	return d.Options.GetOne(code)
 }
 
-// AddOption appends an option to the existing ones. If the last option is an
-// OptionEnd, it will be inserted before that. It does not deal with End
-// options that appead before the end, like in malformed packets.
-func (d *DHCPv4) AddOption(option Option) {
-	if len(d.Options) == 0 || d.Options[len(d.Options)-1].Code() != OptionEnd {
-		d.Options = append(d.Options, option)
-	} else {
-		end := d.Options[len(d.Options)-1]
-		d.Options[len(d.Options)-1] = option
-		d.Options = append(d.Options, end)
-	}
-}
-
-// UpdateOption updates the existing options with the passed option, adding it
-// at the end if not present already
+// UpdateOption replaces an existing option with the same option code with the
+// given one, adding it if not already present.
 func (d *DHCPv4) UpdateOption(option Option) {
-	for idx, opt := range d.Options {
-		if opt.Code() == option.Code() {
-			d.Options[idx] = option
-			// don't look further
-			return
-		}
-	}
-	// if not found, add it
-	d.AddOption(option)
+	d.Options.Update(option)
 }
 
 // MessageType returns the message type, trying to extract it from the
@@ -474,11 +441,13 @@ func (d *DHCPv4) Summary() string {
 // IsOptionRequested returns true if that option is within the requested
 // options of the DHCPv4 message.
 func (d *DHCPv4) IsOptionRequested(requested OptionCode) bool {
-	for _, optprl := range d.GetOption(OptionParameterRequestList) {
-		for _, o := range optprl.(*OptParameterRequestList).RequestedOpts {
-			if o == requested {
-				return true
-			}
+	optprl := d.GetOneOption(OptionParameterRequestList)
+	if optprl == nil {
+		return false
+	}
+	for _, o := range optprl.(*OptParameterRequestList).RequestedOpts {
+		if o == requested {
+			return true
 		}
 	}
 	return false
diff --git a/dhcpv4/dhcpv4_test.go b/dhcpv4/dhcpv4_test.go
index e5b8ad6..fb0ef70 100644
--- a/dhcpv4/dhcpv4_test.go
+++ b/dhcpv4/dhcpv4_test.go
@@ -180,64 +180,42 @@ func TestGetOption(t *testing.T) {
 	}
 
 	hostnameOpt := &OptionGeneric{OptionCode: OptionHostName, Data: []byte("darkstar")}
-	bootFileOpt1 := &OptBootfileName{"boot.img"}
-	bootFileOpt2 := &OptBootfileName{"boot2.img"}
-	d.AddOption(hostnameOpt)
-	d.AddOption(&OptBootfileName{"boot.img"})
-	d.AddOption(&OptBootfileName{"boot2.img"})
-
-	require.Equal(t, d.GetOption(OptionHostName), []Option{hostnameOpt})
-	require.Equal(t, d.GetOption(OptionBootfileName), []Option{bootFileOpt1, bootFileOpt2})
-	require.Equal(t, d.GetOption(OptionRouter), []Option{})
+	bootFileOpt := &OptBootfileName{"boot.img"}
+	d.UpdateOption(hostnameOpt)
+	d.UpdateOption(bootFileOpt)
 
 	require.Equal(t, d.GetOneOption(OptionHostName), hostnameOpt)
-	require.Equal(t, d.GetOneOption(OptionBootfileName), bootFileOpt1)
+	require.Equal(t, d.GetOneOption(OptionBootfileName), bootFileOpt)
 	require.Equal(t, d.GetOneOption(OptionRouter), nil)
 }
 
-func TestAddOption(t *testing.T) {
+func TestUpdateOption(t *testing.T) {
 	d, err := New()
 	require.NoError(t, err)
 
 	hostnameOpt := &OptionGeneric{OptionCode: OptionHostName, Data: []byte("darkstar")}
-	bootFileOpt1 := &OptionGeneric{OptionCode: OptionBootfileName, Data: []byte("boot.img")}
-	bootFileOpt2 := &OptionGeneric{OptionCode: OptionBootfileName, Data: []byte("boot2.img")}
-	d.AddOption(hostnameOpt)
-	d.AddOption(bootFileOpt1)
-	d.AddOption(bootFileOpt2)
+	bootFileOpt1 := &OptBootfileName{"boot.img"}
+	bootFileOpt2 := &OptBootfileName{"boot2.img"}
+	d.UpdateOption(hostnameOpt)
+	d.UpdateOption(bootFileOpt1)
+	d.UpdateOption(bootFileOpt2)
 
 	options := d.Options
-	require.Equal(t, len(options), 4)
-	require.Equal(t, options[3].Code(), OptionEnd)
-}
-
-func TestUpdateOption(t *testing.T) {
-	d, err := New()
-	require.NoError(t, err)
-	require.Equal(t, 1, len(d.Options))
-	require.Equal(t, OptionEnd, d.Options[0].Code())
-	// test that it will add the option since it's missing
-	d.UpdateOption(&OptDomainName{DomainName: "slackware.it"})
-	require.Equal(t, 2, len(d.Options))
-	require.Equal(t, OptionDomainName, d.Options[0].Code())
-	require.Equal(t, OptionEnd, d.Options[1].Code())
-	// test that it won't add another option of the same type
-	d.UpdateOption(&OptDomainName{DomainName: "slackware.it"})
-	require.Equal(t, 2, len(d.Options))
-	require.Equal(t, OptionDomainName, d.Options[0].Code())
-	require.Equal(t, OptionEnd, d.Options[1].Code())
+	require.Equal(t, len(options), 2)
+	require.Equal(t, d.Options.GetOne(OptionBootfileName), bootFileOpt2)
+	require.Equal(t, d.Options.GetOne(OptionHostName), hostnameOpt)
 }
 
 func TestDHCPv4NewRequestFromOffer(t *testing.T) {
 	offer, err := New()
 	require.NoError(t, err)
 	offer.SetBroadcast()
-	offer.AddOption(&OptMessageType{MessageType: MessageTypeOffer})
+	offer.UpdateOption(&OptMessageType{MessageType: MessageTypeOffer})
 	req, err := NewRequestFromOffer(offer)
 	require.Error(t, err)
 
 	// Now add the option so it doesn't error out.
-	offer.AddOption(&OptServerIdentifier{ServerID: net.IPv4(192, 168, 0, 1)})
+	offer.UpdateOption(&OptServerIdentifier{ServerID: net.IPv4(192, 168, 0, 1)})
 
 	// Broadcast request
 	req, err = NewRequestFromOffer(offer)
@@ -257,8 +235,8 @@ func TestDHCPv4NewRequestFromOffer(t *testing.T) {
 func TestDHCPv4NewRequestFromOfferWithModifier(t *testing.T) {
 	offer, err := New()
 	require.NoError(t, err)
-	offer.AddOption(&OptMessageType{MessageType: MessageTypeOffer})
-	offer.AddOption(&OptServerIdentifier{ServerID: net.IPv4(192, 168, 0, 1)})
+	offer.UpdateOption(&OptMessageType{MessageType: MessageTypeOffer})
+	offer.UpdateOption(&OptServerIdentifier{ServerID: net.IPv4(192, 168, 0, 1)})
 	userClass := WithUserClass([]byte("linuxboot"), false)
 	req, err := NewRequestFromOffer(offer, userClass)
 	require.NoError(t, err)
@@ -306,7 +284,6 @@ func TestNewDiscovery(t *testing.T) {
 	require.Equal(t, hwAddr, m.ClientHWAddr)
 	require.True(t, m.IsBroadcast())
 	require.True(t, m.Options.Has(OptionParameterRequestList))
-	require.True(t, m.Options.Has(OptionEnd))
 }
 
 func TestNewInform(t *testing.T) {
@@ -328,7 +305,7 @@ func TestIsOptionRequested(t *testing.T) {
 	require.False(t, pkt.IsOptionRequested(OptionDomainNameServer))
 
 	optprl := OptParameterRequestList{RequestedOpts: []OptionCode{OptionDomainNameServer}}
-	pkt.AddOption(&optprl)
+	pkt.UpdateOption(&optprl)
 	require.True(t, pkt.IsOptionRequested(OptionDomainNameServer))
 }
 
diff --git a/dhcpv4/modifiers.go b/dhcpv4/modifiers.go
index db67303..e7f6576 100644
--- a/dhcpv4/modifiers.go
+++ b/dhcpv4/modifiers.go
@@ -37,7 +37,7 @@ func WithHwAddr(hwaddr net.HardwareAddr) Modifier {
 // WithOption appends a DHCPv4 option provided by the user
 func WithOption(opt Option) Modifier {
 	return func(d *DHCPv4) *DHCPv4 {
-		d.AddOption(opt)
+		d.UpdateOption(opt)
 		return d
 	}
 }
@@ -52,7 +52,7 @@ func WithUserClass(uc []byte, rfc bool) Modifier {
 			UserClasses: [][]byte{uc},
 			Rfc3004:     rfc,
 		}
-		d.AddOption(&ouc)
+		d.UpdateOption(&ouc)
 		return d
 	}
 }
@@ -85,7 +85,7 @@ func WithNetboot(d *DHCPv4) *DHCPv4 {
 		OptParams = &OptParameterRequestList{
 			RequestedOpts: []OptionCode{OptionTFTPServerName, OptionBootfileName},
 		}
-		d.AddOption(OptParams)
+		d.UpdateOption(OptParams)
 	}
 	return d
 }
@@ -96,7 +96,7 @@ func WithRequestedOptions(optionCodes ...OptionCode) Modifier {
 		params := d.GetOneOption(OptionParameterRequestList)
 		if params == nil {
 			params = &OptParameterRequestList{}
-			d.AddOption(params)
+			d.UpdateOption(params)
 		}
 		opts := params.(*OptParameterRequestList)
 		for _, optionCode := range optionCodes {
diff --git a/dhcpv4/modifiers_test.go b/dhcpv4/modifiers_test.go
index 6d3976e..e653817 100644
--- a/dhcpv4/modifiers_test.go
+++ b/dhcpv4/modifiers_test.go
@@ -77,7 +77,7 @@ func TestWithNetbootExistingTFTP(t *testing.T) {
 	OptParams := &OptParameterRequestList{
 		RequestedOpts: []OptionCode{OptionTFTPServerName},
 	}
-	d.AddOption(OptParams)
+	d.UpdateOption(OptParams)
 	d = WithNetboot(d)
 	require.Equal(t, "Parameter Request List -> [TFTP Server Name, Bootfile Name]", d.Options[0].String())
 }
@@ -87,7 +87,7 @@ func TestWithNetbootExistingBootfileName(t *testing.T) {
 	OptParams := &OptParameterRequestList{
 		RequestedOpts: []OptionCode{OptionBootfileName},
 	}
-	d.AddOption(OptParams)
+	d.UpdateOption(OptParams)
 	d = WithNetboot(d)
 	require.Equal(t, "Parameter Request List -> [Bootfile Name, TFTP Server Name]", d.Options[0].String())
 }
@@ -97,7 +97,7 @@ func TestWithNetbootExistingBoth(t *testing.T) {
 	OptParams := &OptParameterRequestList{
 		RequestedOpts: []OptionCode{OptionBootfileName, OptionTFTPServerName},
 	}
-	d.AddOption(OptParams)
+	d.UpdateOption(OptParams)
 	d = WithNetboot(d)
 	require.Equal(t, "Parameter Request List -> [Bootfile Name, TFTP Server Name]", d.Options[0].String())
 }
diff --git a/dhcpv4/options.go b/dhcpv4/options.go
index 44bf1d3..3d774d1 100644
--- a/dhcpv4/options.go
+++ b/dhcpv4/options.go
@@ -95,21 +95,6 @@ func ParseOption(code OptionCode, data []byte) (Option, error) {
 // Options is a collection of options.
 type Options []Option
 
-// Get will attempt to get all options that match a DHCPv4 option from its
-// OptionCode.  If the option was not found it will return an empty list.
-//
-// According to RFC 3396, options that are specified more than once are
-// concatenated, and hence this should always just return one option.
-func (o Options) Get(code OptionCode) []Option {
-	opts := []Option{}
-	for _, opt := range o {
-		if opt.Code() == code {
-			opts = append(opts, opt)
-		}
-	}
-	return opts
-}
-
 // GetOne will attempt to get an  option that match a Option code.  If there
 // are multiple options with the same OptionCode it will only return the first
 // one found.  If no matching option is found nil will be returned.
@@ -127,6 +112,28 @@ func (o Options) Has(code OptionCode) bool {
 	return o.GetOne(code) != nil
 }
 
+// Update replaces an existing option with the same option code with the given
+// one, adding it if not already present.
+//
+// Per RFC 2131, Section 4.1, "options may appear only once."
+//
+// An End option is ignored.
+func (o *Options) Update(option Option) {
+	if option.Code() == OptionEnd {
+		return
+	}
+
+	for idx, opt := range *o {
+		if opt.Code() == option.Code() {
+			(*o)[idx] = option
+			// Don't look further.
+			return
+		}
+	}
+	// If not found, add it.
+	*o = append(*o, option)
+}
+
 // OptionsFromBytes parses a sequence of bytes until the end and builds a list
 // of options from it.
 //
@@ -158,8 +165,8 @@ func OptionsFromBytesWithParser(data []byte, coder OptionCodeGetter, parser Opti
 	options := make(map[OptionCode][]byte, 10)
 	var order []OptionCode
 
-	// Due to RFC 3396 allowing an option to be specified multiple times,
-	// we have to collect all option data first, and then parse it.
+	// Due to RFC 2131, 3396 allowing an option to be specified multiple
+	// times, we have to collect all option data first, and then parse it.
 	var end bool
 	for buf.Len() >= 1 {
 		// 1 byte: option code
@@ -187,8 +194,14 @@ func OptionsFromBytesWithParser(data []byte, coder OptionCodeGetter, parser Opti
 		if _, ok := options[c]; !ok {
 			order = append(order, c)
 		}
-		// RFC 3396: Just concatenate the data if the option code was
-		// specified multiple times.
+
+		// RFC 2131, Section 4.1 "Options may appear only once, [...].
+		// The client concatenates the values of multiple instances of
+		// the same option into a single parameter list for
+		// configuration."
+		//
+		// See also RFC 3396 for concatenation order and options longer
+		// than 255 bytes.
 		options[c] = append(options[c], data...)
 	}
 
@@ -205,7 +218,7 @@ func OptionsFromBytesWithParser(data []byte, coder OptionCodeGetter, parser Opti
 		}
 	}
 
-	opts := make(Options, 0, 10)
+	opts := make(Options, 0, len(options))
 	for _, code := range order {
 		parsedOpt, err := parser(code, options[code])
 		if err != nil {
diff --git a/dhcpv4/server_test.go b/dhcpv4/server_test.go
index bb2ee11..4307924 100644
--- a/dhcpv4/server_test.go
+++ b/dhcpv4/server_test.go
@@ -41,7 +41,7 @@ func DORAHandler(conn net.PacketConn, peer net.Addr, m *DHCPv4) {
 		log.Printf("NewReplyFromRequest failed: %v", err)
 		return
 	}
-	reply.AddOption(&OptServerIdentifier{ServerID: net.IP{1, 2, 3, 4}})
+	reply.UpdateOption(&OptServerIdentifier{ServerID: net.IP{1, 2, 3, 4}})
 	opt := m.GetOneOption(OptionDHCPMessageType)
 	if opt == nil {
 		log.Printf("No message type found!")
@@ -49,9 +49,9 @@ func DORAHandler(conn net.PacketConn, peer net.Addr, m *DHCPv4) {
 	}
 	switch opt.(*OptMessageType).MessageType {
 	case MessageTypeDiscover:
-		reply.AddOption(&OptMessageType{MessageType: MessageTypeOffer})
+		reply.UpdateOption(&OptMessageType{MessageType: MessageTypeOffer})
 	case MessageTypeRequest:
-		reply.AddOption(&OptMessageType{MessageType: MessageTypeAck})
+		reply.UpdateOption(&OptMessageType{MessageType: MessageTypeAck})
 	default:
 		log.Printf("Unhandled message type: %v", opt.(*OptMessageType).MessageType)
 		return
diff --git a/dhcpv4/ztpv4/ztp_test.go b/dhcpv4/ztpv4/ztp_test.go
index 9950bf5..680f15a 100644
--- a/dhcpv4/ztpv4/ztp_test.go
+++ b/dhcpv4/ztpv4/ztp_test.go
@@ -54,13 +54,13 @@ func TestParseV4VendorClass(t *testing.T) {
 			}
 
 			if tc.vc != "" {
-				packet.AddOption(&dhcpv4.OptClassIdentifier{
+				packet.UpdateOption(&dhcpv4.OptClassIdentifier{
 					Identifier: tc.vc,
 				})
 			}
 
 			if tc.hostname != "" {
-				packet.AddOption(&dhcpv4.OptHostName{
+				packet.UpdateOption(&dhcpv4.OptHostName{
 					HostName: tc.hostname,
 				})
 			}