Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefix for ID in send message and Markable stanza #128

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sashati
Copy link

@sashati sashati commented Jan 24, 2021

In this PR, I added a Prefix for ID generation, since some applications use prefix in ID for some logics. Also I added a Markable flag to the Chat struct, then we can embed the Markable stanza in Send message

@SamWhited
Copy link

SamWhited commented Jan 24, 2021

TL;DR — There's no rule that a dog can't play basketball, but that doesn't mean it's actually a good idea.


While I don't think anything in RFC 6120 specifically forbids embedding information in the id attribute (as long as it's still unique and unpredictable, as this is), this isn't a thing anyone should be doing IMO. It's not part of the spec and it's creating a sort of side-channel unstandardized extension point in the protocol. This may seem harmless if it's just your server or client using it, but eventually other entities that haven't read the spec may end up relying on it and it's a way to create unnamespaced defacto extensions.

Instead you should embed your payload properly with a custom namespace, or define a new extension if it's something that you need mirrored with similar rules to IDs (which may be mirrored in IQ and some presence stanzas). I do know of at least one server doing this to embed IQ routing information so that their cluster routing can be stateless, but I don't think this means it's something we should encourage for the reasons I've already stated.

While this doesn't apply in a normative sense to stanza IDs, please see the Security Consideration section of XEP-0359: Unique and Stable Stanza IDs which specifically forbids this for the IDs generated in that spec. The same sorts of considerations likely apply to stanza IDs.

EDIT: if this is something you absolutely must have, maybe it's worth having some sort of "advanced user" API that lets you set your own IDs entirely instead of randomly generating one and mixing it with a prefix? That way people who want to do this have the flexibility to do so, but we don't make it so easy that we're encouraging something that's not really part of the protocol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants