tlv: implement delete and upsert methods

foodgroup/chat.go

@@ -137,7 +137,7 @@ func (s ChatService) transformChatMessage(inBody wire.SNAC_0x0E_0x05_ChatChannel
 		if enc == "ISO 8859" {
 			// fix malformed content encoding type sent by Kopete, which causes
 			// chat messages to show up blank in Windows AIM chat windows
-			msgBlock.Replace(wire.NewTLVBE(wire.ChatTLVMessageInfoEncoding, []byte("ISO-8859-1")))
+			msgBlock.Set(wire.NewTLVBE(wire.ChatTLVMessageInfoEncoding, []byte("ISO-8859-1")))
 		}
 	}
 

foodgroup/feedbag.go

@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
-	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -270,9 +269,7 @@ func (s *FeedbagService) UpsertItem(ctx context.Context, instance *state.Session
 					// Authorization has since been granted (IServerd strips
 					// SSI_TLV_AUTH before inserting when auth is already recorded).
 					// Clear the pending flag so the row is stored as a normal buddy.
-					item.TLVList = slices.DeleteFunc(item.TLVList, func(t wire.TLV) bool {
-						return t.Tag == wire.FeedbagAttributesPending
-					})
+					item.Remove(wire.FeedbagAttributesPending)
 				}
 			case instance.UIN() == 0 && sn.UIN() != 0:
 				// sender:aim, recipient: icq
@@ -863,9 +860,7 @@ func (s *FeedbagService) clearPendingAuth(ctx context.Context, granter state.Ide
 		return false, nil
 	}
 	// remove the pending buddy authorization tag
-	buddyItem.TLVList = slices.DeleteFunc(buddyItem.TLVList, func(tlv wire.TLV) bool {
-		return tlv.Tag == wire.FeedbagAttributesPending
-	})
+	buddyItem.Remove(wire.FeedbagAttributesPending)
 
 	updates := []wire.FeedbagItem{*buddyItem}
 	if err = s.feedbagManager.FeedbagUpsert(ctx, requester, updates); err != nil {

foodgroup/icbm.go

@@ -323,7 +323,7 @@ func addExternalIP(instance *state.SessionInstance, tlv wire.TLV) (wire.TLV, err
 		// server. unlike AOL's original behavior, this allows NATed clients
 		// to use rendezvous by replacing their LAN IP with the correct
 		// external IP.
-		frag.Replace(wire.NewTLVBE(wire.ICBMRdvTLVTagsRequesterIP, ip.AsSlice()))
+		frag.Set(wire.NewTLVBE(wire.ICBMRdvTLVTagsRequesterIP, ip.AsSlice()))
 		// append the client's IP as seen by the server. the recipient uses
 		// this to verify that the sender's claimed IP matches what the server
 		// detects. although redundant since we override the requester IP

foodgroup/oservice.go

@@ -273,7 +273,7 @@ func (s OServiceService) SetUserInfoFields(ctx context.Context, instance *state.
 	// it does not reflect aggregated state of the session. this is necessary
 	// for the "invisible" button to properly toggle on the client.
 	info := instance.Session().TLVUserInfo()
-	info.Replace(wire.NewTLVBE(wire.OServiceUserInfoStatus, instance.UserStatusBitmask()))
+	info.Set(wire.NewTLVBE(wire.OServiceUserInfoStatus, instance.UserStatusBitmask()))
 
 	return wire.SNACMessage{
 		Frame: wire.SNACFrame{

state/feedbag_list.go

@@ -112,6 +112,19 @@ func (f *FeedbagList) DeleteGroup(groupName string) {
 // AddBuddy upserts a buddy item in the given group (by name), optionally
 // attaching alias and note attributes. Returns true if a new buddy was inserted.
 func (f *FeedbagList) AddBuddy(groupName, screenName, alias, note string) (bool, error) {
+	var attrs wire.TLVList
+	if alias != "" {
+		attrs.Append(wire.NewTLVBE(wire.FeedbagAttributesAlias, alias))
+	}
+	if note != "" {
+		attrs.Append(wire.NewTLVBE(wire.FeedbagAttributesNote, note))
+	}
+	return f.addBuddyItem(groupName, screenName, attrs)
+}
+
+// addBuddyItem upserts a buddy item carrying the given attribute TLVs into the
+// named group. Returns true if a new buddy was inserted.
+func (f *FeedbagList) addBuddyItem(groupName, screenName string, attrs wire.TLVList) (bool, error) {
 	group := f.groupByName(groupName)
 	if group == nil {
 		return false, fmt.Errorf("group %q not found", groupName)
@@ -121,12 +134,7 @@ func (f *FeedbagList) AddBuddy(groupName, screenName, alias, note string) (bool,
 		GroupID: group.GroupID,
 		Name:    screenName,
 	}
-	if alias != "" {
-		item.Append(wire.NewTLVBE(wire.FeedbagAttributesAlias, alias))
-	}
-	if note != "" {
-		item.Append(wire.NewTLVBE(wire.FeedbagAttributesNote, note))
-	}
+	item.TLVList = attrs
 	result, inserted := f.upsertItem(item)
 	if inserted {
 		group.AppendOrderMembers(result.ItemID)
@@ -196,7 +204,7 @@ func (f *FeedbagList) RenameGroup(oldName, newName string) error {
 // order. When toGroup names a different group than fromGroup, the buddy's
 // feedbag item is deleted from the source group and re-inserted into the
 // destination (a buddy's identity includes its GroupID at the protocol level),
-// carrying over its alias and note attributes. When beforeBuddy is non-empty,
+// carrying over all of its attribute TLVs. When beforeBuddy is non-empty,
 // the buddy is positioned immediately before that buddy in the destination
 // group's order; otherwise it is appended. Returns ErrGroupNotFound or
 // ErrBuddyNotFound if the source group/buddy or destination group is missing.
@@ -217,13 +225,15 @@ func (f *FeedbagList) MoveBuddy(fromGroup, toGroup, buddyName, beforeBuddy strin
 			return fmt.Errorf("%w: %q", ErrGroupNotFound, toGroup)
 		}
 
-		alias, _ := srcBuddy.String(wire.FeedbagAttributesAlias)
-		note, _ := srcBuddy.String(wire.FeedbagAttributesNote)
+		// preserve every attribute TLV (alias, note, auth state, etc.), not
+		// just alias/note. Clone so the re-inserted item does not share a
+		// backing array with the snapshot queued for deletion.
+		attrs := slices.Clone(srcBuddy.TLVList)
 
 		if err := f.DeleteBuddy(fromGroup, buddyName); err != nil {
 			return err
 		}
-		if _, err := f.AddBuddy(toGroup, buddyName, alias, note); err != nil {
+		if _, err := f.addBuddyItem(toGroup, buddyName, attrs); err != nil {
 			return err
 		}
 	}
@@ -249,9 +259,9 @@ func (f *FeedbagList) SetBuddyAlias(buddyName, alias string) (bool, error) {
 	}
 	for _, buddy := range buddies {
 		if alias != "" {
-			setItemTLV(buddy, wire.FeedbagAttributesAlias, alias)
+			buddy.Set(wire.NewTLVBE(wire.FeedbagAttributesAlias, alias))
 		} else {
-			clearItemTLV(buddy, wire.FeedbagAttributesAlias)
+			buddy.Remove(wire.FeedbagAttributesAlias)
 		}
 		f.trackUpdate(buddy)
 	}
@@ -267,9 +277,9 @@ func (f *FeedbagList) SetGroupCollapsed(groupName string, collapsed bool) error
 		return fmt.Errorf("%w: %q", ErrGroupNotFound, groupName)
 	}
 	if collapsed {
-		setItemTLV(group, wire.FeedbagAttributesCollapsed, []byte{})
+		group.Set(wire.NewTLVBE(wire.FeedbagAttributesCollapsed, []byte{}))
 	} else {
-		clearItemTLV(group, wire.FeedbagAttributesCollapsed)
+		group.Remove(wire.FeedbagAttributesCollapsed)
 	}
 	f.trackUpdate(group)
 	return nil
@@ -468,36 +478,10 @@ func (f *FeedbagList) reorderInGroupOrder(group *wire.FeedbagItem, itemID, befor
 	reordered = append(reordered, itemID)
 	reordered = append(reordered, filtered[insertAt:]...)
 
-	if group.HasTag(wire.FeedbagAttributesOrder) {
-		group.Replace(wire.NewTLVBE(wire.FeedbagAttributesOrder, reordered))
-	} else {
-		group.Append(wire.NewTLVBE(wire.FeedbagAttributesOrder, reordered))
-	}
+	group.Set(wire.NewTLVBE(wire.FeedbagAttributesOrder, reordered))
 	f.trackUpdate(group)
 }
 
-// setItemTLV sets a single attribute TLV on item, replacing an existing TLV
-// with the same tag or appending a new one. Mirrors AppendOrderMembers'
-// replace-or-append pattern since TLVList.Replace no-ops when the tag is absent.
-func setItemTLV(item *wire.FeedbagItem, tag uint16, value any) {
-	if item.HasTag(tag) {
-		item.Replace(wire.NewTLVBE(tag, value))
-	} else {
-		item.Append(wire.NewTLVBE(tag, value))
-	}
-}
-
-// clearItemTLV removes every TLV with the given tag from item.
-func clearItemTLV(item *wire.FeedbagItem, tag uint16) {
-	filtered := item.TLVList[:0:0]
-	for _, tlv := range item.TLVList {
-		if tlv.Tag != tag {
-			filtered = append(filtered, tlv)
-		}
-	}
-	item.TLVList = filtered
-}
-
 // trackUpdate adds item to the pending-updates list if not already present.
 func (f *FeedbagList) trackUpdate(item *wire.FeedbagItem) {
 	if slices.Contains(f.pendingUpdates, item) {

state/feedbag_list_test.go

@@ -1144,7 +1144,7 @@ func TestFeedbagList_RenameGroup(t *testing.T) {
 }
 
 func TestFeedbagList_MoveBuddy(t *testing.T) {
-	t.Run("moves buddy across groups carrying alias", func(t *testing.T) {
+	t.Run("moves buddy across groups carrying all attributes", func(t *testing.T) {
 		fl := NewFeedbagList([]wire.FeedbagItem{
 			{Name: "Buddies", ClassID: wire.FeedbagClassIdGroup, GroupID: 1,
 				TLVLBlock: wire.TLVLBlock{TLVList: wire.TLVList{
@@ -1153,6 +1153,9 @@ func TestFeedbagList_MoveBuddy(t *testing.T) {
 			{Name: "alice", ClassID: wire.FeedbagClassIdBuddy, GroupID: 1, ItemID: 10,
 				TLVLBlock: wire.TLVLBlock{TLVList: wire.TLVList{
 					wire.NewTLVBE(wire.FeedbagAttributesAlias, "Al"),
+					wire.NewTLVBE(wire.FeedbagAttributesNote, "friend"),
+					// a non-alias/note attribute that must survive the move
+					wire.NewTLVBE(wire.FeedbagAttributesPending, []byte{}),
 				}}},
 			{Name: "Coworkers", ClassID: wire.FeedbagClassIdGroup, GroupID: 2},
 		}, func(n int) int { return 99 })
@@ -1177,6 +1180,11 @@ func TestFeedbagList_MoveBuddy(t *testing.T) {
 		alias, ok := newBuddy.Bytes(wire.FeedbagAttributesAlias)
 		assert.True(t, ok)
 		assert.Equal(t, []byte("Al"), alias)
+		note, ok := newBuddy.Bytes(wire.FeedbagAttributesNote)
+		assert.True(t, ok)
+		assert.Equal(t, []byte("friend"), note)
+		// the non-alias/note attribute is preserved rather than stripped
+		assert.True(t, newBuddy.HasTag(wire.FeedbagAttributesPending))
 	})
 
 	t.Run("reorders within a group before another buddy", func(t *testing.T) {

wire/buddy_prefs.go

@@ -215,18 +215,8 @@ func SetBuddyPref(list TLVList, prefNum uint16, value bool) TLVList {
 		valueBytes[index] &^= mask
 	}
 
-	list = setTLVBytes(list, validTag, validBytes)
-	list = setTLVBytes(list, valueTag, valueBytes)
-	return list
-}
-
-// setTLVBytes replaces the value of tag in list, or appends it when absent.
-func setTLVBytes(list TLVList, tag uint16, b []byte) TLVList {
-	if list.HasTag(tag) {
-		list.Replace(NewTLVBE(tag, b))
-		return list
-	}
-	list.Append(NewTLVBE(tag, b))
+	list.Set(NewTLVBE(validTag, validBytes))
+	list.Set(NewTLVBE(valueTag, valueBytes))
 	return list
 }
 

wire/snacs.go

@@ -2800,11 +2800,8 @@ func (f *FeedbagItem) IsEqual(other FeedbagItem) bool {
 // the order TLV already exists, the new IDs are appended to it. Otherwise a
 // new order TLV is created.
 func (f *FeedbagItem) AppendOrderMembers(memberIDs ...uint16) {
-	if existing, ok := f.Uint16SliceBE(FeedbagAttributesOrder); ok {
-		f.Replace(NewTLVBE(FeedbagAttributesOrder, append(existing, memberIDs...)))
-	} else {
-		f.Append(NewTLVBE(FeedbagAttributesOrder, memberIDs))
-	}
+	existing, _ := f.Uint16SliceBE(FeedbagAttributesOrder)
+	f.Set(NewTLVBE(FeedbagAttributesOrder, append(existing, memberIDs...)))
 }
 
 // RemoveOrderMembers removes the specified member IDs from the item's order
@@ -2824,7 +2821,7 @@ func (f *FeedbagItem) RemoveOrderMembers(memberIDs ...uint16) {
 			filtered = append(filtered, id)
 		}
 	}
-	f.Replace(NewTLVBE(FeedbagAttributesOrder, filtered))
+	f.Set(NewTLVBE(FeedbagAttributesOrder, filtered))
 }
 
 // ICQDCInfo represents ICQ direct connect settings.

wire/tlv.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/binary"
 	"fmt"
+	"slices"
 )
 
 // TLV represents dynamically typed data in the OSCAR protocol. Each message
@@ -142,14 +143,27 @@ func (s *TLVList) HasTag(tag uint16) bool {
 	return false
 }
 
-// Replace updates the values of TLVs in the list with the same tag as new. If
-// no matching tag is found, the list remains unchanged.
-func (s *TLVList) Replace(new TLV) {
+// Set replaces every TLV with the same tag with the new TLV. If no matching tag
+// is found, the new TLV is appended to the list.
+func (s *TLVList) Set(new TLV) {
+	found := false
 	for i, old := range *s {
 		if old.Tag == new.Tag {
 			(*s)[i].Value = new.Value
+			found = true
 		}
 	}
+	if !found {
+		*s = append(*s, new)
+	}
+}
+
+// Remove deletes every TLV with the given tag from the list. If no matching tag
+// is found, the list remains unchanged.
+func (s *TLVList) Remove(tag uint16) {
+	*s = slices.DeleteFunc(*s, func(tlv TLV) bool {
+		return tlv.Tag == tag
+	})
 }
 
 // String retrieves the string value associated with the specified tag from the

wire/tlv_test.go

@@ -406,7 +406,7 @@ func TestTLVList_ICQString(t *testing.T) {
 	})
 }
 
-func TestTLVList_Replace(t *testing.T) {
+func TestTLVList_Set(t *testing.T) {
 	tests := []struct {
 		name        string
 		given       TLVList
@@ -443,12 +443,74 @@ func TestTLVList_Replace(t *testing.T) {
 				NewTLVLE(0x02, []byte{0x02}),
 				NewTLVLE(0x01, []byte{0x03}),
 				NewTLVLE(0x03, []byte{0x04}),
+				NewTLVLE(0x07, []byte{0xAA}),
 			},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			tt.given.Replace(tt.replacement)
+			tt.given.Set(tt.replacement)
+			assert.Equal(t, tt.want, tt.given)
+		})
+	}
+}
+
+func TestTLVList_Remove(t *testing.T) {
+	tests := []struct {
+		name  string
+		given TLVList
+		tag   uint16
+		want  TLVList
+	}{
+		{
+			name: "remove multiple TLVs with the same tag",
+			given: TLVList{
+				NewTLVLE(0x01, []byte{0x01}),
+				NewTLVLE(0x02, []byte{0x02}),
+				NewTLVLE(0x01, []byte{0x03}),
+				NewTLVLE(0x03, []byte{0x04}),
+			},
+			tag: 0x01,
+			want: TLVList{
+				NewTLVLE(0x02, []byte{0x02}),
+				NewTLVLE(0x03, []byte{0x04}),
+			},
+		},
+		{
+			name: "remove a single TLV",
+			given: TLVList{
+				NewTLVLE(0x01, []byte{0x01}),
+				NewTLVLE(0x02, []byte{0x02}),
+				NewTLVLE(0x03, []byte{0x04}),
+			},
+			tag: 0x02,
+			want: TLVList{
+				NewTLVLE(0x01, []byte{0x01}),
+				NewTLVLE(0x03, []byte{0x04}),
+			},
+		},
+		{
+			name: "no matching tag leaves the list unchanged",
+			given: TLVList{
+				NewTLVLE(0x01, []byte{0x01}),
+				NewTLVLE(0x02, []byte{0x02}),
+			},
+			tag: 0x07,
+			want: TLVList{
+				NewTLVLE(0x01, []byte{0x01}),
+				NewTLVLE(0x02, []byte{0x02}),
+			},
+		},
+		{
+			name:  "remove from an empty list",
+			given: TLVList{},
+			tag:   0x01,
+			want:  TLVList{},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tt.given.Remove(tt.tag)
 			assert.Equal(t, tt.want, tt.given)
 		})
 	}