Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle state before, send history visibility in output #2532

Merged
merged 6 commits into from
Jun 13, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion roomserver/api/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ type OutputNewRoomEvent struct {
// The transaction ID of the send request if sent by a local user and one
// was specified
TransactionID *TransactionID `json:"transaction_id,omitempty"`
// The history visibility of the event.
HistoryVisibility string `json:"history_visibility"`
neilalexander marked this conversation as resolved.
Show resolved Hide resolved
}

func (o *OutputNewRoomEvent) NeededStateEventIDs() ([]*gomatrixserverlib.HeaderedEvent, []string) {
Expand All @@ -187,7 +189,8 @@ func (o *OutputNewRoomEvent) NeededStateEventIDs() ([]*gomatrixserverlib.Headere
// should build their current room state up from OutputNewRoomEvents only.
type OutputOldRoomEvent struct {
// The Event.
Event *gomatrixserverlib.HeaderedEvent `json:"event"`
Event *gomatrixserverlib.HeaderedEvent `json:"event"`
HistoryVisibility string `json:"history_visibility"`
}

// An OutputNewInviteEvent is written whenever an invite becomes active.
Expand Down
108 changes: 107 additions & 1 deletion roomserver/internal/input/input_events.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,22 @@ func (r *Inputer) processRoomEvent(
}
}

// Get the state before the event so that we can work out if the event was
// allowed at the time, and also to get the history visibility. We won't
// bother doing this if the event was already rejected as it just ends up
// burning CPU time.
historyVisibility := "joined" // Default to restrictive.
if rejectionErr == nil && !isRejected && !softfail {
var err error
historyVisibility, rejectionErr, err = r.processStateBefore(ctx, input, missingPrev)
if err != nil {
return fmt.Errorf("r.processStateBefore: %w", err)
}
if rejectionErr != nil {
isRejected = true
}
}

// Store the event.
_, _, stateAtEvent, redactionEvent, redactedEventID, err := r.DB.StoreEvent(ctx, event, authEventNIDs, isRejected || softfail)
if err != nil {
Expand Down Expand Up @@ -360,6 +376,7 @@ func (r *Inputer) processRoomEvent(
input.SendAsServer, // send as server
input.TransactionID, // transaction ID
input.HasState, // rewrites state?
historyVisibility, // the history visibility
); err != nil {
return fmt.Errorf("r.updateLatestEvents: %w", err)
}
Expand All @@ -368,7 +385,8 @@ func (r *Inputer) processRoomEvent(
{
Type: api.OutputTypeOldRoomEvent,
OldRoomEvent: &api.OutputOldRoomEvent{
Event: headered,
Event: headered,
HistoryVisibility: historyVisibility,
},
},
})
Expand Down Expand Up @@ -402,6 +420,94 @@ func (r *Inputer) processRoomEvent(
return nil
}

// processStateBefore works out what the state is before the event and
// then checks the event auths against the state at the time. It also
// tries to determine what the history visibility was of the event at
// the time, so that it can be sent in the output event to downstream
// components.
// nolint:nakedret
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm with the linter on this one. This function is huge, and having naked returns make it hard to figure out what is returned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm really not, because the alternative is we have a (string, error, error) signature, we have to document in the comments why there are two error types (one is "the event was rejected but please continue", the other is "something went really wrong, stop the world") and then it's still not clear in the rest of the function.

func (r *Inputer) processStateBefore(
ctx context.Context,
input *api.InputRoomEvent,
missingPrev bool,
) (historyVisibility string, rejectionErr error, err error) {
historyVisibility = "joined" // Default to restrictive.
event := input.Event.Unwrap()
var stateBeforeEvent []*gomatrixserverlib.Event
if input.HasState {
// If we're overriding the state then we need to go and retrieve
// them from the database. It's a hard error if they are missing.
stateEvents, err := r.DB.EventsFromIDs(ctx, input.StateEventIDs)
if err != nil {
return "", nil, fmt.Errorf("r.DB.EventsFromIDs: %w", err)
}
stateBeforeEvent = make([]*gomatrixserverlib.Event, 0, len(stateEvents))
for _, entry := range stateEvents {
stateBeforeEvent = append(stateBeforeEvent, entry.Event)
}
} else if missingPrev {
// If we're missing prev events and still failed to fetch them
// before then we're stuck.
rejectionErr = fmt.Errorf("event %q should have prev events", event.EventID())
return
} else if event.Type() != gomatrixserverlib.MRoomCreate && len(event.PrevEventIDs()) > 0 {
neilalexander marked this conversation as resolved.
Show resolved Hide resolved
// For all non-create events, there must be prev events, so we'll
// ask the query API for the relevant tuples needed for auth. We
// will include the history visibility here even though we don't
// actually need it for auth, because we want to send it in the
// output events.
tuplesNeeded := gomatrixserverlib.StateNeededForAuth([]*gomatrixserverlib.Event{event}).Tuples()
tuplesNeeded = append(tuplesNeeded, gomatrixserverlib.StateKeyTuple{
EventType: gomatrixserverlib.MRoomHistoryVisibility,
StateKey: "",
})
stateBeforeReq := &api.QueryStateAfterEventsRequest{
RoomID: event.RoomID(),
PrevEventIDs: event.PrevEventIDs(),
StateToFetch: tuplesNeeded,
}
stateBeforeRes := &api.QueryStateAfterEventsResponse{}
if err := r.Queryer.QueryStateAfterEvents(ctx, stateBeforeReq, stateBeforeRes); err != nil {
return "", nil, fmt.Errorf("r.Queryer.QueryStateAfterEvents: %w", err)
}
switch {
case !stateBeforeRes.RoomExists:
rejectionErr = fmt.Errorf("room %q does not exist", event.RoomID())
return
case !stateBeforeRes.PrevEventsExist:
rejectionErr = fmt.Errorf("prev events of %q are not known", event.EventID())
return
default:
stateBeforeEvent = gomatrixserverlib.UnwrapEventHeaders(stateBeforeRes.StateEvents)
}
} else if event.Type() != gomatrixserverlib.MRoomCreate && len(event.PrevEventIDs()) == 0 {
// Non-create events that don't have any prev event IDs are
// impossible in theory, so reject them.
rejectionErr = fmt.Errorf("event %q should have prev events", event.EventID())
return
}
if rejectionErr == nil {
neilalexander marked this conversation as resolved.
Show resolved Hide resolved
// If we haven't rejected the event for some other reason by now,
// see whether the event is allowed against the state at the time.
stateBeforeAuth := gomatrixserverlib.NewAuthEvents(stateBeforeEvent)
if rejectionErr = gomatrixserverlib.Allowed(event, &stateBeforeAuth); rejectionErr != nil {
return
}
}
// Work out what the history visibility was at the time of the
// event.
for _, event := range stateBeforeEvent {
if event.Type() != gomatrixserverlib.MRoomHistoryVisibility || !event.StateKeyEquals("") {
continue
}
if hisVis, err := event.HistoryVisibility(); err == nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if there is no history visibility event? It's not clear if you intend to return the empty string. Only at the call-site does it seem you set a default, perhaps that should be here instead so you can guarantee you always return the history visibility?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default is actually set at the call site (because we don't know if we'll hit processStateBefore or not) and at the start of processStateBefore so that it always returns something.

historyVisibility = hisVis
break
}
}
return
}

// fetchAuthEvents will check to see if any of the
// auth events specified by the given event are unknown. If they are
// then we will go off and request them from the federation and then
Expand Down
38 changes: 21 additions & 17 deletions roomserver/internal/input/input_latest_events.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func (r *Inputer) updateLatestEvents(
sendAsServer string,
transactionID *api.TransactionID,
rewritesState bool,
historyVisibility string,
) (err error) {
span, ctx := opentracing.StartSpanFromContext(ctx, "updateLatestEvents")
defer span.Finish()
Expand All @@ -69,15 +70,16 @@ func (r *Inputer) updateLatestEvents(
defer sqlutil.EndTransactionWithCheck(updater, &succeeded, &err)

u := latestEventsUpdater{
ctx: ctx,
api: r,
updater: updater,
roomInfo: roomInfo,
stateAtEvent: stateAtEvent,
event: event,
sendAsServer: sendAsServer,
transactionID: transactionID,
rewritesState: rewritesState,
ctx: ctx,
api: r,
updater: updater,
roomInfo: roomInfo,
stateAtEvent: stateAtEvent,
event: event,
sendAsServer: sendAsServer,
transactionID: transactionID,
rewritesState: rewritesState,
historyVisibility: historyVisibility,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unclear when this is set:

  • When the latest event is a history visibility event
  • For all events, what the current (before the event) history visibility is.

Needs comments or better yet, a better variable name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is always the history visibility before the event in question, not after it, even if it's a history visibility event. I've added a comment to that effect.

}

if err = u.doUpdateLatestEvents(); err != nil {
Expand Down Expand Up @@ -117,8 +119,9 @@ type latestEventsUpdater struct {
stateBeforeEventRemoves []types.StateEntry
stateBeforeEventAdds []types.StateEntry
// The snapshots of current state before and after processing this event
oldStateNID types.StateSnapshotNID
newStateNID types.StateSnapshotNID
oldStateNID types.StateSnapshotNID
newStateNID types.StateSnapshotNID
historyVisibility string
}

func (u *latestEventsUpdater) doUpdateLatestEvents() error {
Expand Down Expand Up @@ -365,12 +368,13 @@ func (u *latestEventsUpdater) makeOutputNewRoomEvent() (*api.OutputEvent, error)
}

ore := api.OutputNewRoomEvent{
Event: u.event.Headered(u.roomInfo.RoomVersion),
RewritesState: u.rewritesState,
LastSentEventID: u.lastEventIDSent,
LatestEventIDs: latestEventIDs,
TransactionID: u.transactionID,
SendAsServer: u.sendAsServer,
Event: u.event.Headered(u.roomInfo.RoomVersion),
RewritesState: u.rewritesState,
LastSentEventID: u.lastEventIDSent,
LatestEventIDs: latestEventIDs,
TransactionID: u.transactionID,
SendAsServer: u.sendAsServer,
HistoryVisibility: u.historyVisibility,
}

eventIDMap, err := u.stateEventMap()
Expand Down