Filters don't work

[turt2live]

I tested with this filter:

{
  "presence": {
    "limit": 0,
    "types": [
      ""
    ]
  },
  "account_data": {
    "limit": 1,
    "not_types": [
      ""
    ]
  },
  "room": {
    "rooms": [
      "!a:b"
    ]
  }
}

I expected a very small response, particularly because I filter almost everything out. However, I end up getting a full sync response (with all my rooms). I also shouldn't have to supply the types or not_types (those are me trying to figure out why the limit doesn't work), and rooms: [] should surely prevent all rooms.

[KB1RD]

I also had trouble with the "limit" filter (which returned everything) and the "types" filter in rooms.account_data and that seemed to filter nothing. The "senders" filter does work and I have been using this to remove fields from my search results:

{
      presence: {senders: []},
      room: {
        ephemeral: {senders: []},
        state: {senders: []},
        timeline: {senders: []},
        account_data: {types: ["m.tag"]},
      },
      account_data: {senders: []},
    }
}

This gives me only the account data for my rooms, but I still have account_data of types other than m.tag.

[turt2live]

Here's the full range of my testing:

$ curl -s -X POST -H "Authorization: Bearer ${token}" -H "Content-Type: application/json" -d '{"presence":{"limit":0,"types":[""]},"account_data":{"limit":1,"not_types":[""]},"room":{"rooms":["!a:b"]}}' 'https://dev.t2host.io/_matrix/client/r0/user/@abc123:dev.t2host.io/filter'
{
    "filter_id": "1"
}


$ curl -s -X GET -H "Authorization: Bearer ${token}" 'https://dev.t2host.io/_matrix/client/r0/sync?filter=1'
{
    "account_data": {
        "events": [
            {
                "content": { ... trimmed for brevity ... },
                "type": "m.push_rules"
            },
            {
                "content": {
                    "custom": "account data"
                },
                "type": "io.t2l.test"
            },
            {
                "content": {
                    "@travis:dev.t2host.io": [
                        "!bsdbojVPKxLEcPHjrk:dev.t2host.io"
                    ]
                },
                "type": "m.direct"
            }
        ]
    },
    "device_lists": {
        "changed": [],
        "left": []
    },
    "device_one_time_keys_count": {
        "signed_curve25519": 50
    },
    "groups": {
        "invite": {},
        "join": {},
        "leave": {}
    },
    "next_batch": "s4836_89841_125_454_728_9_1_512_5",
    "presence": {
        "events": []
    },
    "rooms": {
        "invite": {},
        "join": {
            "!OzNadulyJcsaNNWAEF:dev.t2host.io": {
                "account_data": {
                    "events": [
                        {
                            "content": {
                                "event_id": "$1544827004300TRERE:dev.t2host.io"
                            },
                            "type": "m.fully_read"
                        }
                    ]
                },
                "ephemeral": {
                    "events": []
                },
                "state": {
                    "events": []
                },
                "summary": {},
                "timeline": {
                    "events": [],
                    "limited": false,
                    "prev_batch": "s4836_89841_125_454_728_9_1_512_5"
                },
                "unread_notifications": {}
            },
            "!bsdbojVPKxLEcPHjrk:dev.t2host.io": {
                "account_data": {
                    "events": [
                        {
                            "content": {
                                "event_id": "$1544827012307goBjF:dev.t2host.io"
                            },
                            "type": "m.fully_read"
                        }
                    ]
                },
                "ephemeral": {
                    "events": []
                },
                "state": {
                    "events": []
                },
                "summary": {},
                "timeline": {
                    "events": [],
                    "limited": false,
                    "prev_batch": "s4836_89841_125_454_728_9_1_512_5"
                },
                "unread_notifications": {}
            }
        },
        "leave": {}
    },
    "to_device": {
        "events": []
    }
}

[KB1RD]

Ok, so I think I've figured out the issues with broken account_data in the room object:
In synapse/handlers/sync.py on lines 1671-1673, the variable account_data is set from the filtered results of the variable account_data_events:

    account_data = sync_config.filter_collection.filter_room_account_data(
        account_data_events
     )

However, on line 1718, the account_data_events variable is used instead of the filtered version. I suppose it would make sense to change the account_data assignment to assign to the original account_data_events:

    account_data_events = sync_config.filter_collection.filter_room_account_data(
        account_data_events
    )

Now, this only fixes part of the problem, but still, it's a start. Let me know if I'm wrong about any of this!

EDIT: You'd also have to change line 1677-1678:

    if not (always_include or batch or account_data_events or ephemeral or full_state):
        return

[hawkowl]

@turt2live @KB1RD @richvdh Can this be closed out with #4309?

[richvdh]

I believe #4309 only fixes part of it. It would be good to update this issue (or possibly better yet, open a new one) with what is left, though.

[turt2live]

sorry, I missed the comments from December ._.

I think there's still more - will track down the problems and open issues. Do people prefer checklists or distinct numbers?

[KB1RD]

This appears to be mainly isolated to limit filtering, which works as expected only on the timeline.

[richvdh]

This appears to be mainly isolated to limit filtering, which works as expected only on the timeline.

Closing in favour of #7306 then