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

Create pushrules with non-ascii ruleId fails on Python 2 #4165

Closed
spantaleev opened this issue Nov 8, 2018 · 2 comments
Closed

Create pushrules with non-ascii ruleId fails on Python 2 #4165

spantaleev opened this issue Nov 8, 2018 · 2 comments
Assignees

Comments

@spantaleev
Copy link
Contributor

Creating a new keyword notification seems to fail on Python 2 when the keyword contains non-ASCII text.

When trying to add a keyword called тест (cyrillic), Riot would send a PUT request with a payload of {"actions":["notify",{"set_tweak":"sound","value":"default"},{"set_tweak":"highlight"}],"pattern":"тест"} to /_matrix/client/r0/pushrules/global/content/%D1%82%D0%B5%D1%81%D1%82.

This fails with the following exception:

File "/usr/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 1418, in _inlineCallbacks
result = g.send(result)
File "/usr/local/lib/python2.7/site-packages/synapse/http/server.py", line 316, in _async_render
callback_return = yield callback(request, **kwargs)
File "/usr/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 1613, in unwindGenerator
return _cancellableInlineCallbacks(gen)
File "/usr/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 1529, in _cancellableInlineCallbacks
_inlineCallbacks(None, g, status)
--- <exception caught here> ---
File "/usr/local/lib/python2.7/site-packages/synapse/http/server.py", line 81, in wrapped_request_handler
yield h(self, request)
File "/usr/local/lib/python2.7/site-packages/synapse/http/server.py", line 316, in _async_render
callback_return = yield callback(request, **kwargs)
File "/usr/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 1418, in _inlineCallbacks
result = g.send(result)
File "/usr/local/lib/python2.7/site-packages/synapse/rest/client/v1/push_rule.py", line 45, in on_PUT
spec = _rule_spec_from_path(request.postpath)
File "/usr/local/lib/python2.7/site-packages/synapse/rest/client/v1/push_rule.py", line 212, in _rule_spec_from_path
rule_id = path[0].decode('ascii')
exceptions.UnicodeDecodeError: 'ascii' codec can't decode byte 0xd1 in position 0: ordinal not in range(128)

Looks like Riot is using the new keyword for both the pattern (naturally..) and for the actual ruleId.

I don't know whether the {ruleId} can be anything according to the spec.. If it should be a safe ASCII string, then this may be a bug in Riot. Still, it's a bug in Synapse too, because failing with an exception doesn't sound ideal.

Looks like this is a regression since 02aa418 (#3823) / Synapse 0.33.5.

I'm reproducing it on Synapse 0.33.8 (Python 2). I haven't tested whether it works fine in Python 3.

@hawkowl
Copy link
Contributor

hawkowl commented Nov 9, 2018

Thanks for the report @spantaleev, I think I know what the problem is.

(For interest: it's because Twisted unescapes the path before giving it to us, rather than giving us the URL escaped version, which is ASCII safe. My bad.)

hawkowl added a commit that referenced this issue Nov 19, 2018
Features
--------

- Include flags to optionally add `m.login.terms` to the registration flow when consent tracking is enabled.
([\#4004](#4004), [\#4133](#4133),
[\#4142](#4142), [\#4184](#4184))
- Support for replacing rooms with new ones ([\#4091](#4091), [\#4099](#4099),
[\#4100](#4100), [\#4101](#4101))

Bugfixes
--------

- Fix exceptions when using the email mailer on Python 3. ([\#4095](#4095))
- Fix e2e key backup with more than 9 backup versions ([\#4113](#4113))
- Searches that request profile info now no longer fail with a 500. ([\#4122](#4122))
- fix return code of empty key backups ([\#4123](#4123))
- If the typing stream ID goes backwards (as on a worker when the master restarts), the worker's typing handler will no longer erroneously report rooms containing new
typing events. ([\#4127](#4127))
- Fix table lock of device_lists_remote_cache which could freeze the application ([\#4132](#4132))
- Fix exception when using state res v2 algorithm ([\#4135](#4135))
- Generating the user consent URI no longer fails on Python 3. ([\#4140](#4140),
[\#4163](#4163))
- Loading URL previews from the DB cache on Postgres will no longer cause Unicode type errors when responding to the request, and URL previews will no longer fail if
the remote server returns a Content-Type header with the chartype in quotes. ([\#4157](#4157))
- The hash_password script now works on Python 3. ([\#4161](#4161))
- Fix noop checks when updating device keys, reducing spurious device list update notifications. ([\#4164](#4164))

Deprecations and Removals
-------------------------

- The disused and un-specced identicon generator has been removed. ([\#4106](#4106))
- The obsolete and non-functional /pull federation endpoint has been removed. ([\#4118](#4118))
- The deprecated v1 key exchange endpoints have been removed. ([\#4119](#4119))
- Synapse will no longer fetch keys using the fallback deprecated v1 key exchange method and will now always use v2.
([\#4120](#4120))

Internal Changes
----------------

- Fix build of Docker image with docker-compose ([\#3778](#3778))
- Delete unreferenced state groups during history purge ([\#4006](#4006))
- The "Received rdata" log messages on workers is now logged at DEBUG, not INFO. ([\#4108](#4108))
- Reduce replication traffic for device lists ([\#4109](#4109))
- Fix `synapse_replication_tcp_protocol_*_commands` metric label to be full command name, rather than just the first character
([\#4110](#4110))
- Log some bits about room creation ([\#4121](#4121))
- Fix `tox` failure on old systems ([\#4124](#4124))
- Add STATE_V2_TEST room version ([\#4128](#4128))
- Clean up event accesses and tests ([\#4137](#4137))
- The default logging config will now set an explicit log file encoding of UTF-8. ([\#4138](#4138))
- Add helpers functions for getting prev and auth events of an event ([\#4139](#4139))
- Add some tests for the HTTP pusher. ([\#4149](#4149))
- add purge_history.sh and purge_remote_media.sh scripts to contrib/ ([\#4155](#4155))
- HTTP tests have been refactored to contain less boilerplate. ([\#4156](#4156))
- Drop incoming events from federation for unknown rooms ([\#4165](#4165))
@richvdh
Copy link
Member

richvdh commented Dec 4, 2018

fixed in #4248

@richvdh richvdh closed this as completed Dec 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants