4
0
Mike 2 өдөр өмнө
parent
commit
83e0b81c19

+ 14 - 0
cmd/server/factory.go

@@ -525,10 +525,23 @@ func WebAPI(deps Container) *webapi.Server {
 		deps.sqLiteUserStore,
 	)
 
+	iconSource := handlers.BuddyIconSource{
+		IconRetriever: deps.sqLiteUserStore,
+		BARTService: foodgroup.NewBARTService(
+			logger,
+			deps.sqLiteUserStore,
+			deps.inMemorySessionManager,
+			deps.sqLiteUserStore,
+			deps.inMemorySessionManager,
+		),
+		Logger: logger,
+	}
+
 	// Create WebAPI buddy list manager (local to WebAPI)
 	buddyListManager := handlers.NewBuddyListManager(
 		deps.feedbagSvc,
 		locateService,
+		iconSource,
 		logger,
 	)
 
@@ -591,6 +604,7 @@ func WebAPI(deps Container) *webapi.Server {
 		LowerWarnLevel:     deps.icbmSvc.UpdateWarnLevel,
 		FeedbagService:     deps.feedbagSvc,
 		DirSearchService:   foodgroup.NewODirService(logger, deps.sqLiteUserStore),
+		IconSource:         iconSource,
 	}
 	// Pass SQLiteUserStore as the API key validator (it implements middleware.APIKeyValidator)
 	return webapi.NewServer(deps.cfg.WebAPIListeners, logger, handler, deps.sqLiteUserStore, deps.webAPISessionManager)

+ 2 - 0
server/webapi/handler.go

@@ -7,6 +7,7 @@ import (
 	"log/slog"
 	"net/http"
 
+	"github.com/mk6i/open-oscar-server/server/webapi/handlers"
 	"github.com/mk6i/open-oscar-server/state"
 )
 
@@ -27,6 +28,7 @@ type Handler struct {
 	ChatSessionManager ChatSessionManager
 	FeedbagService     FeedbagService
 	DirSearchService   DirSearchService
+	IconSource         handlers.BuddyIconSource
 }
 
 func (h Handler) GetHelloWorldHandler(w http.ResponseWriter, r *http.Request) {

+ 173 - 0
server/webapi/handlers/buddy_icon.go

@@ -0,0 +1,173 @@
+package handlers
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"log/slog"
+	"net/url"
+
+	"github.com/mk6i/open-oscar-server/state"
+	"github.com/mk6i/open-oscar-server/wire"
+)
+
+// ErrNoBuddyIcon indicates that a user has not set a buddy icon.
+var ErrNoBuddyIcon = errors.New("no buddy icon")
+
+// BARTService retrieves BART (Buddy Art) assets by hash.
+type BARTService interface {
+	RetrieveItem(ctx context.Context, inFrame wire.SNACFrame, inBody wire.SNAC_0x10_0x04_BARTDownloadQuery) (wire.SNACMessage, error)
+}
+
+// BuddyIconRetriever resolves a user's buddy icon reference. References live in
+// the feedbag rather than on the session, so they resolve for offline users too.
+type BuddyIconRetriever interface {
+	BuddyIconMetadata(ctx context.Context, screenName state.IdentScreenName) (*wire.BARTID, error)
+}
+
+// BuddyIconSource resolves buddy icons, both as URLs to publish to the web
+// client and as the image bytes those URLs serve.
+//
+// The client never derives an icon URL: it renders whatever string the server
+// puts in a user's buddyIcon field, falling back to a blank-person placeholder
+// when the field is absent.
+type BuddyIconSource struct {
+	IconRetriever BuddyIconRetriever
+	BARTService   BARTService
+	Logger        *slog.Logger
+}
+
+// URL returns the absolute, content-addressed URL that screenName's buddy icon
+// is served from, or an empty string if the user has no icon.
+//
+// The icon hash is part of the URL so that the URL changes whenever the user
+// changes their icon. Browsers cache icons by URL, and the client refetches a
+// user's large icon only when it observes buddyIconUrl change.
+func (s BuddyIconSource) URL(ctx context.Context, baseURL string, screenName state.IdentScreenName) string {
+	// The client loads icons from a different origin than the page it runs on,
+	// so a URL is only publishable if it can be made absolute. Callers that have
+	// no origin to build against pass an empty baseURL to opt out.
+	if baseURL == "" {
+		return ""
+	}
+
+	id, err := s.iconID(ctx, screenName)
+	if err != nil {
+		if !errors.Is(err, ErrNoBuddyIcon) {
+			s.Logger.WarnContext(ctx, "failed to resolve buddy icon",
+				"screenName", screenName.String(), "err", err.Error())
+		}
+		return ""
+	}
+
+	return iconURL(baseURL, screenName, id.Hash)
+}
+
+// PublishedURL returns a buddyIcon URL that is always non-empty when baseURL is
+// set: the content-addressed URL when the user has an icon, otherwise a hash-less
+// URL that resolves to the blank placeholder.
+//
+// Callers that publish icons into buddy-list, presence, or myInfo payloads use
+// this so the client always receives a URL. The web client's shallow user-object
+// merge never drops a stale buddyIconUrl on its own, so a user who clears their
+// icon only stops rendering it once a *different* URL arrives; the hash-less
+// placeholder URL is that different URL.
+func (s BuddyIconSource) PublishedURL(ctx context.Context, baseURL string, screenName state.IdentScreenName) string {
+	if baseURL == "" {
+		return ""
+	}
+
+	id, err := s.iconID(ctx, screenName)
+	switch {
+	case errors.Is(err, ErrNoBuddyIcon):
+		// No icon set: publish the hash-less placeholder rather than nothing, so
+		// a cleared icon propagates to the client.
+		return iconURL(baseURL, screenName, nil)
+	case err != nil:
+		s.Logger.WarnContext(ctx, "failed to resolve buddy icon",
+			"screenName", screenName.String(), "err", err.Error())
+		return ""
+	}
+
+	return iconURL(baseURL, screenName, id.Hash)
+}
+
+// URLForHash formats a buddyIcon URL for a hash already known to the caller,
+// skipping the metadata lookup that URL/PublishedURL do. The event pump uses this
+// on presence broadcasts, whose SNAC already carries the buddy's icon hash (TLV
+// wire.OServiceUserInfoBARTInfo).
+//
+// A non-empty hash yields the content-addressed URL; a nil/empty hash yields the
+// hash-less placeholder URL (which serves the blank icon), so a buddy who cleared
+// or never set an icon still gets a non-empty URL the client's shallow merge can
+// act on. An empty baseURL (no origin to build against) yields "".
+func (s BuddyIconSource) URLForHash(baseURL string, screenName state.IdentScreenName, hash []byte) string {
+	if baseURL == "" {
+		return ""
+	}
+	return iconURL(baseURL, screenName, hash)
+}
+
+// iconURL formats the expressions endpoint URL for screenName's icon. A non-empty
+// hash is content-addressed and cacheable; an empty hash yields the placeholder
+// form that serves the blank icon.
+func iconURL(baseURL string, screenName state.IdentScreenName, hash []byte) string {
+	if len(hash) == 0 {
+		return fmt.Sprintf("%s/expressions/get?t=%s&type=buddyIcon",
+			baseURL, url.QueryEscape(screenName.String()))
+	}
+	return fmt.Sprintf("%s/expressions/get?t=%s&type=buddyIcon&bartId=%x",
+		baseURL, url.QueryEscape(screenName.String()), hash)
+}
+
+// Image returns the image bytes of screenName's current buddy icon. It returns
+// ErrNoBuddyIcon if the user has no icon set.
+func (s BuddyIconSource) Image(ctx context.Context, screenName state.IdentScreenName) ([]byte, error) {
+	id, err := s.iconID(ctx, screenName)
+	if err != nil {
+		return nil, err
+	}
+	return s.ImageForHash(ctx, screenName, id.Hash)
+}
+
+// ImageForHash returns the bytes of the BART asset identified by hash for
+// screenName, independent of the user's current icon reference. This lets a
+// content-addressed URL resolve to the exact image its hash names, so a URL that
+// was cached as immutable never resolves to a different image later. It returns
+// ErrNoBuddyIcon if no asset with that hash exists. Passing the clear-icon hash
+// yields the blank placeholder image.
+func (s BuddyIconSource) ImageForHash(ctx context.Context, screenName state.IdentScreenName, hash []byte) ([]byte, error) {
+	msg, err := s.BARTService.RetrieveItem(ctx, wire.SNACFrame{}, wire.SNAC_0x10_0x04_BARTDownloadQuery{
+		ScreenName: screenName.String(),
+		BARTID: wire.BARTID{
+			Type:     wire.BARTTypesBuddyIcon,
+			BARTInfo: wire.BARTInfo{Hash: hash},
+		},
+	})
+	if err != nil {
+		return nil, fmt.Errorf("RetrieveItem: %w", err)
+	}
+
+	reply, ok := msg.Body.(wire.SNAC_0x10_0x05_BARTDownloadReply)
+	if !ok {
+		return nil, fmt.Errorf("unexpected BART reply body type %T", msg.Body)
+	}
+	if len(reply.Data) == 0 {
+		return nil, ErrNoBuddyIcon
+	}
+
+	return reply.Data, nil
+}
+
+// iconID looks up a user's buddy icon reference, translating "no icon" and
+// "icon cleared" into ErrNoBuddyIcon.
+func (s BuddyIconSource) iconID(ctx context.Context, screenName state.IdentScreenName) (*wire.BARTID, error) {
+	id, err := s.IconRetriever.BuddyIconMetadata(ctx, screenName)
+	if err != nil {
+		return nil, fmt.Errorf("BuddyIconMetadata: %w", err)
+	}
+	if id == nil || id.HasClearIconHash() || len(id.Hash) == 0 {
+		return nil, ErrNoBuddyIcon
+	}
+	return id, nil
+}

+ 263 - 0
server/webapi/handlers/buddy_icon_test.go

@@ -0,0 +1,263 @@
+package handlers
+
+import (
+	"context"
+	"errors"
+	"log/slog"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+
+	"github.com/mk6i/open-oscar-server/state"
+	"github.com/mk6i/open-oscar-server/wire"
+)
+
+// iconGIF stands in for buddy icon image bytes.
+var iconGIF = []byte{0x47, 0x49, 0x46, 0x38, 0x39, 0x61, 0x01, 0x00}
+
+func bartID(hash []byte) *wire.BARTID {
+	return &wire.BARTID{
+		Type:     wire.BARTTypesBuddyIcon,
+		BARTInfo: wire.BARTInfo{Flags: wire.BARTFlagsCustom, Hash: hash},
+	}
+}
+
+func TestBuddyIconSource_URL(t *testing.T) {
+	tests := []struct {
+		name    string
+		baseURL string
+		id      *wire.BARTID
+		idErr   error
+		want    string
+	}{
+		{
+			name:    "user with an icon gets a URL carrying the icon hash",
+			baseURL: "http://api.example.com",
+			id:      bartID([]byte{0xde, 0xad, 0xbe, 0xef}),
+			want:    "http://api.example.com/expressions/get?t=mikekelly&type=buddyIcon&bartId=deadbeef",
+		},
+		{
+			name:    "user without an icon gets no URL",
+			baseURL: "http://api.example.com",
+			id:      nil,
+			want:    "",
+		},
+		{
+			name:    "a cleared icon is not published",
+			baseURL: "http://api.example.com",
+			id:      bartID(wire.GetClearIconHash()),
+			want:    "",
+		},
+		{
+			name:    "a lookup failure is not fatal, it just yields no icon",
+			baseURL: "http://api.example.com",
+			idErr:   errors.New("db exploded"),
+			want:    "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			iconRetriever := &MockBuddyIconRetriever{}
+			iconRetriever.On("BuddyIconMetadata", mock.Anything, state.NewIdentScreenName("mikekelly")).
+				Return(tt.id, tt.idErr).Once()
+
+			s := BuddyIconSource{IconRetriever: iconRetriever, Logger: slog.Default()}
+			got := s.URL(context.Background(), tt.baseURL, state.NewIdentScreenName("mikekelly"))
+
+			assert.Equal(t, tt.want, got)
+			iconRetriever.AssertExpectations(t)
+		})
+	}
+}
+
+func TestBuddyIconSource_URL_NoBaseURLSkipsLookup(t *testing.T) {
+	// Callers with no origin to build an absolute URL against opt out by passing
+	// an empty baseURL. That must not cost a lookup.
+	iconRetriever := &MockBuddyIconRetriever{}
+
+	s := BuddyIconSource{IconRetriever: iconRetriever, Logger: slog.Default()}
+	got := s.URL(context.Background(), "", state.NewIdentScreenName("mikekelly"))
+
+	assert.Empty(t, got)
+	iconRetriever.AssertNotCalled(t, "BuddyIconMetadata", mock.Anything, mock.Anything)
+}
+
+func TestBuddyIconSource_URL_UsesNormalizedScreenName(t *testing.T) {
+	// The URL targets the normalized screen name, which is what the endpoint
+	// resolves against and what the client keys users by.
+	iconRetriever := &MockBuddyIconRetriever{}
+	iconRetriever.On("BuddyIconMetadata", mock.Anything, mock.Anything).
+		Return(bartID([]byte{0x01}), nil).Once()
+
+	s := BuddyIconSource{IconRetriever: iconRetriever, Logger: slog.Default()}
+	got := s.URL(context.Background(), "http://api.example.com", state.NewIdentScreenName("Mike Kelly"))
+
+	assert.Equal(t, "http://api.example.com/expressions/get?t=mikekelly&type=buddyIcon&bartId=01", got)
+}
+
+func TestBuddyIconSource_Image(t *testing.T) {
+	hash := []byte{0xde, 0xad}
+
+	iconRetriever := &MockBuddyIconRetriever{}
+	iconRetriever.On("BuddyIconMetadata", mock.Anything, state.NewIdentScreenName("mikekelly")).
+		Return(bartID(hash), nil).Once()
+
+	// Image resolves the current hash from metadata, then downloads that exact
+	// hash. The download query is keyed by hash; flags are irrelevant to the
+	// lookup, so it carries only the type and hash.
+	bartService := &MockBARTService{}
+	bartService.On("RetrieveItem", mock.Anything, wire.SNACFrame{}, wire.SNAC_0x10_0x04_BARTDownloadQuery{
+		ScreenName: "mikekelly",
+		BARTID:     wire.BARTID{Type: wire.BARTTypesBuddyIcon, BARTInfo: wire.BARTInfo{Hash: hash}},
+	}).Return(wire.SNACMessage{
+		Body: wire.SNAC_0x10_0x05_BARTDownloadReply{Data: iconGIF},
+	}, nil).Once()
+
+	s := BuddyIconSource{IconRetriever: iconRetriever, BARTService: bartService, Logger: slog.Default()}
+	got, err := s.Image(context.Background(), state.NewIdentScreenName("mikekelly"))
+
+	assert.NoError(t, err)
+	assert.Equal(t, iconGIF, got)
+	iconRetriever.AssertExpectations(t)
+	bartService.AssertExpectations(t)
+}
+
+func TestBuddyIconSource_ImageForHash(t *testing.T) {
+	hash := []byte{0xca, 0xfe}
+
+	// ImageForHash downloads the requested hash directly, without consulting the
+	// user's current icon metadata.
+	bartService := &MockBARTService{}
+	bartService.On("RetrieveItem", mock.Anything, wire.SNACFrame{}, wire.SNAC_0x10_0x04_BARTDownloadQuery{
+		ScreenName: "mikekelly",
+		BARTID:     wire.BARTID{Type: wire.BARTTypesBuddyIcon, BARTInfo: wire.BARTInfo{Hash: hash}},
+	}).Return(wire.SNACMessage{
+		Body: wire.SNAC_0x10_0x05_BARTDownloadReply{Data: iconGIF},
+	}, nil).Once()
+
+	s := BuddyIconSource{BARTService: bartService, Logger: slog.Default()}
+	got, err := s.ImageForHash(context.Background(), state.NewIdentScreenName("mikekelly"), hash)
+
+	assert.NoError(t, err)
+	assert.Equal(t, iconGIF, got)
+	bartService.AssertExpectations(t)
+}
+
+func TestBuddyIconSource_ImageForHash_NotFound(t *testing.T) {
+	bartService := &MockBARTService{}
+	bartService.On("RetrieveItem", mock.Anything, mock.Anything, mock.Anything).
+		Return(wire.SNACMessage{Body: wire.SNAC_0x10_0x05_BARTDownloadReply{}}, nil).Once()
+
+	s := BuddyIconSource{BARTService: bartService, Logger: slog.Default()}
+	_, err := s.ImageForHash(context.Background(), state.NewIdentScreenName("mikekelly"), []byte{0x01})
+
+	assert.ErrorIs(t, err, ErrNoBuddyIcon)
+}
+
+func TestBuddyIconSource_PublishedURL(t *testing.T) {
+	tests := []struct {
+		name    string
+		baseURL string
+		id      *wire.BARTID
+		idErr   error
+		want    string
+	}{
+		{
+			name:    "an icon yields a content-addressed URL",
+			baseURL: "http://api.example.com",
+			id:      bartID([]byte{0xde, 0xad, 0xbe, 0xef}),
+			want:    "http://api.example.com/expressions/get?t=mikekelly&type=buddyIcon&bartId=deadbeef",
+		},
+		{
+			name:    "no icon still yields a hash-less placeholder URL",
+			baseURL: "http://api.example.com",
+			id:      nil,
+			want:    "http://api.example.com/expressions/get?t=mikekelly&type=buddyIcon",
+		},
+		{
+			name:    "a cleared icon yields the placeholder URL",
+			baseURL: "http://api.example.com",
+			id:      bartID(wire.GetClearIconHash()),
+			want:    "http://api.example.com/expressions/get?t=mikekelly&type=buddyIcon",
+		},
+		{
+			name:    "a lookup failure yields no URL",
+			baseURL: "http://api.example.com",
+			idErr:   errors.New("db exploded"),
+			want:    "",
+		},
+		{
+			name:    "no base URL yields no URL",
+			baseURL: "",
+			id:      bartID([]byte{0x01}),
+			want:    "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			iconRetriever := &MockBuddyIconRetriever{}
+			iconRetriever.On("BuddyIconMetadata", mock.Anything, state.NewIdentScreenName("mikekelly")).
+				Return(tt.id, tt.idErr).Maybe()
+
+			s := BuddyIconSource{IconRetriever: iconRetriever, Logger: slog.Default()}
+			got := s.PublishedURL(context.Background(), tt.baseURL, state.NewIdentScreenName("mikekelly"))
+
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
+func TestBuddyIconSource_Image_NoIcon(t *testing.T) {
+	iconRetriever := &MockBuddyIconRetriever{}
+	iconRetriever.On("BuddyIconMetadata", mock.Anything, mock.Anything).Return(nil, nil).Once()
+
+	bartService := &MockBARTService{}
+
+	s := BuddyIconSource{IconRetriever: iconRetriever, BARTService: bartService, Logger: slog.Default()}
+	_, err := s.Image(context.Background(), state.NewIdentScreenName("mikekelly"))
+
+	assert.ErrorIs(t, err, ErrNoBuddyIcon)
+	// No icon means there is nothing to ask BART for.
+	bartService.AssertNotCalled(t, "RetrieveItem", mock.Anything, mock.Anything, mock.Anything)
+}
+
+func TestBuddyIconSource_URLForHash(t *testing.T) {
+	// URLForHash never touches the retriever: the hash is supplied by the caller.
+	s := BuddyIconSource{Logger: slog.Default()}
+	sn := state.NewIdentScreenName("Mike Kelly")
+
+	t.Run("hash yields the content-addressed URL", func(t *testing.T) {
+		assert.Equal(t,
+			"http://api.example.com/expressions/get?t=mikekelly&type=buddyIcon&bartId=deadbeef",
+			s.URLForHash("http://api.example.com", sn, []byte{0xde, 0xad, 0xbe, 0xef}))
+	})
+
+	t.Run("no hash yields the placeholder URL", func(t *testing.T) {
+		assert.Equal(t,
+			"http://api.example.com/expressions/get?t=mikekelly&type=buddyIcon",
+			s.URLForHash("http://api.example.com", sn, nil))
+	})
+
+	t.Run("empty baseURL opts out", func(t *testing.T) {
+		assert.Empty(t, s.URLForHash("", sn, []byte{0x01}))
+	})
+}
+
+func TestBuddyIconSource_Image_RetrieveFails(t *testing.T) {
+	iconRetriever := &MockBuddyIconRetriever{}
+	iconRetriever.On("BuddyIconMetadata", mock.Anything, mock.Anything).
+		Return(bartID([]byte{0x01}), nil).Once()
+
+	bartService := &MockBARTService{}
+	bartService.On("RetrieveItem", mock.Anything, mock.Anything, mock.Anything).
+		Return(wire.SNACMessage{}, errors.New("item missing")).Once()
+
+	s := BuddyIconSource{IconRetriever: iconRetriever, BARTService: bartService, Logger: slog.Default()}
+	_, err := s.Image(context.Background(), state.NewIdentScreenName("mikekelly"))
+
+	assert.ErrorContains(t, err, "item missing")
+	assert.NotErrorIs(t, err, ErrNoBuddyIcon)
+}

+ 10 - 3
server/webapi/handlers/buddy_list_manager.go

@@ -18,14 +18,16 @@ import (
 type BuddyListManager struct {
 	feedbagService FeedbagService
 	locateService  LocateService
+	iconSource     BuddyIconSource
 	logger         *slog.Logger
 }
 
 // NewBuddyListManager creates a new instance of the buddy list manager.
-func NewBuddyListManager(feedbagService FeedbagService, locateService LocateService, logger *slog.Logger) *BuddyListManager {
+func NewBuddyListManager(feedbagService FeedbagService, locateService LocateService, iconSource BuddyIconSource, logger *slog.Logger) *BuddyListManager {
 	return &BuddyListManager{
 		feedbagService: feedbagService,
 		locateService:  locateService,
+		iconSource:     iconSource,
 		logger:         logger,
 	}
 }
@@ -154,7 +156,7 @@ func (m *BuddyListManager) GetBuddyListForUser(ctx context.Context, sess *state.
 			if !ok {
 				continue
 			}
-			info := m.getBuddyInfo(ctx, sess.OSCARSession, b.name)
+			info := m.getBuddyInfo(ctx, sess.OSCARSession, sess.BaseURL, b.name)
 			// The alias belongs in friendly, not displayId: the client renders
 			// friendly in preference to displayId but still shows displayId as
 			// the buddy's actual screen name.
@@ -169,7 +171,7 @@ func (m *BuddyListManager) GetBuddyListForUser(ctx context.Context, sess *state.
 
 // getBuddyInfo retrieves a buddy's current presence by issuing a locate
 // UserInfoQuery on behalf of the requesting session's OSCAR instance.
-func (m *BuddyListManager) getBuddyInfo(ctx context.Context, instance *state.SessionInstance, buddyName string) WebAPIBuddyInfo {
+func (m *BuddyListManager) getBuddyInfo(ctx context.Context, instance *state.SessionInstance, baseURL string, buddyName string) WebAPIBuddyInfo {
 	// Default to offline. The web client keys users by the normalized aimId and
 	// shallow-merges each buddy map onto the shared user object, so a display-form
 	// aimId here overwrites the id every other event is keyed by.
@@ -206,6 +208,11 @@ func (m *BuddyListManager) getBuddyInfo(ctx context.Context, instance *state.Ses
 	info.State = "online"
 	info.Capabilities = []string{}
 
+	// Publish the icon only now that locate has confirmed the buddy is online and
+	// has not blocked the caller. Offline and blocking buddies return above without
+	// an icon, so neither their icon nor its activity-revealing hash leaks.
+	info.BuddyIcon = m.iconSource.PublishedURL(ctx, baseURL, ident)
+
 	// The locate reply carries the screen name as the buddy formatted it.
 	if userInfo.ScreenName != "" {
 		info.DisplayID = userInfo.ScreenName

+ 83 - 3
server/webapi/handlers/buddy_list_manager_test.go

@@ -273,7 +273,7 @@ func TestBuddyListManager_GetBuddyListForUser(t *testing.T) {
 				).Once()
 			}
 
-			m := NewBuddyListManager(fs, ls, slog.Default())
+			m := NewBuddyListManager(fs, ls, newTestIconSource(), slog.Default())
 			sess := &state.WebAPISession{
 				ScreenName:   state.DisplayScreenName(owner.String()),
 				OSCARSession: state.NewSession().AddInstance(),
@@ -321,7 +321,7 @@ func TestBuddyListManager_GetBuddyListForUser_DisplayIDFromLocateReply(t *testin
 		}}, nil,
 	).Once()
 
-	m := NewBuddyListManager(fs, ls, slog.Default())
+	m := NewBuddyListManager(fs, ls, newTestIconSource(), slog.Default())
 	sess := &state.WebAPISession{
 		ScreenName:   state.DisplayScreenName("listowner"),
 		OSCARSession: state.NewSession().AddInstance(),
@@ -339,6 +339,86 @@ func TestBuddyListManager_GetBuddyListForUser_DisplayIDFromLocateReply(t *testin
 	ls.AssertExpectations(t)
 }
 
+// Icons are published only for online, non-blocking buddies: an online buddy with
+// an icon gets a content-addressed URL, an online buddy without one gets the
+// hash-less placeholder URL, and an offline (or blocking) buddy gets no icon and
+// is never even looked up, so neither their icon nor its hash leaks.
+func TestBuddyListManager_GetBuddyListForUser_PublishesBuddyIcons(t *testing.T) {
+	ctx := context.Background()
+
+	fb := []wire.FeedbagItem{
+		{Name: "", GroupID: 0, ItemID: 0, ClassID: wire.FeedbagClassIdGroup,
+			TLVLBlock: wire.TLVLBlock{TLVList: wire.TLVList{wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{100})}}},
+		{Name: "Buddies", GroupID: 100, ItemID: 0, ClassID: wire.FeedbagClassIdGroup,
+			TLVLBlock: wire.TLVLBlock{TLVList: wire.TLVList{wire.NewTLVBE(wire.FeedbagAttributesOrder, []uint16{1, 2, 3})}}},
+		{ItemID: 1, ClassID: wire.FeedbagClassIdBuddy, GroupID: 100, Name: "onlineicon"},
+		{ItemID: 2, ClassID: wire.FeedbagClassIdBuddy, GroupID: 100, Name: "offlinebud"},
+		{ItemID: 3, ClassID: wire.FeedbagClassIdBuddy, GroupID: 100, Name: "onlinenoicon"},
+	}
+
+	fs := &MockFeedbagService{}
+	fs.On("Query", mock.Anything, mock.Anything, mock.Anything).Return(
+		wire.SNACMessage{Body: wire.SNAC_0x13_0x06_FeedbagReply{Items: fb}}, nil,
+	).Once()
+
+	online := func(name string) wire.SNACMessage {
+		return wire.SNACMessage{Body: wire.SNAC_0x02_0x06_LocateUserInfoReply{
+			TLVUserInfo: wire.TLVUserInfo{ScreenName: name},
+		}}
+	}
+	locateFor := func(name string) any {
+		return mock.MatchedBy(func(q wire.SNAC_0x02_0x05_LocateUserInfoQuery) bool { return q.ScreenName == name })
+	}
+
+	ls := &MockLocateService{}
+	ls.On("UserInfoQuery", mock.Anything, mock.Anything, mock.Anything, locateFor("onlineicon")).
+		Return(online("onlineicon"), nil).Once()
+	ls.On("UserInfoQuery", mock.Anything, mock.Anything, mock.Anything, locateFor("onlinenoicon")).
+		Return(online("onlinenoicon"), nil).Once()
+	ls.On("UserInfoQuery", mock.Anything, mock.Anything, mock.Anything, locateFor("offlinebud")).
+		Return(wire.SNACMessage{}, errors.New("offline")).Once()
+
+	iconRetriever := &MockBuddyIconRetriever{}
+	iconRetriever.On("BuddyIconMetadata", mock.Anything, state.NewIdentScreenName("onlineicon")).
+		Return(bartID([]byte{0xab, 0xcd}), nil).Once()
+	iconRetriever.On("BuddyIconMetadata", mock.Anything, state.NewIdentScreenName("onlinenoicon")).
+		Return(nil, nil).Once()
+
+	m := NewBuddyListManager(fs, ls, BuddyIconSource{
+		IconRetriever: iconRetriever,
+		Logger:        slog.Default(),
+	}, slog.Default())
+
+	sess := &state.WebAPISession{
+		ScreenName:   state.DisplayScreenName("listowner"),
+		OSCARSession: state.NewSession().AddInstance(),
+		BaseURL:      "http://api.example.com",
+	}
+	got, err := m.GetBuddyListForUser(ctx, sess)
+	require.NoError(t, err)
+	require.Len(t, got, 1)
+	require.Len(t, got[0].Buddies, 3)
+
+	// onlineicon: content-addressed URL carrying the icon hash.
+	assert.Equal(t, "online", got[0].Buddies[0].State)
+	assert.Equal(t,
+		"http://api.example.com/expressions/get?t=onlineicon&type=buddyIcon&bartId=abcd",
+		got[0].Buddies[0].BuddyIcon)
+
+	// offlinebud: no icon, and its metadata is never queried.
+	assert.Equal(t, "offline", got[0].Buddies[1].State)
+	assert.Empty(t, got[0].Buddies[1].BuddyIcon)
+	iconRetriever.AssertNotCalled(t, "BuddyIconMetadata", mock.Anything, state.NewIdentScreenName("offlinebud"))
+
+	// onlinenoicon: hash-less placeholder URL so a cleared icon still propagates.
+	assert.Equal(t, "online", got[0].Buddies[2].State)
+	assert.Equal(t,
+		"http://api.example.com/expressions/get?t=onlinenoicon&type=buddyIcon",
+		got[0].Buddies[2].BuddyIcon)
+
+	iconRetriever.AssertExpectations(t)
+}
+
 // The feedbag service relays a session's own writes only to the owner's other
 // instances, so renaming a buddy from the web client produces no SNAC for that
 // session. Without an explicit invalidation, its cached aliases would keep serving
@@ -369,7 +449,7 @@ func TestBuddyListManager_SetBuddyAttributeInFeedbag_InvalidatesAliasCache(t *te
 	fs.On("Query", mock.Anything, mock.Anything, mock.Anything).
 		Return(wire.SNACMessage{Body: wire.SNAC_0x13_0x06_FeedbagReply{Items: feedbag("MIKE")}}, nil).Once()
 
-	m := NewBuddyListManager(fs, &MockLocateService{}, slog.Default())
+	m := NewBuddyListManager(fs, &MockLocateService{}, newTestIconSource(), slog.Default())
 	sess := &state.WebAPISession{
 		ScreenName:   state.DisplayScreenName("listowner"),
 		OSCARSession: state.NewSession().AddInstance(),

+ 8 - 8
server/webapi/handlers/buddylist_test.go

@@ -377,7 +377,7 @@ func TestBuddyListHandler_AddBuddy(t *testing.T) {
 			sessionManager := &MockWebAPISessionManager{}
 			feedbagService := &MockFeedbagService{}
 			blmFeedbagService := &MockFeedbagService{}
-			blm := NewBuddyListManager(blmFeedbagService, &MockLocateService{}, slog.Default())
+			blm := NewBuddyListManager(blmFeedbagService, &MockLocateService{}, newTestIconSource(), slog.Default())
 			logger := slog.Default()
 
 			handler := &BuddyListHandler{
@@ -514,7 +514,7 @@ func TestBuddyListHandler_AddGroup(t *testing.T) {
 			sm := &MockWebAPISessionManager{}
 			fs := &MockFeedbagService{}
 			blmFs := &MockFeedbagService{}
-			blm := NewBuddyListManager(blmFs, &MockLocateService{}, slog.Default())
+			blm := NewBuddyListManager(blmFs, &MockLocateService{}, newTestIconSource(), slog.Default())
 
 			aimsid := ""
 			if v := tt.queryParams["aimsid"]; len(v) > 0 {
@@ -550,7 +550,7 @@ func TestBuddyListHandler_RemoveBuddy(t *testing.T) {
 	type setupFunc func(*MockWebAPISessionManager, *BuddyListManager, *MockFeedbagService, string) *state.WebAPISession
 
 	newBuddyListManager := func(fs *MockFeedbagService) *BuddyListManager {
-		return NewBuddyListManager(fs, &MockLocateService{}, slog.Default())
+		return NewBuddyListManager(fs, &MockLocateService{}, newTestIconSource(), slog.Default())
 	}
 
 	tests := []struct {
@@ -705,7 +705,7 @@ func TestBuddyListHandler_RemoveGroup(t *testing.T) {
 	type setupFunc func(*MockWebAPISessionManager, *BuddyListManager, *MockFeedbagService, string) *state.WebAPISession
 
 	newBuddyListManager := func(fs *MockFeedbagService) *BuddyListManager {
-		return NewBuddyListManager(fs, &MockLocateService{}, slog.Default())
+		return NewBuddyListManager(fs, &MockLocateService{}, newTestIconSource(), slog.Default())
 	}
 
 	tests := []struct {
@@ -967,7 +967,7 @@ func TestRequireSession(t *testing.T) {
 
 func TestBuddyListHandler_RenameGroup(t *testing.T) {
 	newBLM := func(fs *MockFeedbagService) *BuddyListManager {
-		return NewBuddyListManager(fs, &MockLocateService{}, slog.Default())
+		return NewBuddyListManager(fs, &MockLocateService{}, newTestIconSource(), slog.Default())
 	}
 	sessWithOSCAR := func(aimsid string) *state.WebAPISession {
 		return &state.WebAPISession{
@@ -1052,7 +1052,7 @@ func TestBuddyListHandler_RenameGroup(t *testing.T) {
 
 func TestBuddyListHandler_MoveBuddy(t *testing.T) {
 	newBLM := func(fs *MockFeedbagService) *BuddyListManager {
-		return NewBuddyListManager(fs, &MockLocateService{}, slog.Default())
+		return NewBuddyListManager(fs, &MockLocateService{}, newTestIconSource(), slog.Default())
 	}
 	sessWithOSCAR := func(aimsid string) *state.WebAPISession {
 		return &state.WebAPISession{
@@ -1144,7 +1144,7 @@ func TestBuddyListHandler_MoveBuddy(t *testing.T) {
 
 func TestBuddyListHandler_SetBuddyAttribute(t *testing.T) {
 	newBLM := func(fs *MockFeedbagService) *BuddyListManager {
-		return NewBuddyListManager(fs, &MockLocateService{}, slog.Default())
+		return NewBuddyListManager(fs, &MockLocateService{}, newTestIconSource(), slog.Default())
 	}
 	sessWithOSCAR := func(aimsid string) *state.WebAPISession {
 		return &state.WebAPISession{
@@ -1229,7 +1229,7 @@ func TestBuddyListHandler_SetBuddyAttribute(t *testing.T) {
 
 func TestBuddyListHandler_SetGroupAttribute(t *testing.T) {
 	newBLM := func(fs *MockFeedbagService) *BuddyListManager {
-		return NewBuddyListManager(fs, &MockLocateService{}, slog.Default())
+		return NewBuddyListManager(fs, &MockLocateService{}, newTestIconSource(), slog.Default())
 	}
 	sessWithOSCAR := func(aimsid string) *state.WebAPISession {
 		return &state.WebAPISession{

+ 107 - 27
server/webapi/handlers/expressions.go

@@ -1,53 +1,133 @@
 package handlers
 
 import (
+	"encoding/hex"
+	"errors"
 	"log/slog"
 	"net/http"
+
+	"github.com/mk6i/open-oscar-server/state"
+	"github.com/mk6i/open-oscar-server/wire"
 )
 
 // ExpressionsHandler handles Web AIM API expressions/buddy icon endpoints.
 type ExpressionsHandler struct {
-	Logger *slog.Logger
+	IconSource BuddyIconSource
+	Logger     *slog.Logger
 }
 
 // NewExpressionsHandler creates a new ExpressionsHandler.
-func NewExpressionsHandler(logger *slog.Logger) *ExpressionsHandler {
+func NewExpressionsHandler(iconSource BuddyIconSource, logger *slog.Logger) *ExpressionsHandler {
 	return &ExpressionsHandler{
-		Logger: logger,
+		IconSource: iconSource,
+		Logger:     logger,
 	}
 }
 
 // Get handles GET /expressions/get requests for buddy icons and expressions.
+//
+// The AIM client calls this endpoint two different ways:
+//
+//   - With type=buddyIcon it fetches the image itself. The buddyIcon URL
+//     published in presence, buddylist and myInfo payloads points here, and the
+//     client renders it directly as an <img> source.
+//   - With no type it asks for the user's expressions as JSON, then scans the
+//     returned array for the entry typed bigBuddyIcon and uses its url to render
+//     hovercards and other large views. We have only one icon per user, so it is
+//     offered as both.
 func (h *ExpressionsHandler) Get(w http.ResponseWriter, r *http.Request) {
-	// Parse parameters
-	format := r.URL.Query().Get("f")
-	targetUser := r.URL.Query().Get("t")
-	expressionType := r.URL.Query().Get("type")
-
-	h.Logger.Debug("expressions/get request",
-		"format", format,
-		"target", targetUser,
-		"type", expressionType,
+	ctx := r.Context()
+
+	target := r.URL.Query().Get("t")
+	if target == "" {
+		SendError(w, http.StatusBadRequest, "missing target")
+		return
+	}
+	screenName := state.NewIdentScreenName(target)
+
+	switch r.URL.Query().Get("type") {
+	case "buddyIcon", "bigBuddyIcon":
+		h.serveIcon(w, r, screenName)
+		return
+	}
+
+	iconURL := h.IconSource.URL(ctx, baseURLFromRequest(r), screenName)
+
+	// f=redirect asks for the icon itself rather than a description of it.
+	if r.URL.Query().Get("f") == "redirect" {
+		if iconURL == "" {
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		http.Redirect(w, r, iconURL, http.StatusFound)
+		return
+	}
+
+	expressions := []any{}
+	if iconURL != "" {
+		expressions = append(expressions, map[string]any{
+			"type": "bigBuddyIcon",
+			"url":  iconURL,
+		})
+	}
+
+	resp := BaseResponse{}
+	resp.Response.StatusCode = 200
+	resp.Response.StatusText = "OK"
+	resp.Response.Data = map[string]any{"expressions": expressions}
+	SendResponse(w, r, resp, h.Logger)
+}
+
+// serveIcon writes a user's buddy icon image.
+//
+// A bartId names an exact image by content hash, so the endpoint serves that
+// image regardless of the user's current icon and lets browsers cache it
+// forever. Without a bartId it serves whatever the user's icon is now — which
+// keeps changing — so that response is not cacheable, and a user with no icon
+// gets the blank placeholder so the client's <img> still renders.
+func (h *ExpressionsHandler) serveIcon(w http.ResponseWriter, r *http.Request, screenName state.IdentScreenName) {
+	ctx := r.Context()
+
+	var (
+		icon      []byte
+		err       error
+		immutable bool
 	)
+	if raw := r.URL.Query().Get("bartId"); raw != "" {
+		hash, decodeErr := hex.DecodeString(raw)
+		if decodeErr != nil || len(hash) == 0 {
+			http.Error(w, "invalid bartId", http.StatusBadRequest)
+			return
+		}
+		icon, err = h.IconSource.ImageForHash(ctx, screenName, hash)
+		immutable = true
+	} else {
+		icon, err = h.IconSource.Image(ctx, screenName)
+		if errors.Is(err, ErrNoBuddyIcon) {
+			// Serve the blank placeholder rather than 404 so a cleared icon still
+			// renders something and the client stops showing the previous one.
+			icon, err = h.IconSource.ImageForHash(ctx, screenName, wire.GetClearIconHash())
+		}
+	}
 
-	// For redirect format, return a 404 (no buddy icon)
-	// In a real implementation, this would redirect to the actual icon URL
-	if format == "redirect" {
-		w.WriteHeader(http.StatusNotFound)
+	switch {
+	case errors.Is(err, ErrNoBuddyIcon):
+		// The client swaps in its own placeholder when the icon fails to load.
+		http.Error(w, "icon not found", http.StatusNotFound)
+		return
+	case err != nil:
+		h.Logger.ErrorContext(ctx, "failed to retrieve buddy icon",
+			"screenName", screenName.String(), "err", err.Error())
+		http.Error(w, "internal server error", http.StatusInternalServerError)
 		return
 	}
 
-	// For other formats, return an empty response indicating no expressions
-	response := map[string]interface{}{
-		"response": map[string]interface{}{
-			"statusCode": 200,
-			"statusText": "OK",
-			"data": map[string]interface{}{
-				"expressions": []interface{}{},
-			},
-		},
+	if immutable {
+		w.Header().Set("Cache-Control", "public, max-age=31536000, immutable")
+	} else {
+		w.Header().Set("Cache-Control", "no-cache")
 	}
 
-	// Send response in requested format
-	SendResponse(w, r, response, h.Logger)
+	w.Header().Set("Content-Type", http.DetectContentType(icon))
+	_, _ = w.Write(icon)
 }

+ 214 - 0
server/webapi/handlers/expressions_test.go

@@ -0,0 +1,214 @@
+package handlers
+
+import (
+	"bytes"
+	"encoding/json"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+
+	"github.com/mk6i/open-oscar-server/wire"
+)
+
+// blankIconGIF stands in for the blank placeholder the real BART service returns
+// for the clear-icon hash; distinct from iconGIF so tests can tell them apart.
+var blankIconGIF = []byte{0x47, 0x49, 0x46, 0x38, 0x39, 0x61, 0xff}
+
+// newExpressionsHandler builds a handler whose target user has the given icon,
+// or no icon when id is nil. Its BART mock mirrors the real service: the
+// clear-icon hash resolves to the blank placeholder, any other hash to iconGIF.
+func newExpressionsHandler(id *wire.BARTID) *ExpressionsHandler {
+	iconRetriever := &MockBuddyIconRetriever{}
+	iconRetriever.On("BuddyIconMetadata", mock.Anything, mock.Anything).Return(id, nil).Maybe()
+
+	bartService := &MockBARTService{}
+	bartService.On("RetrieveItem", mock.Anything, mock.Anything,
+		mock.MatchedBy(func(q wire.SNAC_0x10_0x04_BARTDownloadQuery) bool { return q.HasClearIconHash() })).
+		Return(wire.SNACMessage{Body: wire.SNAC_0x10_0x05_BARTDownloadReply{Data: blankIconGIF}}, nil).Maybe()
+	bartService.On("RetrieveItem", mock.Anything, mock.Anything, mock.Anything).
+		Return(wire.SNACMessage{Body: wire.SNAC_0x10_0x05_BARTDownloadReply{Data: iconGIF}}, nil).Maybe()
+
+	return NewExpressionsHandler(BuddyIconSource{
+		IconRetriever: iconRetriever,
+		BARTService:   bartService,
+		Logger:        slog.Default(),
+	}, slog.Default())
+}
+
+func TestExpressionsHandler_Get_ServesIconBytes(t *testing.T) {
+	h := newExpressionsHandler(bartID([]byte{0xde, 0xad}))
+
+	r := httptest.NewRequest(http.MethodGet, "/expressions/get?t=mikekelly&type=buddyIcon&bartId=dead", nil)
+	w := httptest.NewRecorder()
+	h.Get(w, r)
+
+	assert.Equal(t, http.StatusOK, w.Code)
+	assert.Equal(t, iconGIF, w.Body.Bytes())
+	assert.Equal(t, "image/gif", w.Header().Get("Content-Type"))
+	// The URL pins the icon hash, so it always resolves to the same image.
+	assert.Equal(t, "public, max-age=31536000, immutable", w.Header().Get("Cache-Control"))
+}
+
+func TestExpressionsHandler_Get_IconWithoutHashIsNotCached(t *testing.T) {
+	// Without a hash the URL keeps resolving to whatever the current icon is, so
+	// caching it would pin a stale image.
+	h := newExpressionsHandler(bartID([]byte{0xde, 0xad}))
+
+	r := httptest.NewRequest(http.MethodGet, "/expressions/get?t=mikekelly&type=buddyIcon", nil)
+	w := httptest.NewRecorder()
+	h.Get(w, r)
+
+	assert.Equal(t, http.StatusOK, w.Code)
+	assert.Equal(t, "no-cache", w.Header().Get("Cache-Control"))
+}
+
+func TestExpressionsHandler_Get_MissingIconServesPlaceholder(t *testing.T) {
+	// A user with no icon still serves the blank placeholder for the hash-less
+	// URL, so the client's <img> renders something and a cleared icon stops
+	// showing the previous one rather than 404ing.
+	h := newExpressionsHandler(nil)
+
+	r := httptest.NewRequest(http.MethodGet, "/expressions/get?t=mikekelly&type=buddyIcon", nil)
+	w := httptest.NewRecorder()
+	h.Get(w, r)
+
+	assert.Equal(t, http.StatusOK, w.Code)
+	assert.Equal(t, blankIconGIF, w.Body.Bytes())
+	assert.Equal(t, "no-cache", w.Header().Get("Cache-Control"))
+}
+
+func TestExpressionsHandler_Get_BartIdServesRequestedHash(t *testing.T) {
+	// Even though the user's *current* icon is hash B, a URL pinned to hash A must
+	// serve A's bytes and cache immutably — otherwise a cached URL could later
+	// resolve to a different image.
+	iconRetriever := &MockBuddyIconRetriever{}
+	iconRetriever.On("BuddyIconMetadata", mock.Anything, mock.Anything).
+		Return(bartID([]byte{0xbb}), nil).Maybe()
+
+	bytesA := []byte{0x47, 0x49, 0x46, 0x38, 0x39, 0x61, 0xa1}
+	bartService := &MockBARTService{}
+	bartService.On("RetrieveItem", mock.Anything, mock.Anything,
+		mock.MatchedBy(func(q wire.SNAC_0x10_0x04_BARTDownloadQuery) bool {
+			return bytes.Equal(q.Hash, []byte{0xaa})
+		})).
+		Return(wire.SNACMessage{Body: wire.SNAC_0x10_0x05_BARTDownloadReply{Data: bytesA}}, nil).Once()
+
+	h := NewExpressionsHandler(BuddyIconSource{
+		IconRetriever: iconRetriever,
+		BARTService:   bartService,
+		Logger:        slog.Default(),
+	}, slog.Default())
+
+	r := httptest.NewRequest(http.MethodGet, "/expressions/get?t=mikekelly&type=buddyIcon&bartId=aa", nil)
+	w := httptest.NewRecorder()
+	h.Get(w, r)
+
+	assert.Equal(t, http.StatusOK, w.Code)
+	assert.Equal(t, bytesA, w.Body.Bytes())
+	assert.Equal(t, "public, max-age=31536000, immutable", w.Header().Get("Cache-Control"))
+	bartService.AssertExpectations(t)
+}
+
+func TestExpressionsHandler_Get_UnknownBartIdNotFound(t *testing.T) {
+	iconRetriever := &MockBuddyIconRetriever{}
+	iconRetriever.On("BuddyIconMetadata", mock.Anything, mock.Anything).
+		Return(bartID([]byte{0xbb}), nil).Maybe()
+
+	// An empty reply means the hash is not stored.
+	bartService := &MockBARTService{}
+	bartService.On("RetrieveItem", mock.Anything, mock.Anything, mock.Anything).
+		Return(wire.SNACMessage{Body: wire.SNAC_0x10_0x05_BARTDownloadReply{}}, nil).Once()
+
+	h := NewExpressionsHandler(BuddyIconSource{
+		IconRetriever: iconRetriever,
+		BARTService:   bartService,
+		Logger:        slog.Default(),
+	}, slog.Default())
+
+	r := httptest.NewRequest(http.MethodGet, "/expressions/get?t=mikekelly&type=buddyIcon&bartId=abcdef", nil)
+	w := httptest.NewRecorder()
+	h.Get(w, r)
+
+	assert.Equal(t, http.StatusNotFound, w.Code)
+}
+
+func TestExpressionsHandler_Get_ListsBigBuddyIcon(t *testing.T) {
+	// This is the shape the client scans for its large icon rendering.
+	h := newExpressionsHandler(bartID([]byte{0xde, 0xad}))
+
+	r := httptest.NewRequest(http.MethodGet, "/expressions/get?t=mikekelly", nil)
+	r.Host = "api.example.com"
+	w := httptest.NewRecorder()
+	h.Get(w, r)
+
+	assert.Equal(t, http.StatusOK, w.Code)
+
+	var got struct {
+		Response struct {
+			StatusCode int `json:"statusCode"`
+			Data       struct {
+				Expressions []struct {
+					Type string `json:"type"`
+					URL  string `json:"url"`
+				} `json:"expressions"`
+			} `json:"data"`
+		} `json:"response"`
+	}
+	assert.NoError(t, json.Unmarshal(w.Body.Bytes(), &got))
+
+	assert.Equal(t, 200, got.Response.StatusCode)
+	assert.Len(t, got.Response.Data.Expressions, 1)
+	assert.Equal(t, "bigBuddyIcon", got.Response.Data.Expressions[0].Type)
+	assert.Equal(t,
+		"http://api.example.com/expressions/get?t=mikekelly&type=buddyIcon&bartId=dead",
+		got.Response.Data.Expressions[0].URL)
+}
+
+func TestExpressionsHandler_Get_ListsNothingWithoutIcon(t *testing.T) {
+	h := newExpressionsHandler(nil)
+
+	r := httptest.NewRequest(http.MethodGet, "/expressions/get?t=mikekelly", nil)
+	w := httptest.NewRecorder()
+	h.Get(w, r)
+
+	assert.Equal(t, http.StatusOK, w.Code)
+	assert.Contains(t, w.Body.String(), `"expressions":[]`)
+}
+
+func TestExpressionsHandler_Get_Redirect(t *testing.T) {
+	h := newExpressionsHandler(bartID([]byte{0xde, 0xad}))
+
+	r := httptest.NewRequest(http.MethodGet, "/expressions/get?t=mikekelly&f=redirect", nil)
+	r.Host = "api.example.com"
+	w := httptest.NewRecorder()
+	h.Get(w, r)
+
+	assert.Equal(t, http.StatusFound, w.Code)
+	assert.Equal(t,
+		"http://api.example.com/expressions/get?t=mikekelly&type=buddyIcon&bartId=dead",
+		w.Header().Get("Location"))
+}
+
+func TestExpressionsHandler_Get_RedirectWithoutIcon(t *testing.T) {
+	h := newExpressionsHandler(nil)
+
+	r := httptest.NewRequest(http.MethodGet, "/expressions/get?t=mikekelly&f=redirect", nil)
+	w := httptest.NewRecorder()
+	h.Get(w, r)
+
+	assert.Equal(t, http.StatusNotFound, w.Code)
+}
+
+func TestExpressionsHandler_Get_MissingTarget(t *testing.T) {
+	h := newExpressionsHandler(nil)
+
+	r := httptest.NewRequest(http.MethodGet, "/expressions/get", nil)
+	w := httptest.NewRecorder()
+	h.Get(w, r)
+
+	assert.Equal(t, http.StatusBadRequest, w.Code)
+}

+ 2 - 1
server/webapi/handlers/messaging_test.go

@@ -58,6 +58,7 @@ func createTestSessionManagerWithOSCAR(screenName string, oscarSession *state.Se
 		"test-dev",
 		[]string{"im", "presence", "buddylist", "sentIM", "typing"},
 		oscarSession,
+		"",
 		slog.Default(),
 	)
 	return mgr, session.AimSID
@@ -105,7 +106,7 @@ func sendIMForDest(t *testing.T, dest, locateName, alias string) []types.Event {
 
 	mgr := state.NewWebAPISessionManager()
 	session, err := mgr.CreateSession(context.Background(), state.DisplayScreenName("Ann Dupree"),
-		"test-dev", []string{"im", "sentIM", "conversation"}, oscarInstance, slog.Default())
+		"test-dev", []string{"im", "sentIM", "conversation"}, oscarInstance, "", slog.Default())
 	require.NoError(t, err)
 
 	handler := &MessagingHandler{

+ 34 - 0
server/webapi/handlers/mocks_test.go

@@ -2,6 +2,7 @@ package handlers
 
 import (
 	"context"
+	"log/slog"
 
 	"github.com/stretchr/testify/mock"
 
@@ -23,3 +24,36 @@ func (m *MockLocateService) UserInfoQuery(ctx context.Context, instance *state.S
 	args := m.Called(ctx, instance, inFrame, inBody)
 	return args.Get(0).(wire.SNACMessage), args.Error(1)
 }
+
+// MockBuddyIconRetriever is a mock implementation of BuddyIconRetriever
+type MockBuddyIconRetriever struct {
+	mock.Mock
+}
+
+func (m *MockBuddyIconRetriever) BuddyIconMetadata(ctx context.Context, screenName state.IdentScreenName) (*wire.BARTID, error) {
+	args := m.Called(ctx, screenName)
+	id, _ := args.Get(0).(*wire.BARTID)
+	return id, args.Error(1)
+}
+
+// MockBARTService is a mock implementation of BARTService
+type MockBARTService struct {
+	mock.Mock
+}
+
+func (m *MockBARTService) RetrieveItem(ctx context.Context, inFrame wire.SNACFrame, inBody wire.SNAC_0x10_0x04_BARTDownloadQuery) (wire.SNACMessage, error) {
+	args := m.Called(ctx, inFrame, inBody)
+	return args.Get(0).(wire.SNACMessage), args.Error(1)
+}
+
+// newTestIconSource returns a BuddyIconSource whose users have no buddy icon,
+// for tests that are not exercising icons.
+func newTestIconSource() BuddyIconSource {
+	iconRetriever := &MockBuddyIconRetriever{}
+	iconRetriever.On("BuddyIconMetadata", mock.Anything, mock.Anything).Return(nil, nil).Maybe()
+	return BuddyIconSource{
+		IconRetriever: iconRetriever,
+		BARTService:   &MockBARTService{},
+		Logger:        slog.Default(),
+	}
+}

+ 18 - 12
server/webapi/handlers/presence.go

@@ -19,6 +19,7 @@ type PresenceHandler struct {
 	FeedbagService   FeedbagService
 	BuddyBroadcaster BuddyBroadcaster
 	LocateService    LocateService
+	IconSource       BuddyIconSource
 	Logger           *slog.Logger
 }
 
@@ -68,6 +69,7 @@ type BuddyPresenceInfo struct {
 	IdleTime   int    `json:"idleTime,omitempty" xml:"idleTime,omitempty"`
 	OnlineTime int64  `json:"onlineTime,omitempty" xml:"onlineTime,omitempty"`
 	UserType   string `json:"userType" xml:"userType"` // "aim", "icq", "admin"
+	BuddyIcon  string `json:"buddyIcon,omitempty" xml:"buddyIcon,omitempty"`
 }
 
 // GetPresence handles GET /presence/get requests.
@@ -117,7 +119,7 @@ func (h *PresenceHandler) GetPresence(w http.ResponseWriter, r *http.Request, se
 			if user == "" {
 				continue
 			}
-			info := h.getUserPresence(ctx, session.OSCARSession, state.DisplayScreenName(user), wantProfileMsg)
+			info := h.getUserPresence(ctx, session.OSCARSession, session.BaseURL, state.DisplayScreenName(user), wantProfileMsg)
 			info.Friendly = aliases[info.AimID]
 			presenceList = append(presenceList, info)
 		}
@@ -192,7 +194,7 @@ func (h *PresenceHandler) getBuddyListGroups(ctx context.Context, session *state
 
 		// UserInfoQuery performs the blocking check and online lookup; blocked or
 		// offline buddies come back as "offline", preserving the list structure.
-		presence := h.getUserPresence(ctx, session.OSCARSession, state.DisplayScreenName(item.Name), wantProfileMsg)
+		presence := h.getUserPresence(ctx, session.OSCARSession, session.BaseURL, state.DisplayScreenName(item.Name), wantProfileMsg)
 		group.Buddies = append(group.Buddies, presence)
 	}
 
@@ -217,7 +219,7 @@ func (h *PresenceHandler) getBuddyListGroups(ctx context.Context, session *state
 // on behalf of the requesting OSCAR session (instance). UserInfoQuery performs
 // the OSCAR blocking check and online lookup internally: blocked and offline
 // users both come back as a locate error, which we surface as "offline".
-func (h *PresenceHandler) getUserPresence(ctx context.Context, instance *state.SessionInstance, target state.DisplayScreenName, wantProfileMsg bool) BuddyPresenceInfo {
+func (h *PresenceHandler) getUserPresence(ctx context.Context, instance *state.SessionInstance, baseURL string, target state.DisplayScreenName, wantProfileMsg bool) BuddyPresenceInfo {
 	ident := target.IdentScreenName()
 
 	// Default offline presence
@@ -261,6 +263,12 @@ func (h *PresenceHandler) getUserPresence(ctx context.Context, instance *state.S
 
 	presence.State = "online"
 
+	// Publish the icon only now that locate has confirmed the user is online and
+	// has not blocked the caller. Offline and blocking users return above without
+	// an icon, so neither their icon nor its activity-revealing hash leaks to a
+	// caller they are otherwise invisible to.
+	presence.BuddyIcon = h.IconSource.PublishedURL(ctx, baseURL, ident)
+
 	// The locate reply carries the screen name as the user formatted it, which
 	// beats whatever casing the caller happened to pass in.
 	if info.ScreenName != "" {
@@ -552,7 +560,9 @@ func (h *PresenceHandler) Icon(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	switch h.getUserPresence(r.Context(), instance, state.DisplayScreenName(name), false).State {
+	// This endpoint serves a presence state badge, not the user's buddy icon, so
+	// it has no use for a buddy icon URL.
+	switch h.getUserPresence(r.Context(), instance, "", state.DisplayScreenName(name), false).State {
 	case "away":
 		iconURL = "/static/icons/away_" + iconType + "_" + size + ".png"
 	case "idle":
@@ -595,14 +605,10 @@ func (h *PresenceHandler) pushMyInfo(session *state.WebAPISession, webState, awa
 		return
 	}
 
-	screenName := session.ScreenName.String()
-	myInfo := map[string]interface{}{
-		"aimId":     session.ScreenName.IdentScreenName().String(),
-		"displayId": screenName,
-		"friendly":  screenName,
-		"state":     webState,
-		"userType":  "aim",
-	}
+	// buddyIcon is omitted here (empty) so the client's merge preserves the icon it
+	// already holds; a setState/setStatus does not change the icon. Icon changes
+	// arrive on their own myInfo via the pump's MyInfoRefresher.
+	myInfo := buildMyInfo(session.ScreenName, webState, "")
 	if awayMsg != "" {
 		myInfo["awayMsg"] = awayMsg
 	}

+ 72 - 0
server/webapi/handlers/presence_test.go

@@ -241,6 +241,78 @@ func TestPresenceHandler_GetPresence(t *testing.T) {
 	}
 }
 
+// TestPresenceHandler_GetPresence_PublishesIconForOnlineBuddiesOnly verifies that
+// the icon is published only for an online, non-blocking user, and that an
+// offline or blocking user is never even looked up — so neither their icon nor
+// its hash leaks to a caller they are invisible to.
+func TestPresenceHandler_GetPresence_PublishesIconForOnlineBuddiesOnly(t *testing.T) {
+	ctx := context.Background()
+
+	feedbagService := &MockFeedbagService{}
+	feedbagService.On("Query", mock.Anything, mock.Anything, mock.Anything).
+		Return(wire.SNACMessage{Body: wire.SNAC_0x13_0x06_FeedbagReply{}}, nil).Maybe()
+
+	locateService := &MockLocateService{}
+	locateService.On("UserInfoQuery", mock.Anything, mock.Anything, mock.Anything, screenNameMatcher("onlineuser")).
+		Return(onlineUserInfoReply("onlineuser", 0), nil)
+	locateService.On("UserInfoQuery", mock.Anything, mock.Anything, mock.Anything, screenNameMatcher("offlineuser")).
+		Return(wire.SNACMessage{Body: wire.SNACError{Code: wire.ErrorCodeNotLoggedOn}}, nil)
+
+	iconRetriever := &MockBuddyIconRetriever{}
+	iconRetriever.On("BuddyIconMetadata", mock.Anything, state.NewIdentScreenName("onlineuser")).
+		Return(bartID([]byte{0xab, 0xcd}), nil).Once()
+
+	oscarInstance := state.NewSession().AddInstance()
+	sessionMgr, aimsid := createTestSessionManagerWithOSCAR("testuser", oscarInstance)
+	sess, err := sessionMgr.GetSession(ctx, aimsid)
+	require.NoError(t, err)
+	sess.BaseURL = "http://api.example.com"
+
+	handler := &PresenceHandler{
+		SessionManager: sessionMgr,
+		FeedbagService: feedbagService,
+		LocateService:  locateService,
+		IconSource:     BuddyIconSource{IconRetriever: iconRetriever, Logger: slog.Default()},
+		Logger:         slog.Default(),
+	}
+
+	req, err := http.NewRequest("GET", "/presence/get?aimsid="+aimsid+"&t=onlineuser,offlineuser", nil)
+	require.NoError(t, err)
+	rr := httptest.NewRecorder()
+	requireSession(handler.SessionManager, handler.GetPresence).ServeHTTP(rr, req)
+	require.Equal(t, http.StatusOK, rr.Code)
+
+	var got struct {
+		Response struct {
+			Data struct {
+				Users []struct {
+					AimID     string `json:"aimId"`
+					State     string `json:"state"`
+					BuddyIcon string `json:"buddyIcon"`
+				} `json:"users"`
+			} `json:"data"`
+		} `json:"response"`
+	}
+	require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &got))
+
+	icons := map[string]string{}
+	states := map[string]string{}
+	for _, u := range got.Response.Data.Users {
+		icons[u.AimID] = u.BuddyIcon
+		states[u.AimID] = u.State
+	}
+
+	assert.Equal(t, "online", states["onlineuser"])
+	assert.Equal(t,
+		"http://api.example.com/expressions/get?t=onlineuser&type=buddyIcon&bartId=abcd",
+		icons["onlineuser"])
+
+	assert.Equal(t, "offline", states["offlineuser"])
+	assert.Empty(t, icons["offlineuser"])
+	iconRetriever.AssertNotCalled(t, "BuddyIconMetadata", mock.Anything, state.NewIdentScreenName("offlineuser"))
+	iconRetriever.AssertExpectations(t)
+}
+
 // TestPresenceHandler_GetPresence_BuddyListGrouping verifies that bl=1 places
 // each buddy under its own group using realistic feedbag data, where group rows
 // carry ItemID 0 and a distinct nonzero GroupID, and buddy rows reference those

+ 100 - 42
server/webapi/handlers/session.go

@@ -25,6 +25,7 @@ type SessionHandler struct {
 	OSCARAuthService AuthService
 	FeedbagService   FeedbagService
 	BuddyListManager *BuddyListManager
+	IconSource       BuddyIconSource
 	Logger           *slog.Logger
 	OServiceService  OServiceService
 	FnSessCfg        func(sess *state.Session)
@@ -250,7 +251,16 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// Create WebAPI session
-	session, err := h.SessionManager.CreateSession(r.Context(), screenName, apiKey.DevID, events, instance, h.Logger)
+	// Record the origin the client reached us on. Asset URLs published to the
+	// client (buddy icons) must be absolute, since the client page is served from
+	// a different origin than this API, and they are built in places that have no
+	// request in hand. Pinning the origin here also keeps those URLs on the same
+	// host as the wellKnownUrls advertised below. It is passed into CreateSession
+	// so it is set before the session is published and its listener goroutine can
+	// read it.
+	baseURL := baseURLFromRequest(r)
+
+	session, err := h.SessionManager.CreateSession(r.Context(), screenName, apiKey.DevID, events, instance, baseURL, h.Logger)
 	if err != nil {
 		h.Logger.ErrorContext(ctx, "failed to create session", "err", err.Error())
 		h.sendError(w, r, http.StatusInternalServerError, "failed to create session")
@@ -275,6 +285,21 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 		return LookupBuddyAliases(ctx, h.FeedbagService, session.OSCARSession)
 	}
 
+	// Wire the buddy-icon URL formatter so presence broadcasts (BuddyArrived) can
+	// publish a buddy's current icon from the hash carried in the SNAC, no lookup.
+	session.BuddyIconURL = func(sn state.IdentScreenName, hash []byte) string {
+		return h.IconSource.URLForHash(session.BaseURL, sn, hash)
+	}
+
+	// Wire the myInfo refresher so a self user-info update (icon upload/clear)
+	// re-renders the identity badge. currentWebState reflects the user's live
+	// presence; PublishedURL reflects the feedbag icon, already updated by the time
+	// the OServiceUserInfoUpdate is relayed.
+	session.MyInfoRefresher = func(ctx context.Context) (interface{}, error) {
+		icon := h.IconSource.PublishedURL(ctx, session.BaseURL, screenName.IdentScreenName())
+		return buildMyInfo(screenName, currentWebState(session.OSCARSession), icon), nil
+	}
+
 	// Wire permit/deny refresher so FeedbagUpdateItem SNACs trigger a permitDeny event.
 	session.PermitDenyRefresher = func(ctx context.Context) (interface{}, error) {
 		frame := wire.SNACFrame{FoodGroup: wire.Feedbag, SubGroup: wire.FeedbagQuery}
@@ -289,27 +314,30 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 		return permitDenyData(reply.Items), nil
 	}
 
+	// Now that every refresher callback is wired, start the OSCAR listener. Doing
+	// this inside CreateSession would race these assignments, since the goroutine
+	// reads the callbacks as it converts SNACs into events.
+	session.StartListeningToOSCARSession()
+
 	// Store client info
 	session.ClientName = clientName
 	session.ClientVersion = clientVersion
 	session.FetchTimeout = timeout
 	session.RemoteAddr = r.RemoteAddr
 
+	// The identity badge renders the user's own icon from myInfo, which is the
+	// only event it binds its self-presence render to. PublishedURL always yields
+	// a URL (the blank placeholder when the user has no icon) so that clearing the
+	// icon propagates to the badge.
+	myIconURL := h.IconSource.PublishedURL(ctx, baseURL, screenName.IdentScreenName())
+
 	// Queue myInfo event for authenticated users
 	if authToken != "" {
 		for _, event := range events {
 			if event == "myInfo" || event == "presence" {
-				myInfoData := map[string]interface{}{
-					"aimId":        screenName.IdentScreenName().String(),
-					"displayId":    screenName.String(),
-					"friendly":     screenName.String(),
-					"state":        "online",
-					"onlineTime":   time.Now().Unix(),
-					"memberSince":  time.Now().Unix() - 86400*30, // 30 days ago
-					"capabilities": []string{},
-					"bot":          false,
-					"service":      "AIM",
-				}
+				myInfoData := buildMyInfo(screenName, "online", myIconURL)
+				myInfoData["onlineTime"] = time.Now().Unix()
+				myInfoData["memberSince"] = time.Now().Unix() - 86400*30 // 30 days ago
 				session.EventQueue.Push(types.EventType("myInfo"), myInfoData)
 				break
 			}
@@ -324,8 +352,6 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 	}
 
 	now := time.Now().Unix()
-	scheme := requestScheme(r)
-	baseURL := fmt.Sprintf("%s://%s", scheme, r.Host)
 
 	// Prepare response
 	resp := StartSessionResponse{}
@@ -346,34 +372,26 @@ func (h *SessionHandler) StartSession(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if authToken != "" {
-		myInfoPayload := map[string]interface{}{
-			"aimId":        screenName.IdentScreenName().String(),
-			"displayId":    screenName.String(),
-			"friendly":     screenName.String(),
-			"state":        "online",
-			"onlineTime":   time.Now().Unix(),
-			"memberSince":  time.Now().Unix() - 86400*30, // 30 days ago
-			"capabilities": []string{},
-			"bot":          false,
-			"service":      "AIM",
-			"self": map[string]interface{}{
-				"instNum":        1,
-				"loginTime":      time.Now().Unix(),
-				"sessionTimeout": 30,
-				"events":         events,
-				"assertCaps":     []string{},
-				"rightsInfo": map[string]interface{}{
-					"maxDenies":            500,
-					"maxPermits":           500,
-					"maxWatchers":          3000,
-					"maxBuddies":           500,
-					"maxTempBuddies":       160,
-					"maxIMSize":            3987,
-					"minInterIcbmInterval": 1000,
-					"maxSourceEvil":        900,
-					"maxDstEvil":           999,
-					"maxSigLen":            4096,
-				},
+		myInfoPayload := buildMyInfo(screenName, "online", myIconURL)
+		myInfoPayload["onlineTime"] = time.Now().Unix()
+		myInfoPayload["memberSince"] = time.Now().Unix() - 86400*30 // 30 days ago
+		myInfoPayload["self"] = map[string]interface{}{
+			"instNum":        1,
+			"loginTime":      time.Now().Unix(),
+			"sessionTimeout": 30,
+			"events":         events,
+			"assertCaps":     []string{},
+			"rightsInfo": map[string]interface{}{
+				"maxDenies":            500,
+				"maxPermits":           500,
+				"maxWatchers":          3000,
+				"maxBuddies":           500,
+				"maxTempBuddies":       160,
+				"maxIMSize":            3987,
+				"minInterIcbmInterval": 1000,
+				"maxSourceEvil":        900,
+				"maxDstEvil":           999,
+				"maxSigLen":            4096,
 			},
 		}
 		resp.Response.Data.MyInfo = myInfoPayload
@@ -589,6 +607,40 @@ func (h *SessionHandler) sendError(w http.ResponseWriter, r *http.Request, statu
 	SendResponse(w, r, resp, h.Logger)
 }
 
+// buildMyInfo assembles the shared base of a myInfo payload — the user's own
+// identity blob that the AIM client renders in its identity badge.
+//
+// It carries only fields that are safe to repeat on every myInfo: the client's
+// user-object merge deletes friendly and capabilities before merging, so both
+// must be present on each push or the badge loses them. Time-sensitive fields
+// (onlineTime, memberSince) are intentionally excluded — a mid-session refresh
+// omits them so the client keeps the signon time it already has; the startSession
+// builders add them explicitly. buddyIcon is included only when non-empty; an
+// empty value would be dropped by the client merge anyway, and the placeholder
+// URL (not "") is what clears an icon.
+func buildMyInfo(screenName state.DisplayScreenName, webState, buddyIcon string) map[string]interface{} {
+	// The web client compares userType/service case-sensitively; a UIN account must
+	// report ICQ so it renders as an ICQ contact rather than AIM.
+	userType, service := "aim", "AIM"
+	if screenName.IsUIN() {
+		userType, service = "icq", "ICQ"
+	}
+	myInfo := map[string]interface{}{
+		"aimId":        screenName.IdentScreenName().String(),
+		"displayId":    screenName.String(),
+		"friendly":     screenName.String(),
+		"state":        webState,
+		"userType":     userType,
+		"capabilities": []string{},
+		"bot":          false,
+		"service":      service,
+	}
+	if buddyIcon != "" {
+		myInfo["buddyIcon"] = buddyIcon
+	}
+	return myInfo
+}
+
 func requestScheme(r *http.Request) string {
 	if r.TLS != nil {
 		return "https"
@@ -598,3 +650,9 @@ func requestScheme(r *http.Request) string {
 	}
 	return "http"
 }
+
+// baseURLFromRequest returns the absolute base URL the client reached this
+// server on, used to build asset URLs that the client loads directly.
+func baseURLFromRequest(r *http.Request) string {
+	return fmt.Sprintf("%s://%s", requestScheme(r), r.Host)
+}

+ 41 - 0
server/webapi/handlers/session_test.go

@@ -0,0 +1,41 @@
+package handlers
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/mk6i/open-oscar-server/state"
+)
+
+func TestBuildMyInfo_UserTypeAndService(t *testing.T) {
+	tests := []struct {
+		name       string
+		screenName string
+		wantType   string
+		wantSvc    string
+	}{
+		{"aim screen name", "mikekelly", "aim", "AIM"},
+		{"icq uin", "123456789", "icq", "ICQ"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mi := buildMyInfo(state.DisplayScreenName(tt.screenName), "online", "")
+			assert.Equal(t, tt.wantType, mi["userType"])
+			assert.Equal(t, tt.wantSvc, mi["service"])
+		})
+	}
+}
+
+func TestBuildMyInfo_BuddyIcon(t *testing.T) {
+	t.Run("included when set", func(t *testing.T) {
+		mi := buildMyInfo(state.DisplayScreenName("mikekelly"), "away", "http://x/icon")
+		assert.Equal(t, "http://x/icon", mi["buddyIcon"])
+	})
+	t.Run("omitted when empty so the client merge preserves the current icon", func(t *testing.T) {
+		mi := buildMyInfo(state.DisplayScreenName("mikekelly"), "away", "")
+		_, ok := mi["buddyIcon"]
+		assert.False(t, ok)
+	})
+}

+ 5 - 0
server/webapi/handlers/webapi_event_converter.go

@@ -73,6 +73,11 @@ func ConvertEventForAMF3(event types.Event) map[string]interface{} {
 			if presenceEvent.OnlineTime > 0 {
 				eventData["onlineTime"] = float64(presenceEvent.OnlineTime)
 			}
+			// This branch flattens PresenceEvent through an explicit allowlist, so
+			// buddyIcon must be added here or it never reaches an AMF3 client.
+			if presenceEvent.BuddyIcon != "" {
+				eventData["buddyIcon"] = presenceEvent.BuddyIcon
+			}
 			result["eventData"] = eventData
 		} else if dataMap, ok := event.Data.(map[string]interface{}); ok {
 			// Already a map, ensure timestamps are float64

+ 42 - 0
server/webapi/handlers/webapi_event_converter_test.go

@@ -0,0 +1,42 @@
+package handlers
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/mk6i/open-oscar-server/server/webapi/types"
+)
+
+// The AMF3 converter re-flattens PresenceEvent through an explicit allowlist, so a
+// field absent from that allowlist never reaches an AMF3 client. buddyIcon must be
+// on it.
+func TestConvertEventForAMF3_PresenceCarriesBuddyIcon(t *testing.T) {
+	t.Run("buddyIcon is included when set", func(t *testing.T) {
+		out := ConvertEventForAMF3(types.Event{
+			Type: types.EventTypePresence,
+			Data: types.PresenceEvent{
+				AimID:     "mikekelly",
+				State:     "online",
+				UserType:  "aim",
+				BuddyIcon: "http://api.example.com/expressions/get?t=mikekelly&type=buddyIcon&bartId=dead",
+			},
+		})
+
+		eventData := out["eventData"].(map[string]interface{})
+		assert.Equal(t,
+			"http://api.example.com/expressions/get?t=mikekelly&type=buddyIcon&bartId=dead",
+			eventData["buddyIcon"])
+	})
+
+	t.Run("buddyIcon is omitted when empty", func(t *testing.T) {
+		out := ConvertEventForAMF3(types.Event{
+			Type: types.EventTypePresence,
+			Data: types.PresenceEvent{AimID: "mikekelly", State: "offline", UserType: "aim"},
+		})
+
+		eventData := out["eventData"].(map[string]interface{})
+		_, ok := eventData["buddyIcon"]
+		assert.False(t, ok)
+	})
+}

+ 12 - 5
server/webapi/server.go

@@ -31,6 +31,7 @@ func NewServer(listeners []string, logger *slog.Logger, handler Handler, apiKeyV
 		OSCARAuthService: handler.AuthService,
 		FeedbagService:   handler.FeedbagService,
 		BuddyListManager: handler.BuddyListManager.(*handlers.BuddyListManager),
+		IconSource:       handler.IconSource,
 		Logger:           logger,
 		OServiceService:  handler.OServiceService,
 	}
@@ -45,6 +46,7 @@ func NewServer(listeners []string, logger *slog.Logger, handler Handler, apiKeyV
 		FeedbagService:   handler.FeedbagService,
 		BuddyBroadcaster: handler.BuddyBroadcaster,
 		LocateService:    handler.LocateService,
+		IconSource:       handler.IconSource,
 		Logger:           logger,
 	}
 
@@ -259,11 +261,16 @@ func NewServer(listeners []string, logger *slog.Logger, handler Handler, apiKeyV
 			authMiddleware.CORSMiddleware(
 				http.HandlerFunc(oscarBridgeHandler.StartOSCARSession))))
 
-		// Expressions endpoint (for buddy icons, etc.)
-		expressionsHandler := handlers.NewExpressionsHandler(logger)
-		mux.Handle("GET /expressions/get", authMiddleware.AuthenticateFlexible(
-			authMiddleware.CORSMiddleware(
-				http.HandlerFunc(expressionsHandler.Get))))
+		// Expressions endpoint (for buddy icons, etc.).
+		//
+		// Unauthenticated, like /presence/icon: the buddyIcon URLs this serves are
+		// published to the client and loaded as plain <img> sources, which carry
+		// neither an aimsid nor an API key. Threading a session token through them
+		// instead would leak it into the DOM and defeat caching, since these URLs
+		// outlive the session that produced them. Buddy icons are public assets.
+		expressionsHandler := handlers.NewExpressionsHandler(handler.IconSource, logger)
+		mux.Handle("GET /expressions/get", authMiddleware.CORSMiddleware(
+			http.HandlerFunc(expressionsHandler.Get)))
 
 		// Web AIM calls lifestream/* on the API host (e.g. /lifestream/getUserDetails).
 		lifestreamStub := &handlers.UserInfoStubHandler{Logger: logger}

+ 1 - 0
server/webapi/types/events.go

@@ -47,6 +47,7 @@ type PresenceEvent struct {
 	IdleTime   int    `json:"idleTime,omitempty"`   // Minutes idle
 	OnlineTime int64  `json:"onlineTime,omitempty"` // Unix timestamp
 	UserType   string `json:"userType"`             // "aim", "icq", "admin"
+	BuddyIcon  string `json:"buddyIcon,omitempty"`  // Absolute icon URL; empty preserves the client's current icon, the placeholder URL clears it
 }
 
 // IMEvent represents an instant message event.

+ 67 - 8
state/webapi_session.go

@@ -1,6 +1,7 @@
 package state
 
 import (
+	"bytes"
 	"context"
 	"crypto/rand"
 	"encoding/hex"
@@ -61,6 +62,7 @@ type WebAPISession struct {
 	BOSHost             string                                         // BOS host advertised to the web client
 	BOSPort             int                                            // BOS port advertised to the web client
 	UseSSL              bool                                           // Whether the handoff advertised an SSL BOS connection
+	BaseURL             string                                         // Web API base URL advertised to the web client, used to build absolute asset URLs
 	Events              []string                                       // Subscribed event types
 	EventQueue          *types.EventQueue                              // Per-session event queue
 	DevID               string                                         // Developer ID that created this session
@@ -75,12 +77,16 @@ type WebAPISession struct {
 	TempBuddies         map[string]bool                                // Temporary buddies for this session only
 	BuddyListRefresher  func(ctx context.Context) (interface{}, error) // Called on feedbag changes to push buddylist event
 	PermitDenyRefresher func(ctx context.Context) (interface{}, error) // Called on feedbag changes to push permitDeny event
+	MyInfoRefresher     func(ctx context.Context) (interface{}, error) // Called on self user-info updates (e.g. icon change) to push myInfo event
 	BuddyAliasLoader    func(ctx context.Context) (map[string]string, error)
-	aliases             map[string]string // cached BuddyAliasLoader result, nil when unloaded or invalidated
-	aliasMu             sync.Mutex
-	imLog               map[string][]WebAPIStoredIM
-	imLogMu             sync.Mutex
-	logger              *slog.Logger // Logger for debugging
+	// BuddyIconURL formats the absolute buddyIcon URL for a buddy from the icon
+	// hash carried in a presence SNAC. Returns "" when no URL can be published.
+	BuddyIconURL func(screenName IdentScreenName, hash []byte) string
+	aliases      map[string]string // cached BuddyAliasLoader result, nil when unloaded or invalidated
+	aliasMu      sync.Mutex
+	imLog        map[string][]WebAPIStoredIM
+	imLogMu      sync.Mutex
+	logger       *slog.Logger // Logger for debugging
 }
 
 // IsExpired checks if the session has expired.
@@ -198,9 +204,34 @@ func (s *WebAPISession) handleSNACMessage(msg wire.SNACMessage) {
 		s.handleBuddyMessage(msg)
 	case wire.Feedbag:
 		s.handleFeedbagMessage(msg)
+	case wire.OService:
+		s.handleOServiceMessage(msg)
 	}
 }
 
+// handleOServiceMessage handles OService SNAC messages relayed to the session's
+// own OSCAR instance. The only one we surface is OServiceUserInfoUpdate, which the
+// server relays to a user when their own user info changes (notably a buddy icon
+// upload or clear). The client re-renders its identity badge from myInfo events
+// only, so we translate this into a fresh myInfo.
+func (s *WebAPISession) handleOServiceMessage(msg wire.SNACMessage) {
+	if msg.Frame.SubGroup != wire.OServiceUserInfoUpdate {
+		return
+	}
+	if !s.IsSubscribedTo("myInfo") && !s.IsSubscribedTo("presence") {
+		return
+	}
+	if s.MyInfoRefresher == nil {
+		return
+	}
+	data, err := s.MyInfoRefresher(context.Background())
+	if err != nil {
+		s.logger.Error("failed to refresh myInfo after user-info update", "err", err)
+		return
+	}
+	s.EventQueue.Push(types.EventType("myInfo"), data)
+}
+
 // handleICBMMessage handles ICBM (instant messaging) SNAC messages.
 func (s *WebAPISession) handleICBMMessage(msg wire.SNACMessage) {
 	switch msg.Frame.SubGroup {
@@ -366,6 +397,24 @@ func (s *WebAPISession) handleBuddyArrived(msg wire.SNACMessage) {
 		UserType: "aim",
 	}
 
+	// A BuddyArrived carries the buddy's current icon in TLV 0x1D whenever they
+	// have one, so an icon change (or clear, which arrives as the sentinel hash)
+	// rides along on the presence broadcast. Publish the matching URL: with an
+	// icon it is content-addressed; without one it is the placeholder URL, which
+	// differs from any prior icon URL and so clears a removed icon under the
+	// client's shallow merge. An empty result (no origin known) is omitted, which
+	// preserves whatever icon the client already holds.
+	if s.BuddyIconURL != nil {
+		var hash []byte
+		if b, ok := body.Bytes(wire.OServiceUserInfoBARTInfo); ok {
+			var id wire.BARTID
+			if err := wire.UnmarshalBE(&id, bytes.NewBuffer(b)); err == nil {
+				hash = id.Hash
+			}
+		}
+		presenceEvent.BuddyIcon = s.BuddyIconURL(buddy, hash)
+	}
+
 	s.EventQueue.Push(types.EventTypePresence, presenceEvent)
 }
 
@@ -381,6 +430,8 @@ func (s *WebAPISession) handleBuddyDeparted(msg wire.SNACMessage) {
 	}
 
 	buddy := NewIdentScreenName(body.ScreenName)
+	// BuddyIcon is deliberately omitted: an offline buddy keeps their icon, and
+	// omitting it lets the client's merge preserve the icon it already holds.
 	presenceEvent := types.PresenceEvent{
 		AimID:    buddy.String(),
 		Friendly: s.aliasFor(buddy),
@@ -443,7 +494,13 @@ func NewWebAPISessionManager() *WebAPISessionManager {
 }
 
 // CreateSession creates a new WebAPI session.
-func (m *WebAPISessionManager) CreateSession(ctx context.Context, screenName DisplayScreenName, devID string, events []string, oscarSession *SessionInstance, logger *slog.Logger) (*WebAPISession, error) {
+//
+// The session does not begin listening to its OSCAR instance yet: the caller
+// must wire the session's refresher callbacks (BuddyListRefresher, BuddyIconURL,
+// MyInfoRefresher, ...) and then call StartListeningToOSCARSession. Wiring them
+// after the listener starts would race the goroutine, which reads them as it
+// converts SNACs into events.
+func (m *WebAPISessionManager) CreateSession(ctx context.Context, screenName DisplayScreenName, devID string, events []string, oscarSession *SessionInstance, baseURL string, logger *slog.Logger) (*WebAPISession, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
@@ -464,6 +521,7 @@ func (m *WebAPISessionManager) CreateSession(ctx context.Context, screenName Dis
 		AimSID:          aimsid,
 		ScreenName:      screenName,
 		OSCARSession:    oscarSession,
+		BaseURL:         baseURL,
 		Events:          events,
 		EventQueue:      types.NewEventQueue(1000), // Max 1000 events per session
 		DevID:           devID,
@@ -477,8 +535,9 @@ func (m *WebAPISessionManager) CreateSession(ctx context.Context, screenName Dis
 
 	m.sessions[aimsid] = session
 
-	// Start listening to OSCAR session message channel
-	session.StartListeningToOSCARSession()
+	// The caller starts the OSCAR listener (StartListeningToOSCARSession) once it
+	// has wired the session's refresher callbacks; starting it here would race
+	// those assignments.
 
 	return session, nil
 }

+ 134 - 5
state/webapi_session_test.go

@@ -2,6 +2,7 @@ package state
 
 import (
 	"context"
+	"encoding/hex"
 	"io"
 	"log/slog"
 	"testing"
@@ -286,7 +287,7 @@ func TestWebAPISessionManager_CreateAfterShutdown(t *testing.T) {
 	ctx := context.Background()
 	mgr.Shutdown()
 
-	sess, err := mgr.CreateSession(ctx, DisplayScreenName("testuser"), "dev", []string{"presence"}, nil, nil)
+	sess, err := mgr.CreateSession(ctx, DisplayScreenName("testuser"), "dev", []string{"presence"}, nil, "", nil)
 	assert.Nil(t, sess)
 	assert.ErrorIs(t, err, ErrWebAPISessionManagerClosed)
 }
@@ -301,9 +302,9 @@ func TestWebAPISessionManager_ShutdownDrainsAndClosesSessions(t *testing.T) {
 	inst1 := NewSession().AddInstance()
 	inst2 := NewSession().AddInstance()
 
-	s1, err := mgr.CreateSession(ctx, DisplayScreenName("alice"), "dev", []string{"presence"}, inst1, slog.Default())
+	s1, err := mgr.CreateSession(ctx, DisplayScreenName("alice"), "dev", []string{"presence"}, inst1, "", slog.Default())
 	assert.NoError(t, err)
-	s2, err := mgr.CreateSession(ctx, DisplayScreenName("bob"), "dev", []string{"presence"}, inst2, slog.Default())
+	s2, err := mgr.CreateSession(ctx, DisplayScreenName("bob"), "dev", []string{"presence"}, inst2, "", slog.Default())
 	assert.NoError(t, err)
 
 	mgr.Shutdown()
@@ -335,9 +336,9 @@ func TestWebAPISessionManager_ReapExpired(t *testing.T) {
 	expiredInst := NewSession().AddInstance()
 	liveInst := NewSession().AddInstance()
 
-	expired, err := mgr.CreateSession(ctx, "alice", "dev", []string{"presence"}, expiredInst, slog.Default())
+	expired, err := mgr.CreateSession(ctx, "alice", "dev", []string{"presence"}, expiredInst, "", slog.Default())
 	assert.NoError(t, err)
-	live, err := mgr.CreateSession(ctx, "bob", "dev", []string{"presence"}, liveInst, slog.Default())
+	live, err := mgr.CreateSession(ctx, "bob", "dev", []string{"presence"}, liveInst, "", slog.Default())
 	assert.NoError(t, err)
 
 	// Force alice's session into the past; bob keeps its default future expiry.
@@ -631,3 +632,131 @@ func TestWebAPISession_HandleBuddyArrivedDeparted_NormalizesAimID(t *testing.T)
 	assert.Equal(t, "mikekelly", departed.AimID)
 	assert.Equal(t, "offline", departed.State)
 }
+
+// A BuddyArrived carries the buddy's current icon as TLV 0x1D, so an icon change
+// rides along on the presence broadcast and must reach the presence event. The
+// stub BuddyIconURL stands in for the handlers-side URL formatter, which state
+// cannot import.
+func TestWebAPISession_PublishesBuddyIconOnPresence(t *testing.T) {
+	newSession := func() *WebAPISession {
+		return &WebAPISession{
+			ScreenName: DisplayScreenName("me"),
+			Events:     []string{"presence"},
+			EventQueue: types.NewEventQueue(10),
+			logger:     slog.New(slog.NewTextHandler(io.Discard, nil)),
+			BuddyIconURL: func(sn IdentScreenName, hash []byte) string {
+				if len(hash) == 0 {
+					return "placeholder:" + sn.String()
+				}
+				return "icon:" + hex.EncodeToString(hash)
+			},
+		}
+	}
+
+	arrived := func(sess *WebAPISession, screenName string, hash []byte) {
+		info := wire.TLVUserInfo{ScreenName: screenName}
+		if hash != nil {
+			info.Append(wire.NewTLVBE(wire.OServiceUserInfoBARTInfo, wire.BARTID{
+				Type:     wire.BARTTypesBuddyIcon,
+				BARTInfo: wire.BARTInfo{Hash: hash},
+			}))
+		}
+		sess.handleBuddyArrived(wire.SNACMessage{Body: wire.SNAC_0x03_0x0B_BuddyArrived{TLVUserInfo: info}})
+	}
+
+	lastPresence := func(sess *WebAPISession) types.PresenceEvent {
+		events := sess.EventQueue.GetAllEvents()
+		require.Len(t, events, 1)
+		return events[0].Data.(types.PresenceEvent)
+	}
+
+	t.Run("icon hash yields the content-addressed URL", func(t *testing.T) {
+		sess := newSession()
+		arrived(sess, "Mike Kelly", []byte{0xde, 0xad, 0xbe, 0xef})
+		assert.Equal(t, "icon:deadbeef", lastPresence(sess).BuddyIcon)
+	})
+
+	t.Run("no icon TLV yields the placeholder URL", func(t *testing.T) {
+		sess := newSession()
+		arrived(sess, "Mike Kelly", nil)
+		assert.Equal(t, "placeholder:mikekelly", lastPresence(sess).BuddyIcon)
+	})
+
+	t.Run("cleared icon yields a URL naming the sentinel hash", func(t *testing.T) {
+		sess := newSession()
+		arrived(sess, "Mike Kelly", wire.GetClearIconHash())
+		assert.Equal(t, "icon:"+hex.EncodeToString(wire.GetClearIconHash()), lastPresence(sess).BuddyIcon)
+	})
+
+	t.Run("departed omits the icon so the client preserves it", func(t *testing.T) {
+		sess := newSession()
+		sess.handleBuddyDeparted(wire.SNACMessage{Body: wire.SNAC_0x03_0x0C_BuddyDeparted{
+			TLVUserInfo: wire.TLVUserInfo{ScreenName: "Mike Kelly"},
+		}})
+		assert.Empty(t, lastPresence(sess).BuddyIcon)
+	})
+
+	t.Run("no callback wired omits the icon", func(t *testing.T) {
+		sess := newSession()
+		sess.BuddyIconURL = nil
+		arrived(sess, "Mike Kelly", []byte{0x01})
+		assert.Empty(t, lastPresence(sess).BuddyIcon)
+	})
+}
+
+// A user's own icon change is relayed to their session as OServiceUserInfoUpdate,
+// which the pump turns into a myInfo event so the identity badge re-renders.
+func TestWebAPISession_PushesMyInfoOnUserInfoUpdate(t *testing.T) {
+	newSession := func(events ...string) (*WebAPISession, *int) {
+		var refreshes int
+		return &WebAPISession{
+			ScreenName: DisplayScreenName("me"),
+			Events:     events,
+			EventQueue: types.NewEventQueue(10),
+			logger:     slog.New(slog.NewTextHandler(io.Discard, nil)),
+			MyInfoRefresher: func(_ context.Context) (interface{}, error) {
+				refreshes++
+				return map[string]interface{}{"aimId": "me", "buddyIcon": "icon:new"}, nil
+			},
+		}, &refreshes
+	}
+
+	userInfoUpdate := wire.SNACMessage{Frame: wire.SNACFrame{
+		FoodGroup: wire.OService,
+		SubGroup:  wire.OServiceUserInfoUpdate,
+	}}
+
+	t.Run("subscribed session gets one myInfo event", func(t *testing.T) {
+		sess, refreshes := newSession("myInfo")
+		sess.handleSNACMessage(userInfoUpdate)
+
+		events := sess.EventQueue.GetAllEvents()
+		require.Len(t, events, 1)
+		assert.Equal(t, "myInfo", string(events[0].Type))
+		assert.Equal(t, "icon:new", events[0].Data.(map[string]interface{})["buddyIcon"])
+		assert.Equal(t, 1, *refreshes)
+	})
+
+	t.Run("a presence subscription also delivers myInfo", func(t *testing.T) {
+		sess, _ := newSession("presence")
+		sess.handleSNACMessage(userInfoUpdate)
+		assert.Len(t, sess.EventQueue.GetAllEvents(), 1)
+	})
+
+	t.Run("unsubscribed session gets nothing and does not refresh", func(t *testing.T) {
+		sess, refreshes := newSession("im")
+		sess.handleSNACMessage(userInfoUpdate)
+		assert.Empty(t, sess.EventQueue.GetAllEvents())
+		assert.Equal(t, 0, *refreshes)
+	})
+
+	t.Run("other OService subgroups are ignored", func(t *testing.T) {
+		sess, refreshes := newSession("myInfo")
+		sess.handleSNACMessage(wire.SNACMessage{Frame: wire.SNACFrame{
+			FoodGroup: wire.OService,
+			SubGroup:  wire.OServiceRateParamsQuery,
+		}})
+		assert.Empty(t, sess.EventQueue.GetAllEvents())
+		assert.Equal(t, 0, *refreshes)
+	})
+}