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

Cannot send new events into rooms with floating-point (and stringy?) power levels #14060

Closed
DMRobertson opened this issue Oct 5, 2022 · 5 comments · Fixed by #14073
Closed
Assignees
Labels
A-Push Issues related to push/notifications A-Validation 500 (mostly) errors due to lack of event/parameter validation O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Major Major functionality / product severely impaired, no satisfactory workaround. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. X-Regression Something broke which worked on a previous release X-Release-Blocker Must be resolved before making a release Z-Sentry Issue was discovered by looking at Sentry reports on Matrix.org

Comments

@DMRobertson
Copy link
Contributor

DMRobertson commented Oct 5, 2022

Internal users: https://sentry.tools.element.io/organizations/element/issues/34032/?project=2&query=is%3Aunresolved&sort=freq&statsPeriod=14d

Sentry's report is terse here: it does not include local vars from the call stack.

Failed handle request via "RoomSendEventRestServlet": "<XForwardedForRequest at 0x7f8348f30f40 method='PUT' uri='/_matrix/client/r0/rooms/<room ID>/send/m.room.encrypted/<txn ID>' clientproto='HTTP/1.1' site='12101'>"
TypeError: argument 'notification_power_levels': 'float' object cannot be interpreted as an integer
  File "/home/synapse/src/synapse/http/server.py", line 306, in _async_render_wrapper
  File "/home/synapse/src/synapse/http/server.py", line 512, in _async_render
  File "/home/synapse/src/synapse/rest/client/room.py", line 353, in on_POST
  File "/home/synapse/src/synapse/handlers/message.py", line 1020, in create_and_send_nonmember_event
  File "/home/synapse/src/synapse/util/metrics.py", line 113, in measured_func
  File "/home/synapse/src/synapse/handlers/message.py", line 1382, in handle_new_client_event
  File "/home/synapse/env-pyston-poetry/lib/python3.8-pyston2.3/site-packages/twisted/internet/defer.py", line 1660, in _inlineCallbacks
  File "/home/synapse/src/synapse/handlers/message.py", line 1436, in _persist_events
  File "/home/synapse/src/synapse/util/metrics.py", line 113, in measured_func
  File "/home/synapse/src/synapse/push/bulk_push_rule_evaluator.py", line 292, in action_for_event_by_user

The deepest call is

evaluator = PushRuleEvaluator(
_flatten_dict(event),
room_member_count,
sender_power_level,
power_levels.get("notifications", {}),
relations,
self._relations_match_enabled,
)

which invokes the new Rust mechanism for push stuff:

#[pymethods]
impl PushRuleEvaluator {
/// Create a new `PushRuleEvaluator`. See struct docstring for details.
#[new]
pub fn py_new(
flattened_keys: BTreeMap<String, String>,
room_member_count: u64,
sender_power_level: Option<i64>,
notification_power_levels: BTreeMap<String, i64>,
relations: BTreeMap<String, BTreeSet<(String, String)>>,
relation_match_enabled: bool,
) -> Result<Self, Error> {
let body = flattened_keys
.get("content.body")
.cloned()
.unwrap_or_default();
Ok(PushRuleEvaluator {
flattened_keys,
body,
room_member_count,
notification_power_levels,
relations,
relation_match_enabled,
sender_power_level,
})
}

I am guessing that

  • the error message 'float' object cannot be interpreted as an integer comes from Py03's translation layer. (Implement push rule evaluation in Rust. #13838)
  • the power levels event used in that room has floating power levels (and therefore is bogus, but not prohibited until room v10)
  • people are unable to send messages in that room from Synapse if they believe that power levels event to be in the current state

Will investigate a little more.

@DMRobertson DMRobertson added A-Push Issues related to push/notifications A-Validation 500 (mostly) errors due to lack of event/parameter validation S-Major Major functionality / product severely impaired, no satisfactory workaround. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Oct 5, 2022
@DMRobertson DMRobertson self-assigned this Oct 5, 2022
@DMRobertson
Copy link
Contributor Author

matrix=> select json::json->'content'->'notifications' from current_state_events AS cse join event_json using(event_id) where cse.room_id = '<ROOM ID>' and cse.type = 'm.room.power_levels';
    ?column?    
════════════════
 {"room":100.0}
(1 row)

Right. Well that's not good.

@DMRobertson
Copy link
Contributor Author

The event in question was created at 2020-12-29 08:52:34+00 according to its origin_server_ts so MSC3667 doesn't apply.

We now check that the power level values are integers in sufficiently new rooms.

# Reject events with stringy power levels if required by room version
if (
event.type == EventTypes.PowerLevels
and room_version_obj.msc3667_int_only_power_levels
):
for k, v in event.content.items():
if k in {
"users_default",
"events_default",
"state_default",
"ban",
"redact",
"kick",
"invite",
}:
if not isinstance(v, int):
raise SynapseError(400, f"{v!r} must be an integer.")
if k in {"events", "notifications", "users"}:
if not isinstance(v, dict) or not all(
isinstance(v, int) for v in v.values()
):
raise SynapseError(
400,
f"{v!r} must be a dict wherein all the values are integers.",
)

@DMRobertson DMRobertson changed the title Unexplained TypeError because notification_power_levels is a float Cannot send new events into rooms with floating-point (and stringy?) power levels Oct 5, 2022
@DMRobertson DMRobertson added X-Regression Something broke which worked on a previous release Z-Sentry Issue was discovered by looking at Sentry reports on Matrix.org labels Oct 5, 2022
@DMRobertson
Copy link
Contributor Author

No idea how we even got floats in the power levels content in the first place, given that

elif isinstance(value, float):
# Note that Infinity, -Infinity, and NaN are also considered floats.
raise SynapseError(400, "Bad JSON value: float", Codes.BAD_JSON)
exists. But maybe it didn't at the time.

@richvdh
Copy link
Member

richvdh commented Oct 6, 2022

No idea how we even got floats in the power levels content in the first place, given that

elif isinstance(value, float):
# Note that Infinity, -Infinity, and NaN are also considered floats.
raise SynapseError(400, "Bad JSON value: float", Codes.BAD_JSON)

exists. But maybe it didn't at the time.

Note that this is only enforced (at least over federation) for newer room versions.

@DMRobertson
Copy link
Contributor Author

Merged into develop in 44741aa.

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 A-Validation 500 (mostly) errors due to lack of event/parameter validation O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Major Major functionality / product severely impaired, no satisfactory workaround. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. X-Regression Something broke which worked on a previous release X-Release-Blocker Must be resolved before making a release Z-Sentry Issue was discovered by looking at Sentry reports on Matrix.org
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants