Skip to content

Commit 540062a

Browse files
S7evinKS7evinK
committed
PR review comments, let IRoomVersion handle send leave
1 parent 5d099f9 commit 540062a

File tree

3 files changed

+57
-59
lines changed

3 files changed

+57
-59
lines changed

‎eventversion.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package gomatrixserverlib
22

33
import (
4+
"context"
45
"fmt"
56

67
"github.com/matrix-org/gomatrixserverlib/spec"
@@ -30,6 +31,8 @@ type IRoomVersion interface {
3031
NewEventFromUntrustedJSON(eventJSON []byte) (result PDU, err error)
3132
NewEventBuilder() *EventBuilder
3233
NewEventBuilderFromProtoEvent(pe *ProtoEvent) *EventBuilder
34+
35+
HandleSendLeave(ctx context.Context, event PDU, origin spec.ServerName, eventID, roomID string, querier CurrentStateQuerier, verifier JSONVerifier) (PDU, error)
3336
}
3437

3538
// StateResAlgorithm refers to a version of the state resolution algorithm.
@@ -112,6 +115,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
112115
allowKnockingInEventAuth: KnocksForbidden,
113116
allowRestrictedJoinsInEventAuth: NoRestrictedJoins,
114117
requireIntegerPowerLevels: false,
118+
handleSendLeaveFunc: handleSendLeave,
115119
},
116120
RoomVersionV2: RoomVersionImpl{
117121
ver: RoomVersionV2,
@@ -126,6 +130,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
126130
allowKnockingInEventAuth: KnocksForbidden,
127131
allowRestrictedJoinsInEventAuth: NoRestrictedJoins,
128132
requireIntegerPowerLevels: false,
133+
handleSendLeaveFunc: handleSendLeave,
129134
},
130135
RoomVersionV3: RoomVersionImpl{
131136
ver: RoomVersionV3,
@@ -140,6 +145,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
140145
allowKnockingInEventAuth: KnocksForbidden,
141146
allowRestrictedJoinsInEventAuth: NoRestrictedJoins,
142147
requireIntegerPowerLevels: false,
148+
handleSendLeaveFunc: handleSendLeave,
143149
},
144150
RoomVersionV4: RoomVersionImpl{
145151
ver: RoomVersionV4,
@@ -154,6 +160,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
154160
allowKnockingInEventAuth: KnocksForbidden,
155161
allowRestrictedJoinsInEventAuth: NoRestrictedJoins,
156162
requireIntegerPowerLevels: false,
163+
handleSendLeaveFunc: handleSendLeave,
157164
},
158165
RoomVersionV5: RoomVersionImpl{
159166
ver: RoomVersionV5,
@@ -168,6 +175,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
168175
allowKnockingInEventAuth: KnocksForbidden,
169176
allowRestrictedJoinsInEventAuth: NoRestrictedJoins,
170177
requireIntegerPowerLevels: false,
178+
handleSendLeaveFunc: handleSendLeave,
171179
},
172180
RoomVersionV6: RoomVersionImpl{
173181
ver: RoomVersionV6,
@@ -182,6 +190,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
182190
allowKnockingInEventAuth: KnocksForbidden,
183191
allowRestrictedJoinsInEventAuth: NoRestrictedJoins,
184192
requireIntegerPowerLevels: false,
193+
handleSendLeaveFunc: handleSendLeave,
185194
},
186195
RoomVersionV7: RoomVersionImpl{
187196
ver: RoomVersionV7,
@@ -196,6 +205,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
196205
allowKnockingInEventAuth: KnockOnly,
197206
allowRestrictedJoinsInEventAuth: NoRestrictedJoins,
198207
requireIntegerPowerLevels: false,
208+
handleSendLeaveFunc: handleSendLeave,
199209
},
200210
RoomVersionV8: RoomVersionImpl{
201211
ver: RoomVersionV8,
@@ -210,6 +220,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
210220
allowKnockingInEventAuth: KnockOnly,
211221
allowRestrictedJoinsInEventAuth: RestrictedOnly,
212222
requireIntegerPowerLevels: false,
223+
handleSendLeaveFunc: handleSendLeave,
213224
},
214225
RoomVersionV9: RoomVersionImpl{
215226
ver: RoomVersionV9,
@@ -224,6 +235,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
224235
allowKnockingInEventAuth: KnockOnly,
225236
allowRestrictedJoinsInEventAuth: RestrictedOnly,
226237
requireIntegerPowerLevels: false,
238+
handleSendLeaveFunc: handleSendLeave,
227239
},
228240
RoomVersionV10: RoomVersionImpl{
229241
ver: RoomVersionV10,
@@ -238,6 +250,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
238250
allowKnockingInEventAuth: KnockOrKnockRestricted,
239251
allowRestrictedJoinsInEventAuth: RestrictedOrKnockRestricted,
240252
requireIntegerPowerLevels: true,
253+
handleSendLeaveFunc: handleSendLeave,
241254
},
242255
"org.matrix.msc3667": RoomVersionImpl{ // based on room version 7
243256
ver: RoomVersion("org.matrix.msc3667"),
@@ -252,6 +265,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
252265
allowKnockingInEventAuth: KnockOnly,
253266
allowRestrictedJoinsInEventAuth: NoRestrictedJoins,
254267
requireIntegerPowerLevels: true,
268+
handleSendLeaveFunc: handleSendLeave,
255269
},
256270
"org.matrix.msc3787": RoomVersionImpl{ // roughly, the union of v7 and v9
257271
ver: RoomVersion("org.matrix.msc3787"),
@@ -266,6 +280,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
266280
allowKnockingInEventAuth: KnockOrKnockRestricted,
267281
allowRestrictedJoinsInEventAuth: RestrictedOrKnockRestricted,
268282
requireIntegerPowerLevels: false,
283+
handleSendLeaveFunc: handleSendLeave,
269284
},
270285
}
271286

@@ -334,6 +349,7 @@ type RoomVersionImpl struct {
334349
powerLevelsIncludeNotifications bool
335350
requireIntegerPowerLevels bool
336351
stable bool
352+
handleSendLeaveFunc func(ctx context.Context, event PDU, origin spec.ServerName, eventID, roomID string, querier CurrentStateQuerier, verifier JSONVerifier) (PDU, error)
337353
}
338354

339355
func (v RoomVersionImpl) Version() RoomVersion {
@@ -431,6 +447,17 @@ func (v RoomVersionImpl) RedactEventJSON(eventJSON []byte) ([]byte, error) {
431447
return v.redactionAlgorithm(eventJSON)
432448
}
433449

450+
// HandleSendLeave handles requests to `/send_leave`
451+
func (v RoomVersionImpl) HandleSendLeave(ctx context.Context,
452+
event PDU,
453+
origin spec.ServerName,
454+
eventID, roomID string,
455+
querier CurrentStateQuerier,
456+
verifier JSONVerifier,
457+
) (PDU, error) {
458+
return v.handleSendLeaveFunc(ctx, event, origin, eventID, roomID, querier, verifier)
459+
}
460+
434461
func (v RoomVersionImpl) NewEventFromTrustedJSON(eventJSON []byte, redacted bool) (result PDU, err error) {
435462
return newEventFromTrustedJSON(eventJSON, redacted, v)
436463
}

‎handleleave.go

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,11 @@ type CurrentStateQuerier interface {
106106
CurrentStateEvent(ctx context.Context, roomID spec.RoomID, eventType string, stateKey string) (PDU, error)
107107
}
108108

109-
// HandleSendLeave handles requests to `/send_leave
110-
// Returns the parsed event and an error.
111-
func HandleSendLeave(ctx context.Context,
112-
requestContent []byte,
109+
// handleSendLeave handles requests to `/send_leave
110+
// Returns the parsed event or an error.
111+
func handleSendLeave(ctx context.Context,
112+
event PDU,
113113
origin spec.ServerName,
114-
roomVersion RoomVersion,
115114
eventID, roomID string,
116115
querier CurrentStateQuerier,
117116
verifier JSONVerifier,
@@ -122,21 +121,6 @@ func HandleSendLeave(ctx context.Context,
122121
return nil, err
123122
}
124123

125-
verImpl, err := GetRoomVersion(roomVersion)
126-
if err != nil {
127-
return nil, spec.UnsupportedRoomVersion(fmt.Sprintf("QueryRoomVersionForRoom returned unknown version: %s", roomVersion))
128-
}
129-
130-
// Decode the event JSON from the request.
131-
event, err := verImpl.NewEventFromUntrustedJSON(requestContent)
132-
switch err.(type) {
133-
case BadJSONError:
134-
return nil, spec.BadJSON(err.Error())
135-
case nil:
136-
default:
137-
return nil, spec.NotJSON("The request body could not be decoded into valid JSON. " + err.Error())
138-
}
139-
140124
// Check that the room ID is correct.
141125
if (event.RoomID()) != roomID {
142126
return nil, spec.BadJSON("The room ID in the request path must match the room ID in the leave event JSON")
@@ -190,14 +174,15 @@ func HandleSendLeave(ctx context.Context,
190174
}
191175

192176
// Check that the event is signed by the server sending the request.
193-
redacted, err := verImpl.RedactEventJSON(event.JSON())
177+
resultEvent := event
178+
event.Redact()
194179
if err != nil {
195180
util.GetLogger(ctx).WithError(err).Errorf("unable to redact event")
196181
return nil, spec.BadJSON("The event JSON could not be redacted")
197182
}
198183
verifyRequests := []VerifyJSONRequest{{
199184
ServerName: sender.Domain(),
200-
Message: redacted,
185+
Message: event.JSON(),
201186
AtTS: event.OriginServerTS(),
202187
StrictValidityChecking: true,
203188
}}
@@ -220,5 +205,5 @@ func HandleSendLeave(ctx context.Context,
220205
return nil, spec.BadJSON("The membership in the event content must be set to leave")
221206
}
222207

223-
return event, nil
208+
return resultEvent, nil
224209
}

‎handleleave_test.go

Lines changed: 22 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -242,14 +242,14 @@ func (v *noopJSONVerifier) VerifyJSONs(ctx context.Context, requests []VerifyJSO
242242

243243
func TestHandleSendLeave(t *testing.T) {
244244
type args struct {
245-
ctx context.Context
246-
requestContent []byte
247-
origin spec.ServerName
248-
roomVersion RoomVersion
249-
eventID string
250-
roomID string
251-
querier CurrentStateQuerier
252-
verifier JSONVerifier
245+
ctx context.Context
246+
event PDU
247+
origin spec.ServerName
248+
roomVersion RoomVersion
249+
eventID string
250+
roomID string
251+
querier CurrentStateQuerier
252+
verifier JSONVerifier
253253
}
254254

255255
_, sk, err := ed25519.GenerateKey(rand.Reader)
@@ -318,79 +318,65 @@ func TestHandleSendLeave(t *testing.T) {
318318
}{
319319
{
320320
name: "invalid roomID",
321-
args: args{roomID: "@notvalid:localhost"},
322-
wantErr: assert.Error,
323-
},
324-
{
325-
name: "invalid room version",
326-
args: args{roomID: "!notvalid:localhost", roomVersion: "-1"},
327-
wantErr: assert.Error,
328-
},
329-
{
330-
name: "invalid content body",
331-
args: args{roomID: "!notvalid:localhost", roomVersion: RoomVersionV1, requestContent: []byte("{")},
332-
wantErr: assert.Error,
333-
},
334-
{
335-
name: "not canonical JSON",
336-
args: args{roomID: "!notvalid:localhost", roomVersion: RoomVersionV10, requestContent: []byte(`{"int":9007199254740992}`)}, // number to large, not canonical json
321+
args: args{roomID: "@notvalid:localhost", roomVersion: RoomVersionV10},
337322
wantErr: assert.Error,
338323
},
339324
{
340325
name: "wrong roomID in request",
341-
args: args{roomID: "!notvalid:localhost", roomVersion: RoomVersionV10, requestContent: createEvent.JSON()},
326+
args: args{roomID: "!notvalid:localhost", roomVersion: RoomVersionV10, event: createEvent},
342327
wantErr: assert.Error,
343328
},
344329
{
345330
name: "wrong eventID in request",
346-
args: args{roomID: "!valid:localhost", roomVersion: RoomVersionV10, requestContent: createEvent.JSON()},
331+
args: args{roomID: "!valid:localhost", roomVersion: RoomVersionV10, event: createEvent},
347332
wantErr: assert.Error,
348333
},
349334
{
350335
name: "empty statekey",
351-
args: args{roomID: "!valid:localhost", roomVersion: RoomVersionV10, eventID: createEvent.EventID(), requestContent: createEvent.JSON()},
336+
args: args{roomID: "!valid:localhost", roomVersion: RoomVersionV10, eventID: createEvent.EventID(), event: createEvent},
352337
wantErr: assert.Error,
353338
},
354339
{
355340
name: "wrong request origin",
356-
args: args{roomID: "!valid:localhost", roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), requestContent: leaveEvent.JSON()},
341+
args: args{roomID: "!valid:localhost", roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), event: leaveEvent},
357342
wantErr: assert.Error,
358343
},
359344
{
360345
name: "never joined the room no-ops",
361-
args: args{roomID: "!valid:localhost", querier: dummyQuerier{}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), requestContent: leaveEvent.JSON()},
346+
args: args{roomID: "!valid:localhost", querier: dummyQuerier{}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), event: leaveEvent},
362347
wantErr: assert.NoError,
363348
},
364349
{
365350
name: "already left the room no-ops",
366-
args: args{roomID: "!valid:localhost", querier: dummyQuerier{pdu: leaveEvent}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), requestContent: leaveEvent.JSON()},
351+
args: args{roomID: "!valid:localhost", querier: dummyQuerier{pdu: leaveEvent}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), event: leaveEvent},
367352
wantErr: assert.NoError,
368353
},
369354
{
370355
name: "JSON validation fails",
371-
args: args{ctx: context.Background(), roomID: "!valid:localhost", querier: dummyQuerier{pdu: createEvent}, verifier: &noopJSONVerifier{err: fmt.Errorf("err")}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), requestContent: leaveEvent.JSON()},
356+
args: args{ctx: context.Background(), roomID: "!valid:localhost", querier: dummyQuerier{pdu: createEvent}, verifier: &noopJSONVerifier{err: fmt.Errorf("err")}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), event: leaveEvent},
372357
wantErr: assert.Error,
373358
},
374359
{
375360
name: "JSON validation fails 2",
376-
args: args{ctx: context.Background(), roomID: "!valid:localhost", querier: dummyQuerier{pdu: createEvent}, verifier: &noopJSONVerifier{results: []VerifyJSONResult{{Error: fmt.Errorf("err")}}}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), requestContent: leaveEvent.JSON()},
361+
args: args{ctx: context.Background(), roomID: "!valid:localhost", querier: dummyQuerier{pdu: createEvent}, verifier: &noopJSONVerifier{results: []VerifyJSONResult{{Error: fmt.Errorf("err")}}}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), event: leaveEvent},
377362
wantErr: assert.Error,
378363
},
379364
{
380365
name: "membership not set to leave",
381-
args: args{ctx: context.Background(), roomID: "!valid:localhost", querier: dummyQuerier{pdu: createEvent}, verifier: &noopJSONVerifier{results: []VerifyJSONResult{{}}}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: joinEvent.EventID(), requestContent: joinEvent.JSON()},
366+
args: args{ctx: context.Background(), roomID: "!valid:localhost", querier: dummyQuerier{pdu: createEvent}, verifier: &noopJSONVerifier{results: []VerifyJSONResult{{}}}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: joinEvent.EventID(), event: joinEvent},
382367
wantErr: assert.Error,
383368
},
384369
{
385370
name: "membership set to leave",
386-
args: args{ctx: context.Background(), roomID: "!valid:localhost", querier: dummyQuerier{pdu: createEvent}, verifier: &noopJSONVerifier{results: []VerifyJSONResult{{}}}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), requestContent: leaveEvent.JSON()},
371+
args: args{ctx: context.Background(), roomID: "!valid:localhost", querier: dummyQuerier{pdu: createEvent}, verifier: &noopJSONVerifier{results: []VerifyJSONResult{{}}}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), event: leaveEvent},
387372
wantErr: assert.NoError,
388373
},
389374
}
390375
for _, tt := range tests {
391376
t.Run(tt.name, func(t *testing.T) {
392-
_, err := HandleSendLeave(tt.args.ctx, tt.args.requestContent, tt.args.origin, tt.args.roomVersion, tt.args.eventID, tt.args.roomID, tt.args.querier, tt.args.verifier)
393-
if !tt.wantErr(t, err, fmt.Sprintf("HandleSendLeave(%v, %v, %v, %v, %v, %v, %v, %v)", tt.args.ctx, tt.args.requestContent, tt.args.origin, tt.args.roomVersion, tt.args.eventID, tt.args.roomID, tt.args.querier, tt.args.verifier)) {
377+
verImpl := MustGetRoomVersion(tt.args.roomVersion)
378+
_, err := verImpl.HandleSendLeave(tt.args.ctx, tt.args.event, tt.args.origin, tt.args.eventID, tt.args.roomID, tt.args.querier, tt.args.verifier)
379+
if !tt.wantErr(t, err, fmt.Sprintf("handleSendLeave(%v, %v, %v, %v, %v, %v, %v, %v)", tt.args.ctx, tt.args.event, tt.args.origin, tt.args.roomVersion, tt.args.eventID, tt.args.roomID, tt.args.querier, tt.args.verifier)) {
394380
return
395381
}
396382
})

0 commit comments

Comments
 (0)