Skip to content

Room version 12 tests - #791

Merged
kegsay merged 2 commits into
mainfrom
kegan/v12
Aug 11, 2025
Merged

kegsay merged 2 commits into
mainfrom
kegan/v12

Conversation

@kegsay

@kegsay kegsay commented Aug 11, 2025

Copy link
Copy Markdown
Member

No description provided.

@kegsay
kegsay requested review from a team as code owners August 11, 2025 17:18
@kegsay
kegsay merged commit 495c811 into main Aug 11, 2025
@kegsay
kegsay deleted the kegan/v12 branch August 11, 2025 19:20
Comment thread tests/v12_test.go
return eventIDs
}

func TestMSC4311FullCreateEventOnStrippedState(t *testing.T) {

@MadLittleMods MadLittleMods Apr 23, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this test is mixing up what MSC4311 proposes. Perhaps these were changes to the MSC that came after?

For the client API's like /sync, it only proposes that m.room.create is a required stripped state event.

For the federation API's, alongside requiring m.room.create, it also mandates using the full event PDU format for all events in the invite_room_state/knock_room_state on m.room.member events (in unsigned)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like this is being addressed in #796

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Removing the flawed implementation in Synapse via element-hq/synapse#19723

@MadLittleMods MadLittleMods mentioned this pull request May 4, 2026
1 task done
MadLittleMods added a commit to element-hq/synapse that referenced this pull request Sep 22, 2026
…over federation and always include `m.room.create` event (#19723)

### Background

This PR was originally just trying to remove the flawed [MSC4311](matrix-org/matrix-spec-proposals#4311) partial implementation as client side API's like `/sync` should still use stripped events. But it turns out we were just re-using the client logic for the federation side and things might break if we didn't include the full `m.room.create` event so this PR now introduces MSC4311 support to use full PDU's in the `invite_room_state`/`knock_room_state` in the federation API's.

The flawed implementation was originally introduced in 0eb7252 (no PR I assume because part of Hydra security fix) which was part of [Synapse v1.136.0](https://github.com/element-hq/synapse/blob/7530874a1250d6ad975b39582a784c594d29a505/CHANGES.md#synapse-11360-2025-08-12).

Spawning from reviewing #19722 and noticing that we have [`TestMSC4311FullCreateEventOnStrippedState`](https://github.com/matrix-org/complement/blob/1e2e12eebc1edb27bbf12108ec849a8254b6ddcd/tests/v12_test.go#L1341-L1376) in Complement which already passes even though that test looks [flawed](matrix-org/complement#791 (comment)):

> I think this test is mixing up what [MSC4311](matrix-org/matrix-spec-proposals#4311) proposes. Perhaps these were changes to the MSC that came after?
> 
> For the client API's like `/sync`, it only proposes that `m.room.create` is a required *stripped* state event.
> 
> For the federation API's, alongside requiring `m.room.create`, it also mandates using the full event PDU format for all events in the `invite_room_state`/`knock_room_state` on `m.room.member` events (in `unsigned`)

### What does this PR do?

 1. Always use stripped state for client API's
    1. Remove flawed [MSC4311](matrix-org/matrix-spec-proposals#4311) partial implementation (as explained above)
    1. Sanitize stripped state when we receive events over federation
 1. Use full PDU's when sending `invite_room_state`/`knock_room_state` over federation
 1. Validate PDU's and warn when receiving `invite_room_state`/`knock_room_state` over federation
     1. In the future, we will strictly validate and reject

Complement tests: matrix-org/complement#796

---

Part of #19414
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants