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

[WIP] Fix and complete server-side JID validation and escaping. #843

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

Conversation

Natureshadow
Copy link
Contributor

No description provided.

@edhelas
Copy link
Member

edhelas commented May 23, 2019

Thanks a lo, I'll review it asap!
You've added a dependency, is it OK regarding the packages (like the Debian one?).

@edhelas edhelas self-assigned this May 23, 2019
@Natureshadow
Copy link
Contributor Author

Natureshadow commented May 23, 2019 via email

Copy link
Member

@edhelas edhelas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot <3
I've put a few comments, mostly formating and cleaning. I'll do a live test of the branch after that to see if everything looks fine.

app/helpers/StringHelper.php Show resolved Hide resolved
app/helpers/StringHelper.php Outdated Show resolved Hide resolved
app/helpers/StringHelper.php Outdated Show resolved Hide resolved
app/helpers/StringHelper.php Show resolved Hide resolved
app/helpers/StringHelper.php Outdated Show resolved Hide resolved
app/helpers/StringHelper.php Outdated Show resolved Hide resolved
app/widgets/Chat/Chat.php Outdated Show resolved Hide resolved
@Natureshadow
Copy link
Contributor Author

Natureshadow commented May 27, 2019

I updated a lot, please review again, @edhelas ☺!

@Natureshadow Natureshadow changed the title Fix and complete server-side JID validation and escaping. [WIP] Fix and complete server-side JID validation and escaping. May 27, 2019
No need to explode all over the place.
The previous code could lead to data from different users being handled for
the wrong UI element.  For example, from the view of this code, the JIDs
"foo-bar@example.com", "foo_bar@example.com", "foo.bar@example.com", which
are all distinct and valid JIds, were handled as one and the same.
Previously, the use of - and _ for sufficēs was mixed, sometimes even for
the same names, and the elements were only matche due to the old, non-unique
cleanupId code turning _ into -.
@edhelas edhelas mentioned this pull request Mar 2, 2021
@edhelas
Copy link
Member

edhelas commented Mar 2, 2021

As I'm not planning to maintain a specifical stringprep dependency from that I'm assuming that the only proper way to do so would be to directly call "idn" (http://www.gnu.org/software/libidn/manual/html_node/Invoking-idn.html) and call it from PHP. I'm already using it during the registration process (see https://api.movim.eu/accounts/register) and it has a complete Nodeprep implementation.

@edhelas edhelas closed this Mar 2, 2021
@edhelas
Copy link
Member

edhelas commented Mar 1, 2023

Let's see if I can revive this PR :)

@edhelas edhelas reopened this Mar 1, 2023
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

3 participants