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

Synapse queries the IS's /store-invite endpoint with the wrong content-type #5634

Closed
babolivier opened this issue Jul 7, 2019 · 1 comment
Assignees
Labels
z-bug (Deprecated Label) z-p2 (Deprecated Label)

Comments

@babolivier
Copy link
Contributor

Synapse currently sends requests to /store-invite using the application/x-www-form-urlencoded content-type header: https://github.com/matrix-org/synapse/blob/master/synapse/handlers/room_member.py#L908

The IS spec says:

all API calls use a Content-Type of application/json

It currently works because Sydent supports both application/x-www-form-urlencoded and application/json but this doesn't sound like something we'd want to do.

babolivier added a commit to matrix-org/sydent that referenced this issue Jul 8, 2019
The spec says that /store-invite requests should be done using the 'application/json' mimetype, but currently Sydent chokes on such requests, because it first extracts its params from the request, doing the right thing if the content-type is JSON, but, when trying to compute the dict of substitutions to apply to the email template, looks for them in request.args, which is empty when using 'application/json'.

This bug went unnoticed because Synapse isn't compliant with the spec here and sends that data using the 'application/x-www-form-urlencoded' mimetype. This is tracked in matrix-org/synapse#5634.

This makes Sydent not try to re-extract the params from the request.args, but instead use the ones it has already extracted the right way.
babolivier added a commit to matrix-org/sydent that referenced this issue Jul 8, 2019
The spec says that /store-invite requests should be done using the 'application/json' mimetype, but currently Sydent chokes on such requests, because it first extracts its params from the request, doing the right thing if the content-type is JSON, but, when trying to compute the dict of substitutions to apply to the email template, looks for them in request.args, which is empty when using 'application/json'.

On top of that, json.dumps() outputs a dict which keys and string values are of type 'unicode', which Python 2 doesn't consider to be strings (or at least not the same as 'str'), so the arguments wouldn't be added to the substitutions dict.

This bug went unnoticed because Synapse isn't compliant with the spec here and sends that data using the 'application/x-www-form-urlencoded' mimetype. This is tracked in matrix-org/synapse#5634.

This makes Sydent not try to re-extract the params from the request.args, but instead use the ones it has already extracted the right way. It also changes the type comparison on the params' values, comparing them with six.string_types instead of only the 'str' type.
@neilisfragile neilisfragile added z-bug (Deprecated Label) z-p2 (Deprecated Label) labels Jul 9, 2019
@babolivier babolivier self-assigned this Jul 9, 2019
@babolivier
Copy link
Contributor Author

Fixed on develop

anoadragon453 pushed a commit to matrix-org/sydent that referenced this issue Apr 22, 2020
The spec says that /store-invite requests should be done using the 'application/json' mimetype, but currently Sydent chokes on such requests, because it first extracts its params from the request, doing the right thing if the content-type is JSON, but, when trying to compute the dict of substitutions to apply to the email template, looks for them in request.args, which is empty when using 'application/json'.

On top of that, json.dumps() outputs a dict which keys and string values are of type 'unicode', which Python 2 doesn't consider to be strings (or at least not the same as 'str'), so the arguments wouldn't be added to the substitutions dict.

This bug went unnoticed because Synapse isn't compliant with the spec here and sends that data using the 'application/x-www-form-urlencoded' mimetype. This is tracked in matrix-org/synapse#5634.

This makes Sydent not try to re-extract the params from the request.args, but instead use the ones it has already extracted the right way. It also changes the type comparison on the params' values, comparing them with six.string_types instead of only the 'str' type.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
z-bug (Deprecated Label) z-p2 (Deprecated Label)
Projects
None yet
Development

No branches or pull requests

2 participants