From 58b224500dccee8c1804cd6b25a172ffe4bbc263 Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Tue, 23 Apr 2024 07:21:51 -0500 Subject: [PATCH 01/13] minor: Fix log message, formatting. --- .../extensions/DefaultPacketExtensionProvider.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/jitsi/xmpp/extensions/DefaultPacketExtensionProvider.java b/src/main/java/org/jitsi/xmpp/extensions/DefaultPacketExtensionProvider.java index 0e28581..6cbd895 100644 --- a/src/main/java/org/jitsi/xmpp/extensions/DefaultPacketExtensionProvider.java +++ b/src/main/java/org/jitsi/xmpp/extensions/DefaultPacketExtensionProvider.java @@ -109,10 +109,12 @@ public C parse(XmlPullParser parser, int depth, XmlEnvironment xmlEnvironment) namespace = parser.getNamespace(); if (logger.isLoggable(Level.FINEST)) + { logger.finest("Will parse event " + eventType - + " for " + elementName - + " ns=" + namespace - + " class=" + packetExtension.getClass().getSimpleName()); + + " for " + elementName + + " ns=" + namespace + + " class=" + packetExtension.getClass().getSimpleName()); + } if (eventType == XmlPullParser.Event.START_ELEMENT) { @@ -122,7 +124,7 @@ public C parse(XmlPullParser parser, int depth, XmlEnvironment xmlEnvironment) if (provider == null) { //we don't know how to handle this kind of extensions. - logger.fine("Could not add a provider for element " + logger.fine("Could not find a provider for element " + elementName + " from namespace " + namespace); } else From e6bd67f2609224b1125d13bba78e18b2f2978070 Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Tue, 23 Apr 2024 13:50:23 -0500 Subject: [PATCH 02/13] Add JitsiXmppStringprep. --- pom.xml | 7 ++ src/main/kotlin/org/jitsi/xmpp/Smack.kt | 49 ++++++++++++ .../xmpp/stringprep/JitsiXmppStringprep.kt | 76 +++++++++++++++++++ 3 files changed, 132 insertions(+) create mode 100644 src/main/kotlin/org/jitsi/xmpp/Smack.kt create mode 100644 src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt diff --git a/pom.xml b/pom.xml index 5b25ea1..f18a7f9 100644 --- a/pom.xml +++ b/pom.xml @@ -7,6 +7,8 @@ bundle UTF-8 + + 1.0.3 4.4.6 5.10.0 1.9.10 @@ -39,6 +41,11 @@ smack-xmlparser-stax ${smack.version} + + org.jxmpp + jxmpp-stringprep-rocksxmppprecis + ${jxmpp.version} + org.jitsi jitsi-utils diff --git a/src/main/kotlin/org/jitsi/xmpp/Smack.kt b/src/main/kotlin/org/jitsi/xmpp/Smack.kt new file mode 100644 index 0000000..d415c94 --- /dev/null +++ b/src/main/kotlin/org/jitsi/xmpp/Smack.kt @@ -0,0 +1,49 @@ +/* + * Copyright @ 2024 - present 8x8, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jitsi.xmpp + +import org.jitsi.utils.logging2.createLogger +import org.jitsi.xmpp.stringprep.JitsiXmppStringprep +import org.jivesoftware.smack.SmackConfiguration +import org.jivesoftware.smack.parsing.ExceptionLoggingCallback +import org.jivesoftware.smackx.bytestreams.socks5.Socks5Proxy +import org.jxmpp.JxmppContext +import org.jxmpp.jid.impl.JidCreate + +object Smack { + val logger = createLogger() + + fun initialize() { + logger.info("Setting XML parsing limits.") + System.setProperty("jdk.xml.entityExpansionLimit", "0") + System.setProperty("jdk.xml.maxOccurLimit", "0") + System.setProperty("jdk.xml.elementAttributeLimit", "524288") + System.setProperty("jdk.xml.totalEntitySizeLimit", "0") + System.setProperty("jdk.xml.maxXMLNameLimit", "524288") + System.setProperty("jdk.xml.entityReplacementLimit", "0") + + // Force XmppStringPrepUtil to load before we override the context, otherwise it gets reverted. + // https://github.com/igniterealtime/jxmpp/pull/44 + JidCreate.from("example.com") + logger.info("Using JitsiXmppStringprep.") + JxmppContext.setDefaultXmppStringprep(JitsiXmppStringprep.INSTANCE) + + // if there is a parsing error, do not break the connection to the server(the default behaviour) as we need + // it for the other conferences. + SmackConfiguration.setDefaultParsingExceptionCallback(ExceptionLoggingCallback()) + Socks5Proxy.setLocalSocks5ProxyEnabled(false) + } +} diff --git a/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt b/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt new file mode 100644 index 0000000..95aebdf --- /dev/null +++ b/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt @@ -0,0 +1,76 @@ +/* + * Copyright @ 2024 - present 8x8, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jitsi.xmpp.stringprep + +import org.jxmpp.stringprep.XmppStringprep +import org.jxmpp.stringprep.XmppStringprepException +import org.jxmpp.stringprep.rocksxmppprecis.RocksXmppPrecisStringprep +import rocks.xmpp.precis.PrecisProfile +import java.net.IDN +import java.text.Normalizer +import java.util.regex.Pattern + +/** + * Extends [RocksXmppPrecisStringprep] to allow underscores (_) in the domain part. + * + * This is needed because jitsi-meet URLs of the form https://domain/tenant/room get translated into a JID of the + * form room@tenant.conference.domain, and the tenant field has been allowed to use underscores for a long time (in + * fact '.' in the tenant is translated into '_'). + */ +class JitsiXmppStringprep : XmppStringprep by RocksXmppPrecisStringprep.INSTANCE { + override fun domainprep(string: String?): String { + try { + return idnWithUnderscoreProfile.enforce(string) + } catch (e: IllegalArgumentException) { + throw XmppStringprepException(string, e) + } + } + + companion object { + val INSTANCE = JitsiXmppStringprep() + private val idnWithUnderscoreProfile = IDNWithUnderscoreProfile() + } +} + +/** + * Based on [PrecisProfiles.IDN], but allows underscores. + */ +class IDNWithUnderscoreProfile : PrecisProfile(false) { + override fun prepare(input: CharSequence): String { + val str = input.toString() + val strNoUnderscore = str.trim('_').trimEnd('_').replace('_', '-') + + // Throws if [strNoUnderscore] contains invalid characters + IDN.toASCII(strNoUnderscore, IDN.USE_STD3_ASCII_RULES) + + return IDN.toUnicode(IDN.toASCII(str), IDN.USE_STD3_ASCII_RULES) + } + + override fun applyWidthMappingRule(charSequence: CharSequence) = widthMap(charSequence) + override fun applyAdditionalMappingRule(charSequence: CharSequence) = + LABEL_SEPARATOR.matcher(charSequence).replaceAll(".") + override fun applyCaseMappingRule(charSequence: CharSequence) = charSequence.toString().lowercase() + + override fun applyNormalizationRule(charSequence: CharSequence) = + Normalizer.normalize(charSequence, Normalizer.Form.NFC) + + override fun applyDirectionalityRule(charSequence: CharSequence) = charSequence + + companion object { + private const val DOTS: String = "[.\u3002\uFF0E\uFF61]" + private val LABEL_SEPARATOR: Pattern = Pattern.compile(DOTS) + } +} From 4cfc3e7b4b81612de0d02ba0b03061d38a709f3c Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Tue, 23 Apr 2024 13:51:43 -0500 Subject: [PATCH 03/13] test: Move JidTest from jicofo. --- src/test/kotlin/org/jitsi/xmpp/JidTest.kt | 121 ++++++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 src/test/kotlin/org/jitsi/xmpp/JidTest.kt diff --git a/src/test/kotlin/org/jitsi/xmpp/JidTest.kt b/src/test/kotlin/org/jitsi/xmpp/JidTest.kt new file mode 100644 index 0000000..e91edad --- /dev/null +++ b/src/test/kotlin/org/jitsi/xmpp/JidTest.kt @@ -0,0 +1,121 @@ +/* + * Jicofo, the Jitsi Conference Focus. + * + * Copyright @ 2024-Present 8x8, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jitsi.xmpp + +import io.kotest.assertions.throwables.shouldThrow +import io.kotest.assertions.withClue +import io.kotest.core.spec.IsolationMode +import io.kotest.core.spec.style.ShouldSpec +import io.kotest.core.test.TestCase +import io.kotest.matchers.shouldNotBe +import io.kotest.matchers.types.shouldBeInstanceOf +import org.jitsi.xmpp.stringprep.JitsiXmppStringprep +import org.jxmpp.JxmppContext +import org.jxmpp.jid.impl.JidCreate +import org.jxmpp.stringprep.XmppStringprepException + +/** + * Test JID parsing. The lists below are based on the jxmpp corpora here, plus a couple additional ones: + * https://github.com/igniterealtime/jxmpp/tree/master/jxmpp-strings-testframework/src/main/resources/xmpp-strings/jids/valid/main + * https://github.com/igniterealtime/jxmpp/blob/master/jxmpp-strings-testframework/src/main/resources/xmpp-strings/jids/invalid/main + */ +class JidTest : ShouldSpec() { + override fun isolationMode(): IsolationMode { + return IsolationMode.SingleInstance + } + override suspend fun beforeAny(testCase: TestCase) { + super.beforeAny(testCase) + Smack.initialize() + } + + init { + context("Parsing valid JIDs") { + JxmppContext.getDefaultContext().xmppStringprep.shouldBeInstanceOf() + validJids.forEach { + withClue(it) { + JidCreate.from(it) shouldNotBe null + } + } + } + context("Parsing invalid JIDs") { + JxmppContext.getDefaultContext().xmppStringprep.shouldBeInstanceOf() + invalidJids.forEach { + withClue(it) { + shouldThrow { + JidCreate.from((it)) + } + } + } + } + } +} + +val validJids = listOf( + "juliet@example.com", + "juliet@example.com/foo", + "juliet@example.com/foo bar", + "juliet@example.com/foo@bar", + "foo\\20bar@example.com", + "fussball@example.com", + "fußball@example.com", + "π@example.com", + "Σ@example.com", + "ς@example.com", + "king@example.com/♚", + "example.com", + "example.com/foobar", + "a.example.com/b@example.net", + "server/resource@foo", + "server/resource@foo/bar", + "user@CaSe-InSeNsItIvE", + "user@192.168.1.1", + // "user@[2001:638:a000:4134::ffff:40]", + // "user@[2001:638:a000:4134::ffff:40%eno1]", + // "user@averylongdomainpartisstillvalideventhoughitexceedsthesixtyfourbytelimitofdnslabels", + "long-conference-name-1245c711a15e466687b6333577d83e0b@" + + "conference.vpaas-magic-cookie-a32a0c3311ee432eab711fa1fdf34793.8x8.vc", + "user@example.org/🍺" +) + +val invalidJids = listOf( + "jul\u0001iet@example.c", + "\"juliet\"@example.com", + "foo bar@example.com", + // This fails due to a corner case in JidCreate when "example.com" is already cached as a DomainpartJid + // "@example.com/", + "henryⅣ@example.com", + "♚@example.com", + "juliet@", + "/foobar", + "node@/server", + "@server", + "@server/resource", + "@/resource", + "@/", + "/", + "@", + "user@", + "user@@", + "user@@host", + "user@@host/resource", + "user@@host/", + "xsf@muc.xmpp.org/؜x", + "username@example.org@example.org", + "foo\u0000bar@example.org", + "foobar@ex\u0000ample.org" +) From 894500bbcae8d78799f4638406607f34a79a3cc3 Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Tue, 23 Apr 2024 14:15:22 -0500 Subject: [PATCH 04/13] Add tests for JIDs with _, allow in any position. --- src/main/kotlin/org/jitsi/xmpp/Smack.kt | 2 +- .../org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt | 5 ++--- src/test/kotlin/org/jitsi/xmpp/JidTest.kt | 12 ++++++++++-- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/main/kotlin/org/jitsi/xmpp/Smack.kt b/src/main/kotlin/org/jitsi/xmpp/Smack.kt index d415c94..3754ef7 100644 --- a/src/main/kotlin/org/jitsi/xmpp/Smack.kt +++ b/src/main/kotlin/org/jitsi/xmpp/Smack.kt @@ -37,7 +37,7 @@ object Smack { // Force XmppStringPrepUtil to load before we override the context, otherwise it gets reverted. // https://github.com/igniterealtime/jxmpp/pull/44 - JidCreate.from("example.com") + JidCreate.from("example") logger.info("Using JitsiXmppStringprep.") JxmppContext.setDefaultXmppStringprep(JitsiXmppStringprep.INSTANCE) diff --git a/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt b/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt index 95aebdf..97e882b 100644 --- a/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt +++ b/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt @@ -51,10 +51,9 @@ class JitsiXmppStringprep : XmppStringprep by RocksXmppPrecisStringprep.INSTANCE class IDNWithUnderscoreProfile : PrecisProfile(false) { override fun prepare(input: CharSequence): String { val str = input.toString() - val strNoUnderscore = str.trim('_').trimEnd('_').replace('_', '-') - // Throws if [strNoUnderscore] contains invalid characters - IDN.toASCII(strNoUnderscore, IDN.USE_STD3_ASCII_RULES) + // 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) } diff --git a/src/test/kotlin/org/jitsi/xmpp/JidTest.kt b/src/test/kotlin/org/jitsi/xmpp/JidTest.kt index e91edad..6d17a0a 100644 --- a/src/test/kotlin/org/jitsi/xmpp/JidTest.kt +++ b/src/test/kotlin/org/jitsi/xmpp/JidTest.kt @@ -89,7 +89,11 @@ val validJids = listOf( // "user@averylongdomainpartisstillvalideventhoughitexceedsthesixtyfourbytelimitofdnslabels", "long-conference-name-1245c711a15e466687b6333577d83e0b@" + "conference.vpaas-magic-cookie-a32a0c3311ee432eab711fa1fdf34793.8x8.vc", - "user@example.org/🍺" + "user@example.org/🍺", + // These are not valid according to the XMPP spec, but we accept it intentionally. + "do_main.com", + "u_s_e_r@_do_main_.com", + "user@do_ma-in.com" ) val invalidJids = listOf( @@ -117,5 +121,9 @@ val invalidJids = listOf( "xsf@muc.xmpp.org/؜x", "username@example.org@example.org", "foo\u0000bar@example.org", - "foobar@ex\u0000ample.org" + "foobar@ex\u0000ample.org", + // Leading - in domain part. + "user@-do-main.com", + // Trailing - in domain part. + "user@do-main-.com" ) From b97e588ad2c174ffa17b09e11e0975aff40f71ee Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Tue, 23 Apr 2024 14:23:20 -0500 Subject: [PATCH 05/13] test: Add XmlStringBuilderPerfTest. --- .../extensions/XmlStringBuilderPerfTest.kt | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 src/test/kotlin/org/jitsi/xmpp/extensions/XmlStringBuilderPerfTest.kt diff --git a/src/test/kotlin/org/jitsi/xmpp/extensions/XmlStringBuilderPerfTest.kt b/src/test/kotlin/org/jitsi/xmpp/extensions/XmlStringBuilderPerfTest.kt new file mode 100644 index 0000000..ab863ad --- /dev/null +++ b/src/test/kotlin/org/jitsi/xmpp/extensions/XmlStringBuilderPerfTest.kt @@ -0,0 +1,114 @@ +/* + * Jicofo, the Jitsi Conference Focus. + * + * Copyright @ 2024-Present 8x8, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jitsi.xmpp.extensions + +import io.kotest.core.spec.style.ShouldSpec +import org.jitsi.utils.logging2.createLogger +import org.jivesoftware.smack.util.Supplier +import org.jivesoftware.smack.util.XmlStringBuilder + +/** + * A test for the performance of XmlStringBuilder serialization. See https://github.com/igniterealtime/Smack/pull/569 + */ +class XmlStringBuilderPerfTest : ShouldSpec() { + val countOuter: Int = 500 + val countInner: Int = 50 + val logger = createLogger() + + init { + xcontext("XmlStringBuilder.toString() performance") { + test1() + test2() + test3() + } + } + + private fun test1() { + logger.info("Test 1") + val parent = XmlStringBuilder() + val child = XmlStringBuilder() + val child2 = XmlStringBuilder() + + for (i in 1 until countOuter) { + val cs = XmlStringBuilder() + for (j in 0 until countInner) { + cs.append("abc") + } + child2.append(cs as CharSequence) + } + + child.append(child2 as CharSequence) + parent.append(child as CharSequence) + + time("test1: parent") { "len=" + parent.toString().length } + time("test1: child") { "len=" + child.toString().length } + time("test1: child2") { "len=" + child2.toString().length } + } + + private fun test2() { + logger.info("Test 2: evaluate children first") + val parent = XmlStringBuilder() + val child = XmlStringBuilder() + val child2 = XmlStringBuilder() + + for (i in 1 until countOuter) { + val cs = XmlStringBuilder() + for (j in 0 until countInner) { + cs.append("abc") + } + child2.append(cs as CharSequence) + } + + child.append(child2 as CharSequence) + parent.append(child as CharSequence) + + time("test2: child2") { "len=" + child2.toString().length } + time("test2: child") { "len=" + child.toString().length } + time("test2: parent") { "len=" + parent.toString().length } + } + + private fun test3() { + logger.info("Test 3: use append(XmlStringBuilder)") + val parent = XmlStringBuilder() + val child = XmlStringBuilder() + val child2 = XmlStringBuilder() + + for (i in 1 until countOuter) { + val cs = XmlStringBuilder() + for (j in 0 until countInner) { + cs.append("abc") + } + child2.append(cs) + } + + child.append(child2) + parent.append(child) + + time("test3: parent") { "len=" + parent.toString().length } + time("test3: child") { "len=" + child.toString().length } + time("test3: child2") { "len=" + child2.toString().length } + } + + fun time(name: String, block: Supplier) { + val start = System.currentTimeMillis() + val result = block.get() + val end = System.currentTimeMillis() + + logger.info(name + " took " + (end - start) + "ms: " + result) + } +} From 0d826aa34e52d6ead12290a89e5e9d9843a43fef Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Tue, 23 Apr 2024 15:38:12 -0500 Subject: [PATCH 06/13] chore: Update to smack 4.4.8. https://github.com/igniterealtime/Smack/blob/4.4.8/CHANGELOG.md --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index f18a7f9..c5bec15 100644 --- a/pom.xml +++ b/pom.xml @@ -9,7 +9,7 @@ UTF-8 1.0.3 - 4.4.6 + 4.4.8 5.10.0 1.9.10 5.7.2 From caa39a9ce4d446c90eee10541e52011ad0489fb3 Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Tue, 23 Apr 2024 17:03:19 -0500 Subject: [PATCH 07/13] test:Add another invalid JID. --- src/test/kotlin/org/jitsi/xmpp/JidTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/kotlin/org/jitsi/xmpp/JidTest.kt b/src/test/kotlin/org/jitsi/xmpp/JidTest.kt index 6d17a0a..ef04a63 100644 --- a/src/test/kotlin/org/jitsi/xmpp/JidTest.kt +++ b/src/test/kotlin/org/jitsi/xmpp/JidTest.kt @@ -125,5 +125,6 @@ val invalidJids = listOf( // Leading - in domain part. "user@-do-main.com", // Trailing - in domain part. - "user@do-main-.com" + "user@do-main-.com", + "user@conference..example.org" ) From 6b23236cc3010d5cef54e9496cf60ed6c035129e Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Wed, 24 Apr 2024 11:55:06 -0500 Subject: [PATCH 08/13] squash: Fix typo. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index c5bec15..504962a 100644 --- a/pom.xml +++ b/pom.xml @@ -7,7 +7,7 @@ bundle UTF-8 - + 1.0.3 4.4.8 5.10.0 From fdbbe55fff1d225b4cd733b7300a9234f6998190 Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Thu, 25 Apr 2024 11:08:18 -0500 Subject: [PATCH 09/13] feat: Implement the LDHU test explicitly, test IDNs. --- .../xmpp/stringprep/JitsiXmppStringprep.kt | 49 +++++++- src/test/kotlin/org/jitsi/xmpp/JidTest.kt | 113 ++++++++++++++++++ 2 files changed, 156 insertions(+), 6 deletions(-) diff --git a/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt b/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt index 97e882b..f6c50b4 100644 --- a/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt +++ b/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt @@ -50,12 +50,37 @@ class JitsiXmppStringprep : XmppStringprep by RocksXmppPrecisStringprep.INSTANCE */ class IDNWithUnderscoreProfile : PrecisProfile(false) { override fun prepare(input: CharSequence): String { - val str = input.toString() + // We're calling toASCII and toUnicode without the [IDN.USE_STD3_ASCII_RULES] flag, so we have to do the + // (relaxed) verification. + val ascii = verifyLDHU(IDN.toASCII(input.toString())) + return verifyLDHU(IDN.toUnicode(ascii)) + } - // Throws if it contains invalid characters - IDN.toASCII(str.replace("_", ""), IDN.USE_STD3_ASCII_RULES) + /** + * Assert that, after splitting [s] into labels separated, each label: + * -- Is not empty. + * -- All ASCII characters are Letters/Digits/Hyphen/Underscore. + * -- Does not begin or end with a hyphen. + * + * Based on the implementation in java's IDN. + * + * @throws IllegalStateException if any of the assertions fail. + */ + private fun verifyLDHU(s: String) = s.also { + val dest = StringBuffer(s) + require(dest.isNotEmpty()) { "Empty label is not a legal name" } - return IDN.toUnicode(IDN.toASCII(str), IDN.USE_STD3_ASCII_RULES) + for (i in s.indices) { + require(!dest[i].code.isNonLDHUAsciiCodePoint()) { "Contains non-LDHU ASCII characters: ${dest[i]}" } + if (dest[i].isLabelSeparator()) { + require(i != 0) { "Empty label is not a legal name" } + require(dest[i - 1] != '-') { "Label has trailing hyphen" } + require(!dest[i - 1].isLabelSeparator()) { "Empty label is not a legal name" } + require(i == dest.length - 1 || dest[i + 1] != '-') { "Label has leading hyphen" } + require(i == dest.length - 1 || !dest[i + 1].isLabelSeparator()) { "Empty label" } + } + } + require(dest[0] != '-' && dest[dest.length - 1] != '-') { "Has leading or trailing hyphen" } } override fun applyWidthMappingRule(charSequence: CharSequence) = widthMap(charSequence) @@ -69,7 +94,19 @@ class IDNWithUnderscoreProfile : PrecisProfile(false) { override fun applyDirectionalityRule(charSequence: CharSequence) = charSequence companion object { - private const val DOTS: String = "[.\u3002\uFF0E\uFF61]" - private val LABEL_SEPARATOR: Pattern = Pattern.compile(DOTS) + private val dots = listOf('.', '\u3002', '\uFF0E', '\uFF61').toCharArray() + private val LABEL_SEPARATOR = Pattern.compile("[${dots.joinToString(separator = "")}]") + + private fun Char.isLabelSeparator() = dots.contains(this) + + /** Return true if [this] is a code for an ASCII character that is not a Letter/Digit/Hyphen/Underscore. */ + private fun Int.isNonLDHUAsciiCodePoint(): Boolean { + return (this in 0x0000..0x002C) || + (this == 0x002F) || + (this in 0x003A..0x0040) || + (this in 0x005B..0x005e) || + (this == 0x0060) || + (this in 0x007B..0x007F) + } } } diff --git a/src/test/kotlin/org/jitsi/xmpp/JidTest.kt b/src/test/kotlin/org/jitsi/xmpp/JidTest.kt index ef04a63..82004ad 100644 --- a/src/test/kotlin/org/jitsi/xmpp/JidTest.kt +++ b/src/test/kotlin/org/jitsi/xmpp/JidTest.kt @@ -22,8 +22,10 @@ import io.kotest.assertions.withClue import io.kotest.core.spec.IsolationMode import io.kotest.core.spec.style.ShouldSpec import io.kotest.core.test.TestCase +import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNotBe import io.kotest.matchers.types.shouldBeInstanceOf +import org.jitsi.xmpp.stringprep.IDNWithUnderscoreProfile import org.jitsi.xmpp.stringprep.JitsiXmppStringprep import org.jxmpp.JxmppContext import org.jxmpp.jid.impl.JidCreate @@ -62,9 +64,120 @@ class JidTest : ShouldSpec() { } } } + context("Parsing internationalized domains") { + val idnWithUnderscoreProfile = IDNWithUnderscoreProfile() + + idns.forEach { idnGroup -> + idnGroup.forEach { idn -> + withClue(idn) { + // prepare() doesn't always normalize, but it shouldn't throw an exception. + idnWithUnderscoreProfile.prepare(idn) + + idnWithUnderscoreProfile.enforce(idn) shouldBe idnGroup[0] + JidCreate.from(idn).toString() shouldBe idnGroup[0] + } + } + } + } + context("Parsing invalid domains") { + val idnWithUnderscoreProfile = IDNWithUnderscoreProfile() + + invalidIdns.forEach { idn -> + withClue(idn) { + shouldThrow { + idnWithUnderscoreProfile.prepare(idn) + } + shouldThrow { + idnWithUnderscoreProfile.enforce(idn) + } + } + } + } + context("JIDs with IDNs") { + listOf("user", "юзер", "π", "测试").forEach { username -> + listOf("resource", "ресурс", "🍺").forEach { resource -> + idns.forEach { idnGroup -> + idnGroup.forEach { idn -> + val str = "$username@$idn/$resource" + withClue(str) { + val jid = JidCreate.from(str) + jid.resourceOrNull.toString() shouldBe resource + jid.localpartOrNull.toString() shouldBe username + jid.domain.toString() shouldBe idnGroup[0] + } + } + } + } + } + } } } +/** + * Valid internationalized domain names. The first entry in each group is the normalized form that we expect, the other + * entries are other encodings of the same domain. + * + * https://www.iana.org/domains/reserved + */ +val idns = listOf( + listOf("إختبار", "XN--KGBECHTV", "xn--KGBEchtv"), + listOf("آزمایشی", "XN--HGBK6AJ7F53BBA", "xn--hgbK6AJ7F53BBA"), + listOf("测试", "XN--0ZWM56D", "Xn--0Zwm56D"), + listOf("測試", "XN--G6W251D", "Xn--G6W251d"), + listOf("испытание", "XN--80AKHBYKNJ4F", "XN--80AKHBYKNJ4f", "испытаНИЕ"), + listOf("испыта-ние", "испыта-НИЕ", "xn----7sbqjc3alpk3g"), + listOf("abc-испытание-def", "ABc-испытаНИЕ-DeF", "xn--abc--def-46g4c5ab8d0a3ar6m"), + listOf("испытание.com", "XN--80AKHBYKNJ4F\u3002com", "XN--80AKHBYKNJ4f\uFF0Ecom", "испытаНИЕ\uFF61com"), + listOf("испыта-ние.com", "испыта-НИЕ\u3002com", "xn----7sbqjc3alpk3g\uFF0Ecom", "XN----7SBQJC3ALPK3G\uFF61com"), + listOf( + "abc-испытание-def.com", + "ABc-испытаНИЕ-DeF\u3002com", + "xn--abc--def-46g4c5ab8d0a3ar6m\uFF0Ecom", + "ABc-испытаНИЕ-DeF\uFF61com" + ), + listOf("abc.испытание-def.com", "ABc.испытаНИЕ-DeF\u3002com", "ABc.испытаНИЕ-DeF\uFF61com"), + listOf("परीक्षा", "XN--11B5BS3A9AJ6G", "xn--11B5bs3A9AJ6G"), + listOf("δοκιμή", "XN--JXALPDLP"), + listOf("테스트", "XN--9T4B11YI5A"), + listOf("טעסט", "XN--DEBA0AD"), + listOf("テスト", "XN--ZCKZAH"), + listOf("பரிட்சை", "XN--HLCJ6AYA9ESC7A"), + listOf("bücher.de", "xn--bcher-kva.de"), + listOf("büchxr.de", "xn--bchxr-kva.de"), + listOf("büch_r.de", "xn--bch_r-kva.de"), + listOf("buch_ü"), + listOf("__б__") +) + +val invalidIdns = listOf( + // Invalid ascii characters + "buch?r", + "büch?r", + "büch[r", + // Leading hyphens + "-bücher", + "-bücher.com", + "sub.-bücher.com", + "sub\u3002-bücher\uFF61com", + "sub\uFF0E-bücher.com", + "sub\uFF61-bücher\uFF61com", + // Trailing hyphens + "bücher-", + "bücher-.com", + "sub-.bücher.com", + "bücher-\u3002com", + "bücher-\uFF0Ecom", + "bücher-\uFF61com", + // Empty labels + "example..com", + "example\uFF0E\u3002com", + "example.com..", + "example.com...", + "example\uFF61com\uFF0E\u3002", + "\u3002\uFF61example.com", + ".example.com", +) + val validJids = listOf( "juliet@example.com", "juliet@example.com/foo", From 55827844b1a12824a98e17a49d2b13c565a8338a Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Thu, 25 Apr 2024 13:48:40 -0500 Subject: [PATCH 10/13] Add some more test cases. --- src/test/kotlin/org/jitsi/xmpp/JidTest.kt | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/test/kotlin/org/jitsi/xmpp/JidTest.kt b/src/test/kotlin/org/jitsi/xmpp/JidTest.kt index 82004ad..e7acd28 100644 --- a/src/test/kotlin/org/jitsi/xmpp/JidTest.kt +++ b/src/test/kotlin/org/jitsi/xmpp/JidTest.kt @@ -146,7 +146,10 @@ val idns = listOf( listOf("büchxr.de", "xn--bchxr-kva.de"), listOf("büch_r.de", "xn--bch_r-kva.de"), listOf("buch_ü"), - listOf("__б__") + listOf("__б__"), + // IDNA2003 converts this to "fussball", while IDNA2008 leaves the "ß" as is. OpenJDK 11, 17, 21 implement the older + // standard. This is here to document the behavior and alert if it changes. + listOf("fussball", "fußball") ) val invalidIdns = listOf( @@ -184,6 +187,7 @@ val validJids = listOf( "juliet@example.com/foo bar", "juliet@example.com/foo@bar", "foo\\20bar@example.com", + "foo%bar@example.com/f%b", "fussball@example.com", "fußball@example.com", "π@example.com", @@ -197,13 +201,10 @@ val validJids = listOf( "server/resource@foo/bar", "user@CaSe-InSeNsItIvE", "user@192.168.1.1", - // "user@[2001:638:a000:4134::ffff:40]", - // "user@[2001:638:a000:4134::ffff:40%eno1]", - // "user@averylongdomainpartisstillvalideventhoughitexceedsthesixtyfourbytelimitofdnslabels", "long-conference-name-1245c711a15e466687b6333577d83e0b@" + "conference.vpaas-magic-cookie-a32a0c3311ee432eab711fa1fdf34793.8x8.vc", "user@example.org/🍺", - // These are not valid according to the XMPP spec, but we accept it intentionally. + // These are not valid according to the XMPP spec, but we accept them intentionally. "do_main.com", "u_s_e_r@_do_main_.com", "user@do_ma-in.com" @@ -239,5 +240,11 @@ val invalidJids = listOf( "user@-do-main.com", // Trailing - in domain part. "user@do-main-.com", - "user@conference..example.org" + "user@conference..example.org", + // These are VALID according to the XMPP spec (see the valid corpus), but we currently do not accept them. + // [ is an ASSCI symbol that's not allowed in domain names. + "user@[2001:638:a000:4134::ffff:40]", + "user@[2001:638:a000:4134::ffff:40%eno1]", + // A single label in the domain part is limited to 63 + "user@averylongdomainpartisstillvalideventhoughitexceedsthesixtyfourbytelimitofdnslabels", ) From d14f2ee695f351c27c9dc1c2776dfa1ece241889 Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Thu, 25 Apr 2024 13:57:04 -0500 Subject: [PATCH 11/13] Also accept % in domain labels. --- .../xmpp/stringprep/JitsiXmppStringprep.kt | 19 +++++++++++-------- src/test/kotlin/org/jitsi/xmpp/JidTest.kt | 8 +++++++- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt b/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt index f6c50b4..64b82e1 100644 --- a/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt +++ b/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt @@ -27,8 +27,8 @@ import java.util.regex.Pattern * Extends [RocksXmppPrecisStringprep] to allow underscores (_) in the domain part. * * This is needed because jitsi-meet URLs of the form https://domain/tenant/room get translated into a JID of the - * form room@tenant.conference.domain, and the tenant field has been allowed to use underscores for a long time (in - * fact '.' in the tenant is translated into '_'). + * form room@tenant.conference.domain, and the tenant field has been allowed to use _ and % for a long time (in + * fact '.' in the tenant is translated into '_', while unicode characters get url encoded into e.g. %c3%9f). */ class JitsiXmppStringprep : XmppStringprep by RocksXmppPrecisStringprep.INSTANCE { override fun domainprep(string: String?): String { @@ -59,10 +59,10 @@ class IDNWithUnderscoreProfile : PrecisProfile(false) { /** * Assert that, after splitting [s] into labels separated, each label: * -- Is not empty. - * -- All ASCII characters are Letters/Digits/Hyphen/Underscore. + * -- All ASCII characters are Letters/Digits/Hyphen/Underscore/Percent. * -- Does not begin or end with a hyphen. * - * Based on the implementation in java's IDN. + * Based on the implementation in java's IDN, but relaxed to accept _ and % as part of a label. * * @throws IllegalStateException if any of the assertions fail. */ @@ -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]}" } if (dest[i].isLabelSeparator()) { require(i != 0) { "Empty label is not a legal name" } require(dest[i - 1] != '-') { "Label has trailing hyphen" } @@ -99,9 +99,12 @@ class IDNWithUnderscoreProfile : PrecisProfile(false) { private fun Char.isLabelSeparator() = dots.contains(this) - /** Return true if [this] is a code for an ASCII character that is not a Letter/Digit/Hyphen/Underscore. */ - private fun Int.isNonLDHUAsciiCodePoint(): Boolean { - return (this in 0x0000..0x002C) || + /** + * Return true if [this] is a code for an ASCII character that is not a Letter/Digit/Hyphen/Underscore/Percent. + */ + private fun Int.isNonLDHUPAsciiCodePoint(): Boolean { + return (this in 0x0000..0x0024) || + (this in 0x0026..0x002C) || (this == 0x002F) || (this in 0x003A..0x0040) || (this in 0x005B..0x005e) || diff --git a/src/test/kotlin/org/jitsi/xmpp/JidTest.kt b/src/test/kotlin/org/jitsi/xmpp/JidTest.kt index e7acd28..527efe0 100644 --- a/src/test/kotlin/org/jitsi/xmpp/JidTest.kt +++ b/src/test/kotlin/org/jitsi/xmpp/JidTest.kt @@ -147,6 +147,9 @@ val idns = listOf( listOf("büch_r.de", "xn--bch_r-kva.de"), listOf("buch_ü"), listOf("__б__"), + listOf("büch%r.de", "xn--bch%r-kva.de"), + listOf("buch%ü"), + listOf("_%б%_"), // IDNA2003 converts this to "fussball", while IDNA2008 leaves the "ß" as is. OpenJDK 11, 17, 21 implement the older // standard. This is here to document the behavior and alert if it changes. listOf("fussball", "fußball") @@ -207,7 +210,10 @@ val validJids = listOf( // These are not valid according to the XMPP spec, but we accept them intentionally. "do_main.com", "u_s_e_r@_do_main_.com", - "user@do_ma-in.com" + "user@do_ma-in.com", + "do%main.com", + "u%s%e%r@%do%main%.com", + "user@do%ma-in.com" ) val invalidJids = listOf( From 9d0d4bd17dd246677f650056fab7e53c430f9b23 Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Thu, 25 Apr 2024 13:58:31 -0500 Subject: [PATCH 12/13] Add a flag to control the use of JitsiXmppStringprep. --- src/main/kotlin/org/jitsi/xmpp/Smack.kt | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/main/kotlin/org/jitsi/xmpp/Smack.kt b/src/main/kotlin/org/jitsi/xmpp/Smack.kt index 3754ef7..8b1d9ad 100644 --- a/src/main/kotlin/org/jitsi/xmpp/Smack.kt +++ b/src/main/kotlin/org/jitsi/xmpp/Smack.kt @@ -26,7 +26,7 @@ import org.jxmpp.jid.impl.JidCreate object Smack { val logger = createLogger() - fun initialize() { + fun initialize(useJitsiXmppStringprep: Boolean = true) { logger.info("Setting XML parsing limits.") System.setProperty("jdk.xml.entityExpansionLimit", "0") System.setProperty("jdk.xml.maxOccurLimit", "0") @@ -35,11 +35,13 @@ object Smack { System.setProperty("jdk.xml.maxXMLNameLimit", "524288") System.setProperty("jdk.xml.entityReplacementLimit", "0") - // Force XmppStringPrepUtil to load before we override the context, otherwise it gets reverted. - // https://github.com/igniterealtime/jxmpp/pull/44 - JidCreate.from("example") - logger.info("Using JitsiXmppStringprep.") - JxmppContext.setDefaultXmppStringprep(JitsiXmppStringprep.INSTANCE) + if (useJitsiXmppStringprep) { + // Force XmppStringPrepUtil to load before we override the context, otherwise it gets reverted. + // https://github.com/igniterealtime/jxmpp/pull/44 + JidCreate.from("example") + logger.info("Using JitsiXmppStringprep.") + JxmppContext.setDefaultXmppStringprep(JitsiXmppStringprep.INSTANCE) + } // if there is a parsing error, do not break the connection to the server(the default behaviour) as we need // it for the other conferences. From 29be535d8dbe881ea6996e16d3856858d2ae1771 Mon Sep 17 00:00:00 2001 From: Boris Grozev Date: Thu, 25 Apr 2024 14:45:48 -0500 Subject: [PATCH 13/13] squash: Add a P to function name and log message. --- .../org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt b/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt index 64b82e1..9ca7be1 100644 --- a/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt +++ b/src/main/kotlin/org/jitsi/xmpp/stringprep/JitsiXmppStringprep.kt @@ -52,8 +52,8 @@ class IDNWithUnderscoreProfile : PrecisProfile(false) { override fun prepare(input: CharSequence): String { // We're calling toASCII and toUnicode without the [IDN.USE_STD3_ASCII_RULES] flag, so we have to do the // (relaxed) verification. - val ascii = verifyLDHU(IDN.toASCII(input.toString())) - return verifyLDHU(IDN.toUnicode(ascii)) + val ascii = verifyLDHUP(IDN.toASCII(input.toString())) + return verifyLDHUP(IDN.toUnicode(ascii)) } /** @@ -66,12 +66,12 @@ class IDNWithUnderscoreProfile : PrecisProfile(false) { * * @throws IllegalStateException if any of the assertions fail. */ - private fun verifyLDHU(s: String) = s.also { + private fun verifyLDHUP(s: String) = s.also { val dest = StringBuffer(s) require(dest.isNotEmpty()) { "Empty label is not a legal name" } for (i in s.indices) { - require(!dest[i].code.isNonLDHUPAsciiCodePoint()) { "Contains non-LDHU ASCII characters: ${dest[i]}" } + require(!dest[i].code.isNonLDHUPAsciiCodePoint()) { "Contains non-LDHUP ASCII characters: ${dest[i]}" } if (dest[i].isLabelSeparator()) { require(i != 0) { "Empty label is not a legal name" } require(dest[i - 1] != '-') { "Label has trailing hyphen" }