matrix-org / dendrite

Dendrite is a second-generation Matrix homeserver written in Go!
https://matrix-org.github.io/dendrite/
Apache License 2.0
5.75k stars 676 forks source link

`/sync` with only `timeout=0` acts as if `full_state=true` #2066

Open ShadowJonathan opened 2 years ago

ShadowJonathan commented 2 years ago

Background information

This was found while running Complement, so build information from this Dockerfile also applies.

Description

Steps to reproduce

When creating a complement test for an unrelated functionality, this following snippet reacts differently on dendrite compared to synapse;

    deployment := Deploy(t, b.BlueprintOneToOneRoom)
    defer deployment.Destroy(t)

    alice := deployment.Client(t, "hs1", "@alice:hs1")
    bob := deployment.Client(t, "hs1", "@bob:hs1")

    roomID := alice.CreateRoom(t, map[string]interface{}{
        "preset": "public_chat",
    })

    var since string

    // Get floating current next_batch
    res := alice.MustDoFunc(t, "GET", []string{"_matrix", "client", "r0", "sync"}, client.WithQueries(url.Values{
        "timeout": []string{"0"},
    }))
    body := client.ParseJSON(t, res)
    since = client.GetJSONFieldStr(t, body, "next_batch")

    alice.InviteRoom(t, roomID, bob.UserID)

    bob.SyncUntilInvitedTo(t, roomID)

    // This rejects the invite
    bob.LeaveRoom(t, roomID)

    // Full sync
    res = bob.MustDoFunc(t, "GET", []string{"_matrix", "client", "r0", "sync"}, client.WithQueries(url.Values{
        "timeout": []string{"0"},
        // "full_state": []string{"true"},
        // "since": []string{since},
    }))
    body = client.ParseJSON(t, res)
    jsonRes := gjson.GetBytes(body, "rooms.leave."+client.GjsonEscape(roomID))
    if !jsonRes.Exists() {
        t.Errorf("Bob just rejected an invite, it should show up under 'leave' in a full sync")
    }
    since = client.GetJSONFieldStr(t, body, "next_batch")

On synapse, the last condition (as expected) fails, as it is fetching fresh information with incremental sync since since.

However, on dendrite, this condition passes, dendrite is supplying a state_full=true response while it should be default false, according to spec.


There's an alternate explanation for this, but that means an ambiguity in the spec, see https://github.com/matrix-org/matrix-doc/issues/3537

kegsay commented 2 years ago

as it is fetching fresh information with incremental sync since since.

It isn't:

// "since": []string{since},

It's commented out?

I'm guessing the root cause here is the clarification issue you made on the spec. I don't think left rooms should appear in an initial sync as it will bloat the response size.