Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

event_match push rule condition doesn't work on booleans or integers #13465

Open
Johennes opened this issue Aug 5, 2022 · 1 comment
Open
Labels
A-Push Issues related to push/notifications O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Blocks non-critical functionality, workarounds exist. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. Z-Spec-Blocked This change is blocked on specification (e.g. an MSC).

Comments

@Johennes
Copy link

Johennes commented Aug 5, 2022

Description

The event_match condition is evaluated against a flattened variant of the event dictionary that drops all non-string values (see https://github.com/matrix-org/synapse/blob/develop/synapse/push/push_rule_evaluator.py#L345). As a result, it's not possible to match against any boolean or integer JSON values.

As an example, live location sharing uses events of type beacon_info with a "live": true/false value in content to signal whether location sharing was started or ended.

{
  "content": {
    "body": "Live location",
    "live": true,
    "org.matrix.msc3488.asset": {
      "type": "m.self"
    },
    "org.matrix.msc3488.ts": 1659359067708,
    "timeout": 900000
  },
  "origin_server_ts": 1659359069894,
  "sender": "@...",
  "state_key": "@...,
  "type": "org.matrix.msc3672.beacon_info",
  ...
}

Since the value of live is boolean, it's not currently possible to define a push rule that only matches "live": true but not "live": false.

The spec doesn't explicitly state that event_match should only work on strings. It just says the pattern is evaluated against "a field" on the event.

This is a glob pattern match on a field of the event.

Therefore, I think event_match should work on the string-representation of boolean and integer values as well.

Steps to reproduce

Define a push rule that matches on a boolean value, e.g.

{
    "rule_id":"m.rule.live_location_start",
    "default": true,
    "enabled": true,
    "conditions": [
        {
            "kind": "event_match",
            "key": "type",
            "pattern": "*beacon_info",
        },
        {
            "kind": "event_match",
            "key": "content.live",
            "pattern": "true",
        },
        {
            "kind": "event_match",
            "key": "state_key",
            "pattern": "*",
        }
    ],
    "actions": [
        "notify"
    ]
}

Notice how it's not having any effect.

Homeserver

n/a

Synapse Version

Current develop

Installation Method

No response

Platform

n/a

Relevant log output

n/a

Anything else that would be useful to know?

n/a

@H-Shay H-Shay added S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Aug 10, 2022
@anoadragon453 anoadragon453 added A-Push Issues related to push/notifications T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. O-Occasional Affects or can be seen by some users regularly or most users rarely Z-Spec-Blocked This change is blocked on specification (e.g. an MSC). and removed T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Sep 7, 2022
@anoadragon453
Copy link
Member

From the conversation at #13466 (comment), this is blocked on either implementing MSC3862 or MSC3758.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Push Issues related to push/notifications O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Blocks non-critical functionality, workarounds exist. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. Z-Spec-Blocked This change is blocked on specification (e.g. an MSC).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants