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

"default" field in PushRule omitted for user-defined push rules #6636

Closed
Karlinde opened this issue Jan 5, 2020 · 3 comments
Closed

"default" field in PushRule omitted for user-defined push rules #6636

Karlinde opened this issue Jan 5, 2020 · 3 comments
Labels
A-Spec-Compliance places where synapse does not conform to the spec z-bug (Deprecated Label) z-p2 (Deprecated Label)

Comments

@Karlinde
Copy link
Contributor

Karlinde commented Jan 5, 2020

Description

The spec states that for push rules returned from the server, the field default is required. Synapse however seems to omit this field for user-defined rules, leading to deserialization failures for a spec-compliant client that expects it when fetching push rules.

Steps to reproduce

  • Push a user-defined push rule to synapse using PUT /_matrix/client/r0/pushrules/{scope}/{kind}/{ruleId}.
  • Fetch all push rules using GET /_matrix/client/r0/pushrules/.
  • Find the newly created rule in the returned ruleset. It doesn't have a default field.

Since the spec states that the default field is required, I did not expect the server to be allowed to omit it.

Version information

  • Homeserver: private, not federating
  • Platform: Docker in Ubuntu running on a VPS.
@Karlinde
Copy link
Contributor Author

Karlinde commented Jan 6, 2020

Looking at the code, it looks like the issue is that only the built-in rule objects ever get the "default" key set, which seems to happen here.

A simple solution would be to just add rule["default"] = False for each rule in _load_rules().

PR submitted: #6639

@neilisfragile neilisfragile added z-bug (Deprecated Label) A-Spec-Compliance places where synapse does not conform to the spec z-p2 (Deprecated Label) labels Jan 14, 2020
@jplatte
Copy link
Contributor

jplatte commented May 22, 2020

Fixed in #6639?

@clokep
Copy link
Member

clokep commented May 22, 2020

Looks like we never closed this after merging the PR. I'm going to close this, but let us know if there's more to do here!

@clokep clokep closed this as completed May 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Spec-Compliance places where synapse does not conform to the spec z-bug (Deprecated Label) z-p2 (Deprecated Label)
Projects
None yet
Development

No branches or pull requests

4 participants