-
Notifications
You must be signed in to change notification settings - Fork 51
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
Add JitsiXmppStringprep #105
Conversation
bgrozev
commented
Apr 23, 2024
- minor: Fix log message, formatting.
- Add JitsiXmppStringprep.
- test: Move JidTest from jicofo.
- Add tests for JIDs with _, allow in any position.
- test: Add XmlStringBuilderPerfTest.
- chore: Update to smack 4.4.8.
// Throws if it contains invalid characters | ||
IDN.toASCII(str.replace("_", ""), IDN.USE_STD3_ASCII_RULES) | ||
|
||
return IDN.toUnicode(IDN.toASCII(str), IDN.USE_STD3_ASCII_RULES) |
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.
I don't understand what this will do with actual IDNs (domains that have unicode and/or the IDN encoding --xn--). Do we want to write tests for this?
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.
Not sure what's going here, but please be careful with Java and IDN, as per this comment: dnsjava/dnsjava#207 (comment) (disregard the specifics for dnsjava).
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! It does indeed convert "ß" to "ss" under the few openjdk versions I tried. I've added a test to document it, but I think it should be fine for our purpose here.
For context: we're adding stricter validation of the JIDs used in jicofo (and the other components) to prevent obviously invalid JIDs to be processed. But we've been using _
and %
as part of the "tenant" for years and prefer to continue accepting to prevent breaking conference URLs that used to work. Unicode characters in the URL are urlencoded before they are used in JIDs, so in practice this shouldn't affect URLs that use unicode.
As an example the URL https://meet.jit.si/fuß.ball/foo
ends up using the following MUC JID: foo@conference.fu%c3%9f_ball.meet.jit.si
. The domain part is invalid due to %
and _
, but we want to allow it anyway.
@@ -71,7 +71,7 @@ class IDNWithUnderscoreProfile : PrecisProfile(false) { | |||
require(dest.isNotEmpty()) { "Empty label is not a legal name" } | |||
|
|||
for (i in s.indices) { | |||
require(!dest[i].code.isNonLDHUAsciiCodePoint()) { "Contains non-LDHU ASCII characters: ${dest[i]}" } | |||
require(!dest[i].code.isNonLDHUPAsciiCodePoint()) { "Contains non-LDHU ASCII characters: ${dest[i]}" } |
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.
Error message should presumably have P added to it