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

MUC: Return better error on illegal nick changes #4278

Open
lovetox opened this issue Sep 11, 2024 · 3 comments
Open

MUC: Return better error on illegal nick changes #4278

lovetox opened this issue Sep 11, 2024 · 3 comments

Comments

@lovetox
Copy link

lovetox commented Sep 11, 2024

Environment

  • ejabberd version: 24.7

Errors from error.log/crash.log

<presence xmlns="jabber:client" id="2e61e2e0-b181-45a8-bca1-fa8b70803ffb" from="[email protected]/lovetox2🔫" type="error">
  <c xmlns="http://jabber.org/protocol/caps" hash="sha-1" node="https://gajim.org" ver="oHb81TBTQlH4TRHlgbyUyVWVAIo=" />
  <error type="modify">
    <bad-request xmlns="urn:ietf:params:xml:ns:xmpp-stanzas" />
    <text xml:lang="en" xmlns="urn:ietf:params:xml:ns:xmpp-stanzas">Bad value of attribute 'to' in tag &lt;presence/&gt; qualified by namespace 'jabber:client'</text>
</error>
</presence>

Bug description

the <text> element should give a error message that the client can show to the user.
This error will not be useful to the average user.

You could argue that the client should validate the resource before sending it, and in fact the client here in question does it. The problem is that with precis and stringprep, two conflicting standards are in use, and the client cannot find out what the server supports.

@badlop
Copy link
Member

badlop commented Sep 20, 2024

This error message seems to be generated in xmpp library, rfc6120.erl and the actual text is in xmpp_codec.erl. All that source code is automatically generated from xmpp_codec.spec, which precisely describes how the presence stanza must be, and complains precisely about the exact problem when decoding it.

If the error message is so technically precise that it is almost useless to the potential destination human at the end of the line... then maybe the corresponding ejabberd module could detect that error before returning it to the client, and replace the text with a more suitable one?

@lovetox
Copy link
Author

lovetox commented Sep 20, 2024

im asking myself if its even feasible to change this on the server.

i guess you have some base stanza parsing before the stanza hits any module, and there it discovers the to attribute is invalid.
seems complicated to still route the stanza through various modules, and discover its a nick change.

Not sure if this is something you normally do.

of course the other solution is here, as the client triggers the nick change, it could track also the answer, and trying to interpret the error.

@processone processone deleted a comment from Neustradamus Oct 21, 2024
@badlop
Copy link
Member

badlop commented Oct 21, 2024

Right, the stanza is parsed long before reaching mod_muc, so the error message cannot explain explicitly anything about the room nickname.

Furthermore: even if that were possible and done for this specific case you mentioned, then there would be many other cases where the user can introduce invalid data, that clients may not verify, and the server would also report with a generic error message...

I guess we can close this issue as "not planned: won't fix"

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

No branches or pull requests

2 participants