4
0
Эх сурвалжийг харах

fix offline message delivery in ICQlite

Fix faulty offline message delivery behavior where ICQlite client
only receives the first offline message at signon, and the message
never gets acked.

Also removed wire.ErrorCodeNotLoggedOn ICBM offline reply code.
It prevented the ICQLite client from getting the "message delivered
offline message". I don't know why it existed in the first place...
ICQ 2000b behaves fine without it.
Mike 1 сар өмнө
parent
commit
189bbb34c6

+ 0 - 4
foodgroup/icbm.go

@@ -285,10 +285,6 @@ func (s *ICBMService) sendOfflineMessage(ctx context.Context, instance *state.Se
 		return nil, fmt.Errorf("save ICBM offline message failed: %w", err)
 	}
 
-	if instance.UIN() > 0 {
-		return newICBMErr(inFrame.RequestID, wire.ErrorCodeNotLoggedOn), nil
-	}
-
 	if _, requestedConfirmation := inBody.Bytes(wire.ICBMTLVRequestHostAck); requestedConfirmation {
 		// ack message back to sender
 		return &wire.SNACMessage{

+ 1 - 12
foodgroup/icbm_test.go

@@ -466,22 +466,12 @@ func TestICBMService_ChannelMsgToHost(t *testing.T) {
 					ScreenName: "22222222",
 					TLVRestBlock: wire.TLVRestBlock{
 						TLVList: wire.TLVList{
-							wire.NewTLVBE(wire.ICBMTLVRequestHostAck, []byte{}),
 							wire.NewTLVBE(wire.ICBMTLVStore, []byte{}),
 						},
 					},
 				},
 			},
-			expectOutput: &wire.SNACMessage{
-				Frame: wire.SNACFrame{
-					FoodGroup: wire.ICBM,
-					SubGroup:  wire.ICBMErr,
-					RequestID: 1234,
-				},
-				Body: wire.SNACError{
-					Code: wire.ErrorCodeNotLoggedOn,
-				},
-			},
+			expectOutput: nil,
 			timeNow: func() time.Time {
 				return time.Date(2020, time.August, 1, 0, 0, 0, 0, time.UTC)
 			},
@@ -521,7 +511,6 @@ func TestICBMService_ChannelMsgToHost(t *testing.T) {
 									ScreenName: "22222222",
 									TLVRestBlock: wire.TLVRestBlock{
 										TLVList: wire.TLVList{
-											wire.NewTLVBE(wire.ICBMTLVRequestHostAck, []byte{}),
 											wire.NewTLVBE(wire.ICBMTLVStore, []byte{}),
 										},
 									},

+ 1 - 1
foodgroup/icq.go

@@ -445,7 +445,7 @@ func (s *ICQService) OfflineMsgReq(ctx context.Context, inFrame wire.SNACFrame,
 		msgOut := wire.ICQMessageReplyEnvelope{
 			Message: reply,
 		}
-		if err := s.reply(ctx, instance, msgOut, inFrame.RequestID, 0); err != nil {
+		if err := s.reply(ctx, instance, msgOut, inFrame.RequestID, wire.SNACFlagsMoreToCome); err != nil {
 			return fmt.Errorf("sending offline message: %w", err)
 		}
 	}

+ 2 - 0
foodgroup/icq_test.go

@@ -2380,6 +2380,7 @@ func TestICQService_OfflineMsgReq(t *testing.T) {
 								Frame: wire.SNACFrame{
 									FoodGroup: wire.ICQ,
 									SubGroup:  wire.ICQDBReply,
+									Flags:     wire.SNACFlagsMoreToCome,
 									RequestID: 1234,
 								},
 								Body: wire.SNAC_0x15_0x02_DBReply{
@@ -2413,6 +2414,7 @@ func TestICQService_OfflineMsgReq(t *testing.T) {
 								Frame: wire.SNACFrame{
 									FoodGroup: wire.ICQ,
 									SubGroup:  wire.ICQDBReply,
+									Flags:     wire.SNACFlagsMoreToCome,
 									RequestID: 1234,
 								},
 								Body: wire.SNAC_0x15_0x02_DBReply{

+ 1 - 1
foodgroup/oservice.go

@@ -732,7 +732,7 @@ func (s OServiceService) ClientOnline(ctx context.Context, service uint16, inBod
 			}
 		}
 
-		if instance.OfflineMsgCount() > 0 {
+		if instance.UIN() == 0 && instance.OfflineMsgCount() > 0 {
 			if err := s.sendOfflineMessageNotification(ctx, instance); err != nil {
 				return fmt.Errorf("send offline message notification: %w", err)
 			}

+ 39 - 0
foodgroup/oservice_test.go

@@ -2518,6 +2518,45 @@ func TestOServiceService_ClientOnline(t *testing.T) {
 				assert.Equal(t, 0, instance.OfflineMsgCount())
 			},
 		},
+		{
+			name:     "ICQ user with offline messages, no offline message notification sent",
+			instance: newTestInstance("100001", sessOptCannedSignonTime, sessOptOfflineMsgCount(3), sessOptContactsInit, sessOptUIN(100001)),
+			bodyIn:   wire.SNAC_0x01_0x02_OServiceClientOnline{},
+			service:  wire.BOS,
+			mockParams: mockParams{
+				buddyBroadcasterParams: buddyBroadcasterParams{
+					broadcastVisibilityParams: broadcastVisibilityParams{
+						{
+							from:             state.NewIdentScreenName("100001"),
+							filter:           nil,
+							doSendDepartures: false,
+						},
+					},
+				},
+				messageRelayerParams: messageRelayerParams{
+					relayToScreenNameParams: relayToScreenNameParams{
+						{
+							screenName: state.NewIdentScreenName("100001"),
+							message: wire.SNACMessage{
+								Frame: wire.SNACFrame{
+									FoodGroup: wire.Stats,
+									SubGroup:  wire.StatsSetMinReportInterval,
+									RequestID: wire.ReqIDFromServer,
+								},
+								Body: wire.SNAC_0x0B_0x02_StatsSetMinReportInterval{
+									MinReportInterval: 1,
+								},
+							},
+						},
+					},
+				},
+			},
+			validateSess: func(t *testing.T, instance *state.SessionInstance) {
+				assert.True(t, instance.SignonComplete())
+				assert.Equal(t, uint32(100001), instance.UIN())
+				assert.Equal(t, 3, instance.OfflineMsgCount())
+			},
+		},
 		{
 			name: "notify that BOS user is logged in to multiple locations",
 			instance: func() *state.SessionInstance {