-
-
Notifications
You must be signed in to change notification settings - Fork 248
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
base: master
Are you sure you want to change the base?
Conversation
Thanks a lo, I'll review it asap! |
Hi,
Thanks a lo, I'll review it asap!
You've added a dependency, is it OK regarding the packages (like the Debian one?).
For Debian, I will of course take care of that. For everything else, it has
to be - no JID validation is possible without that (except with copying the
code, of course).
|
There was a problem hiding this 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.
That was a bad idea as it breaks full-text search in posts, as every normal word is a valid JID. This reverts commit b35249b.
I updated a lot, please review again, @edhelas ☺! |
No need to explode all over the place.
d977b50
to
54083b7
Compare
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 -.
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. |
Let's see if I can revive this PR :) |
No description provided.