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

send/{eventType}/{txnId} works if txnId is empty #9340

Closed
erdnaxeli opened this issue Feb 6, 2021 · 4 comments
Closed

send/{eventType}/{txnId} works if txnId is empty #9340

erdnaxeli opened this issue Feb 6, 2021 · 4 comments
Labels
A-Spec-Compliance places where synapse does not conform to the spec S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@erdnaxeli
Copy link
Contributor

Description

The /_matrix/client/r0/rooms/{roomId}/send/{eventType}/{txnId} accepts an empty txnId even if the spec says it is required. Actually is works only once, it seems to use an empty string ("") as the txnId, then work as expected with an already used txnId if you try again.

Steps to reproduce

HS_URL=
ROOM_ID=
ACCESS_TOKEN=
curl "https://$HS_URL/_matrix/client/r0/rooms/$ROOM_ID/send/m.room.message/" -X PUT H 'Content-Type: application/json' -H "Authorization: Bearer $ACCESS_TOKEN" --data-raw '{"msgtype":"m.text","body":"test"}'

Version information

  • Homeserver: cloud.cervoi.se

If not matrix.org:

  • Version: 1.26.0

  • Install method: apt

  • Platform: debian10
@clokep clokep added S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. A-Spec-Compliance places where synapse does not conform to the spec labels Feb 8, 2021
@clokep
Copy link
Member

clokep commented Feb 8, 2021

Thanks! Looks like we should add some additional validation to the transaction ID. Note that there's other endpoints that take a transaction ID, so we'd probably want to create some sort of helper to do this.

The only thing I could find in the spec about this is:

The transaction ID for this event. Clients should generate an ID unique across requests with the same access token; it will be used by the server to ensure idempotency of requests.

Which doesn't really preclude an empty transaction ID, although it seems a bit silly to do that. Was there a tighter definition I'm not seeing?

@Shubhankaroad
Copy link

@clokep is this issue still open? Can i work on this?

@richvdh
Copy link
Member

richvdh commented Dec 8, 2021

Given there's nothing in the spec that says an empty transaction id is forbidden, clients have a right to expect it should work, even if it's a pretty weird thing to want to do. Accordingly, I think this is working correctly at present.

(Related: https://github.com/matrix-org/matrix-doc/issues/666 which notes that there is no grammar specified for txn ids)

@richvdh richvdh closed this as completed Dec 8, 2021
@erdnaxeli
Copy link
Contributor Author

I am not happy with this :D
But I can understand why.

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 S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

4 participants