From c7a77c23c8a398da24ba888d49dbf2f89425c0d0 Mon Sep 17 00:00:00 2001 From: Astra Date: Wed, 9 Sep 2026 11:15:56 +0100 Subject: [PATCH] forum: fix reply_to_top_id for replies inside a forum resolveChannelReply applied discussion-thread logic (reply_to_top_id = the replied-to message's own id) to forum replies. Replying to a General message produced reply_to_top_id = , a topic no client can resolve: the reply vanished from every topic view and reply-jump on strict clients said "message doesn't exist". Forum replies now inherit the target's topic via domain.ForumReplyTopicID (target's topic, or its own id if it's a topic-create, else General), and General (topic 1) is accepted as a valid virtual topic everywhere, so sends carrying top_msg_id: 1 are no longer rejected. Non-forum discussion threads are unchanged. --- internal/domain/channel.go | 17 +++ internal/rpc/forum_reply_topic_rpc_test.go | 120 +++++++++++++++++++++ internal/store/memory/channel_helpers.go | 52 ++++++--- internal/store/postgres/channel_helpers.go | 61 +++++++---- 4 files changed, 213 insertions(+), 37 deletions(-) create mode 100644 internal/rpc/forum_reply_topic_rpc_test.go diff --git a/internal/domain/channel.go b/internal/domain/channel.go index 7da4bba3..42adb506 100644 --- a/internal/domain/channel.go +++ b/internal/domain/channel.go @@ -735,6 +735,23 @@ type ChannelMessage struct { Deleted bool } +// ForumReplyTopicID resolves the topic a reply to target belongs to inside a +// forum. Every forum message lives in exactly one topic, and a reply inherits +// the target's topic - never the target's own id. Using target.ID is +// discussion-thread logic (comment threads on a broadcast post) and does not +// apply to forums: it manufactures a topic reference that no channel_forum_topics +// row backs, which strict clients cannot place. A target with no recorded topic +// is in General. +func ForumReplyTopicID(target ChannelMessage) int { + if target.Action != nil && target.Action.Type == ChannelActionTopicCreate { + return target.ID // the target itself is a topic root + } + if target.ReplyTo != nil && target.ReplyTo.TopMessageID > 0 { + return target.ReplyTo.TopMessageID + } + return ForumGeneralTopicID +} + // ProjectChannelHistoryClearMessage returns the owner-local service-message // projection for one channel history boundary. Identity fields from the shared // source are retained when available, while all user payload, media, reply, diff --git a/internal/rpc/forum_reply_topic_rpc_test.go b/internal/rpc/forum_reply_topic_rpc_test.go new file mode 100644 index 00000000..8ef2dad7 --- /dev/null +++ b/internal/rpc/forum_reply_topic_rpc_test.go @@ -0,0 +1,120 @@ +package rpc + +import ( + "context" + "testing" + + "github.com/iamxvbaba/td/clock" + "github.com/iamxvbaba/td/tg" + "go.uber.org/zap/zaptest" + + appchannels "telesrv/internal/app/channels" + appusers "telesrv/internal/app/users" + "telesrv/internal/domain" + "telesrv/internal/store/memory" +) + +// A reply inside a forum must inherit the *target's* topic, never the target's +// own message id. Regression: replying to a General message produced +// reply_to_top_id = , a topic that no client can resolve, so +// the reply vanished from every topic view and reply-jump said "doesn't exist". +func TestForumReplyInheritsTargetTopic(t *testing.T) { + ctx := context.Background() + userStore := memory.NewUserStore() + owner, _ := userStore.Create(ctx, domain.User{AccessHash: 91, Phone: "15550009101", FirstName: "Owner"}) + channelStore := memory.NewChannelStore() + r := New(Config{}, Deps{ + Users: appusers.NewService(userStore), + Channels: appchannels.NewService(channelStore), + }, zaptest.NewLogger(t), clock.System) + ownerCtx := WithUserID(ctx, owner.ID) + + created, err := r.onChannelsCreateChannel(ownerCtx, &tg.ChannelsCreateChannelRequest{Title: "Forum", Megagroup: true}) + if err != nil { + t.Fatalf("create channel: %v", err) + } + channel := created.(*tg.Updates).Chats[0].(*tg.Channel) + input := &tg.InputChannel{ChannelID: channel.ID, AccessHash: channel.AccessHash} + peer := &tg.InputPeerChannel{ChannelID: channel.ID, AccessHash: channel.AccessHash} + if _, err := r.onChannelsToggleForum(ownerCtx, &tg.ChannelsToggleForumRequest{Channel: input, Enabled: true, Tabs: true}); err != nil { + t.Fatalf("toggle forum: %v", err) + } + topicUpd, err := r.onMessagesCreateForumTopic(ownerCtx, &tg.MessagesCreateForumTopicRequest{ + Peer: peer, Title: "Test", IconColor: domain.DefaultForumTopicIconColor, RandomID: 9101001, + }) + if err != nil { + t.Fatalf("create topic: %v", err) + } + testTopicID := forumTopicRootMessageID(t, topicUpd, "Test") + + send := func(text string, randomID int64, reply *tg.InputReplyToMessage) *tg.Message { + req := &tg.MessagesSendMessageRequest{Peer: peer, Message: text, RandomID: randomID} + if reply != nil { + req.SetReplyTo(reply) + } + upd, err := r.onMessagesSendMessage(ownerCtx, req) + if err != nil { + t.Fatalf("send %q: %v", text, err) + } + for _, u := range upd.(*tg.Updates).Updates { + if nm, ok := u.(*tg.UpdateNewChannelMessage); ok { + if m, ok := nm.Message.(*tg.Message); ok && m.Message == text { + return m + } + } + } + t.Fatalf("no new message for %q in %+v", text, upd) + return nil + } + topID := func(m *tg.Message) int { + h, ok := m.ReplyTo.(*tg.MessageReplyHeader) + if !ok { + t.Fatalf("message %d has reply header %T, want *MessageReplyHeader", m.ID, m.ReplyTo) + } + id, _ := h.GetReplyToTopID() + if !h.ForumTopic { + t.Fatalf("message %d reply header missing forum_topic flag: %+v", m.ID, h) + } + return id + } + + // A plain General message (no reply header). + g1 := send("g1", 9101002, nil) + + // Reply to it -> topic must be General (1), not g1.ID. + r1 := send("r1", 9101003, &tg.InputReplyToMessage{ReplyToMsgID: g1.ID}) + if got := topID(r1); got != domain.ForumGeneralTopicID { + t.Fatalf("reply to a General message: reply_to_top_id = %d, want %d (General), not the target id %d", + got, domain.ForumGeneralTopicID, g1.ID) + } + + // Reply again, this time the client also passes top_msg_id: 1 (General). + // Previously this was rejected because General has no channel_forum_topics row. + replyWithTop := &tg.InputReplyToMessage{ReplyToMsgID: g1.ID} + replyWithTop.SetTopMsgID(domain.ForumGeneralTopicID) + r2 := send("r2", 9101004, replyWithTop) + if got := topID(r2); got != domain.ForumGeneralTopicID { + t.Fatalf("reply with top_msg_id=1: reply_to_top_id = %d, want %d", got, domain.ForumGeneralTopicID) + } + + // Post directly into the "Test" topic, then reply to a plain message there. + tInTopic := &tg.InputReplyToMessage{ReplyToMsgID: 0} + tInTopic.SetTopMsgID(testTopicID) + m1 := send("t1", 9101005, tInTopic) + if got := topID(m1); got != testTopicID { + t.Fatalf("message in Test topic: reply_to_top_id = %d, want %d", got, testTopicID) + } + rt := send("rt", 9101006, &tg.InputReplyToMessage{ReplyToMsgID: m1.ID}) + if got := topID(rt); got != testTopicID { + t.Fatalf("reply inside Test topic: reply_to_top_id = %d, want %d (topic), not %d", got, testTopicID, m1.ID) + } + + // Replying to a General message while claiming a mismatched topic is rejected. + bad := &tg.InputReplyToMessage{ReplyToMsgID: g1.ID} + bad.SetTopMsgID(testTopicID) + req := &tg.MessagesSendMessageRequest{Peer: peer, Message: "bad", RandomID: 9101007} + req.SetReplyTo(bad) + if _, err := r.onMessagesSendMessage(ownerCtx, req); err == nil { + t.Fatal("reply with a topic id that doesn't match the target's topic was accepted") + } +} diff --git a/internal/store/memory/channel_helpers.go b/internal/store/memory/channel_helpers.go index 4813aaad..98d31fc4 100644 --- a/internal/store/memory/channel_helpers.go +++ b/internal/store/memory/channel_helpers.go @@ -540,18 +540,14 @@ func (s *ChannelStore) resolveChannelReplyLocked(req domain.SendChannelMessageRe if req.ReplyTo.TopMessageID <= 0 || !channel.Forum { return nil, domain.ErrReplyMessageIDInvalid } - topic, ok := s.topics[req.ChannelID][req.ReplyTo.TopMessageID] - if !ok || topic.Hidden { - return nil, domain.ErrReplyMessageIDInvalid - } - if topic.Closed && !canManageForumTopic(channel, member, topic, req.UserID, selfBoostsApplied) { - return nil, domain.ErrChannelWriteForbidden - } reply := cloneMessageReply(req.ReplyTo) reply.MessageID = 0 reply.Peer = channelPeer - reply.TopMessageID = topic.TopicID reply.ForumTopic = true + if err := s.validateForumReplyTopicLocked(channel, member, req.ReplyTo.TopMessageID, req.UserID, selfBoostsApplied); err != nil { + return nil, err + } + reply.TopMessageID = req.ReplyTo.TopMessageID return reply, nil } target, ok := s.findMessageLocked(req.ChannelID, req.ReplyTo.MessageID) @@ -561,6 +557,22 @@ func (s *ChannelStore) resolveChannelReplyLocked(req domain.SendChannelMessageRe reply := cloneMessageReply(req.ReplyTo) reply.MessageID = target.ID reply.Peer = channelPeer + + if channel.Forum { + // A forum reply belongs to the TARGET's topic, never the target's own id. + topicID := domain.ForumReplyTopicID(target) + if req.ReplyTo.TopMessageID > 0 && req.ReplyTo.TopMessageID != topicID { + return nil, domain.ErrReplyMessageIDInvalid + } + if err := s.validateForumReplyTopicLocked(channel, member, topicID, req.UserID, selfBoostsApplied); err != nil { + return nil, err + } + reply.TopMessageID = topicID + reply.ForumTopic = true + return reply, nil + } + + // Non-forum discussion thread: reply_to_top_id is the comment-thread root. reply.TopMessageID = target.ID if target.ReplyTo != nil && target.ReplyTo.TopMessageID > 0 { reply.TopMessageID = target.ReplyTo.TopMessageID @@ -571,17 +583,25 @@ func (s *ChannelStore) resolveChannelReplyLocked(req domain.SendChannelMessageRe if channel.Forum && req.ReplyTo.TopMessageID > 0 && req.ReplyTo.TopMessageID != reply.TopMessageID { return nil, domain.ErrReplyMessageIDInvalid } - if channel.Forum && reply.TopMessageID > 0 { - if topic, ok := s.topics[req.ChannelID][reply.TopMessageID]; ok && !topic.Hidden { - if topic.Closed && !canManageForumTopic(channel, member, topic, req.UserID, selfBoostsApplied) { - return nil, domain.ErrChannelWriteForbidden - } - reply.ForumTopic = true - } - } return reply, nil } +// validateForumReplyTopicLocked mirrors the postgres store: General +// (ForumGeneralTopicID) is a virtual topic with no row and is always valid. +func (s *ChannelStore) validateForumReplyTopicLocked(channel domain.Channel, member domain.ChannelMember, topicID int, userID int64, selfBoostsApplied int) error { + if topicID == domain.ForumGeneralTopicID { + return nil + } + topic, ok := s.topics[channel.ID][topicID] + if !ok || topic.Hidden { + return domain.ErrReplyMessageIDInvalid + } + if topic.Closed && !canManageForumTopic(channel, member, topic, userID, selfBoostsApplied) { + return domain.ErrChannelWriteForbidden + } + return nil +} + func inactiveChannelDate(dialog domain.Dialog, channel domain.Channel, member domain.ChannelMember) int { if dialog.TopMessageDate > 0 { return dialog.TopMessageDate diff --git a/internal/store/postgres/channel_helpers.go b/internal/store/postgres/channel_helpers.go index d8c81fc2..6825c13e 100644 --- a/internal/store/postgres/channel_helpers.go +++ b/internal/store/postgres/channel_helpers.go @@ -850,21 +850,14 @@ WHERE owner_user_id=$1 AND peer_type='user' AND peer_id=$2 AND box_id=$3 AND NOT if req.ReplyTo.TopMessageID <= 0 || !channel.Forum { return nil, domain.ErrReplyMessageIDInvalid } - topic, err := s.getForumTopic(ctx, db, req.ChannelID, req.ReplyTo.TopMessageID) - if err != nil { - return nil, domain.ErrReplyMessageIDInvalid - } - if topic.Hidden { - return nil, domain.ErrReplyMessageIDInvalid - } - if topic.Closed && !canManageForumTopic(channel, member, topic, req.UserID, selfBoostsApplied) { - return nil, domain.ErrChannelWriteForbidden - } reply := cloneMessageReply(req.ReplyTo) reply.MessageID = 0 reply.Peer = channelPeer - reply.TopMessageID = topic.TopicID reply.ForumTopic = true + if err := s.validateForumReplyTopic(ctx, db, channel, member, req.ReplyTo.TopMessageID, req.UserID, selfBoostsApplied); err != nil { + return nil, err + } + reply.TopMessageID = req.ReplyTo.TopMessageID return reply, nil } target, err := s.getChannelMessage(ctx, db, req.ChannelID, req.ReplyTo.MessageID) @@ -880,6 +873,22 @@ WHERE owner_user_id=$1 AND peer_type='user' AND peer_id=$2 AND box_id=$3 AND NOT reply := cloneMessageReply(req.ReplyTo) reply.MessageID = target.ID reply.Peer = channelPeer + + if channel.Forum { + // A forum reply belongs to the TARGET's topic, never the target's own id. + topicID := domain.ForumReplyTopicID(target) + if req.ReplyTo.TopMessageID > 0 && req.ReplyTo.TopMessageID != topicID { + return nil, domain.ErrReplyMessageIDInvalid + } + if err := s.validateForumReplyTopic(ctx, db, channel, member, topicID, req.UserID, selfBoostsApplied); err != nil { + return nil, err + } + reply.TopMessageID = topicID + reply.ForumTopic = true + return reply, nil + } + + // Non-forum discussion thread: reply_to_top_id is the comment-thread root. reply.TopMessageID = target.ID if target.ReplyTo != nil && target.ReplyTo.TopMessageID > 0 { reply.TopMessageID = target.ReplyTo.TopMessageID @@ -896,19 +905,29 @@ WHERE owner_user_id=$1 AND peer_type='user' AND peer_id=$2 AND box_id=$3 AND NOT if channel.Forum && req.ReplyTo.TopMessageID > 0 && req.ReplyTo.TopMessageID != reply.TopMessageID { return nil, domain.ErrReplyMessageIDInvalid } - if channel.Forum && reply.TopMessageID > 0 { - if topic, err := s.getForumTopic(ctx, db, req.ChannelID, reply.TopMessageID); err == nil && !topic.Hidden { - if topic.Closed && !canManageForumTopic(channel, member, topic, req.UserID, selfBoostsApplied) { - return nil, domain.ErrChannelWriteForbidden - } - reply.ForumTopic = true - } else if err != nil && !errors.Is(err, domain.ErrMessageIDInvalid) { - return nil, err - } - } return reply, nil } +// validateForumReplyTopic checks that topicID is a topic the caller may post +// into. General (ForumGeneralTopicID) is a virtual topic with no +// channel_forum_topics row and is always valid. +func (s *ChannelStore) validateForumReplyTopic(ctx context.Context, db sqlcgen.DBTX, channel domain.Channel, member domain.ChannelMember, topicID int, userID int64, selfBoostsApplied int) error { + if topicID == domain.ForumGeneralTopicID { + return nil + } + topic, err := s.getForumTopic(ctx, db, channel.ID, topicID) + if err != nil { + return domain.ErrReplyMessageIDInvalid + } + if topic.Hidden { + return domain.ErrReplyMessageIDInvalid + } + if topic.Closed && !canManageForumTopic(channel, member, topic, userID, selfBoostsApplied) { + return domain.ErrChannelWriteForbidden + } + return nil +} + func visibleChannelTopAfter(ctx context.Context, db sqlcgen.DBTX, channelID int64, availableMinID int, fallbackDate int) (int, int, error) { var id, date int err := db.QueryRow(ctx, `