2 files changed, 83 insertions, 66 deletions
diff --git a/Swiften/JID/JID.cpp b/Swiften/JID/JID.cpp
index c82674d..fff88e9 100644
--- a/Swiften/JID/JID.cpp
+++ b/Swiften/JID/JID.cpp
@@ -1,23 +1,16 @@
 /*
- * Copyright (c) 2010-2016 Isode Limited.
+ * Copyright (c) 2010-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
-#define SWIFTEN_CACHE_JID_PREP
-
 #include <sstream>
 #include <string>
 #include <vector>
 
-#ifdef SWIFTEN_CACHE_JID_PREP
-#include <mutex>
-#include <unordered_map>
-#endif
-
 #include <boost/optional.hpp>
 
 #include <Swiften/Base/String.h>
 #include <Swiften/IDN/IDNConverter.h>
 #include <Swiften/JID/JID.h>
 
@@ -25,21 +18,12 @@
 #include <memory>
 #include <Swiften/IDN/PlatformIDNConverter.h>
 #endif
 
 using namespace Swift;
 
-#ifdef SWIFTEN_CACHE_JID_PREP
-typedef std::unordered_map<std::string, std::string> PrepCache;
-
-static std::mutex namePrepCacheMutex;
-static PrepCache nodePrepCache;
-static PrepCache domainPrepCache;
-static PrepCache resourcePrepCache;
-#endif
-
 static const std::vector<char> escapedChars = {' ', '"', '&', '\'', '/', '<', '>', '@', ':'};
 
 static IDNConverter* idnConverter = nullptr;
 
 #ifndef SWIFTEN_JID_NO_DEFAULT_IDN_CONVERTER
 namespace {
@@ -101,18 +85,18 @@ void JID::initializeFromString(const std::string& jid) {
         resource = jid.substr(slashIndex + 1, jid.npos);
     }
     else {
         hasResource_ = false;
         bare = jid;
     }
-    std::pair<std::string,std::string> nodeAndDomain = String::getSplittedAtFirst(bare, '@');
-    if (nodeAndDomain.second.empty()) {
-        nameprepAndSetComponents("", nodeAndDomain.first, resource);
+    auto firstMatch = bare.find('@');
+    if (firstMatch != bare.npos) {
+        nameprepAndSetComponents(bare.substr(0, firstMatch), bare.substr(firstMatch + 1), resource);
     }
     else {
-        nameprepAndSetComponents(nodeAndDomain.first, nodeAndDomain.second, resource);
+        nameprepAndSetComponents("", bare, resource);
     }
 }
 
 
 void JID::nameprepAndSetComponents(const std::string& node, const std::string& domain, const std::string& resource) {
     if (domain.empty() || !idnConverter->getIDNAEncoded(domain)) {
@@ -121,60 +105,26 @@ void JID::nameprepAndSetComponents(const std::string& node, const std::string& d
     }
 
     if (hasResource_ && resource.empty()) {
         valid_ = false;
         return;
     }
-#ifndef SWIFTEN_CACHE_JID_PREP
-    node_ = idnConverter->getStringPrepared(node, IDNConverter::XMPPNodePrep);
-    domain_ = idnConverter->getStringPrepared(domain, IDNConverter::NamePrep);
-    resource_ = idnConverter->getStringPrepared(resource, IDNConverter::XMPPResourcePrep);
-#else
-    std::unique_lock<std::mutex> lock(namePrepCacheMutex);
-
-    std::pair<PrepCache::iterator, bool> r;
-
-    r = nodePrepCache.insert(std::make_pair(node, std::string()));
-    if (r.second) {
-        try {
-            r.first->second = idnConverter->getStringPrepared(node, IDNConverter::XMPPNodePrep);
-        }
-        catch (...) {
-            nodePrepCache.erase(r.first);
-            valid_ = false;
-            return;
-        }
-    }
-    node_ = r.first->second;
 
-    r = domainPrepCache.insert(std::make_pair(domain, std::string()));
-    if (r.second) {
-        try {
-            r.first->second = idnConverter->getStringPrepared(domain, IDNConverter::NamePrep);
-        }
-        catch (...) {
-            domainPrepCache.erase(r.first);
-            valid_ = false;
-            return;
+    try {
+        node_ = idnConverter->getStringPrepared(node, IDNConverter::XMPPNodePrep);
+        if (domain.back() == '.') {
+            domain_ = idnConverter->getStringPrepared(domain.substr(0, domain.size() - 1), IDNConverter::NamePrep);
         }
-    }
-    domain_ = r.first->second;
-
-    r = resourcePrepCache.insert(std::make_pair(resource, std::string()));
-    if (r.second) {
-        try {
-            r.first->second = idnConverter->getStringPrepared(resource, IDNConverter::XMPPResourcePrep);
-        }
-        catch (...) {
-            resourcePrepCache.erase(r.first);
-            valid_ = false;
-            return;
+        else {
+            domain_ = idnConverter->getStringPrepared(domain, IDNConverter::NamePrep);
         }
+        resource_ = idnConverter->getStringPrepared(resource, IDNConverter::XMPPResourcePrep);
+    } catch (...) {
+        valid_ = false;
+        return;
     }
-    resource_ = r.first->second;
-#endif
 
     if (domain_.empty()) {
         valid_ = false;
         return;
     }
 }
diff --git a/Swiften/JID/UnitTest/JIDTest.cpp b/Swiften/JID/UnitTest/JIDTest.cpp
index ca3e5ae..0753fb5 100644
--- a/Swiften/JID/UnitTest/JIDTest.cpp
+++ b/Swiften/JID/UnitTest/JIDTest.cpp
@@ -1,8 +1,8 @@
 /*
- * Copyright (c) 2010-2016 Isode Limited.
+ * Copyright (c) 2010-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #include <cppunit/extensions/HelperMacros.h>
 #include <cppunit/extensions/TestFactoryRegistry.h>
@@ -17,17 +17,25 @@ class JIDTest : public CppUnit::TestFixture
         CPPUNIT_TEST(testConstructorWithString);
         CPPUNIT_TEST(testConstructorWithString_Empty);
         CPPUNIT_TEST(testConstructorWithString_NoResource);
         CPPUNIT_TEST(testConstructorWithString_NoNode);
         CPPUNIT_TEST(testConstructorWithString_EmptyResource);
         CPPUNIT_TEST(testConstructorWithString_OnlyDomain);
+        CPPUNIT_TEST(testConstructorWithString_OnlyDomainWithDot);
+        CPPUNIT_TEST(testConstructorWithString_OnlyDomainDotStrippedOff);
+        CPPUNIT_TEST(testConstructorWithString_InvalidOnlyDomainSingleDot);
         CPPUNIT_TEST(testConstructorWithString_InvalidDomain);
+        CPPUNIT_TEST(testConstructorWithString_InvalidDomainEmptyLabel);
         CPPUNIT_TEST(testConstructorWithString_UpperCaseNode);
         CPPUNIT_TEST(testConstructorWithString_UpperCaseDomain);
         CPPUNIT_TEST(testConstructorWithString_UpperCaseResource);
         CPPUNIT_TEST(testConstructorWithString_EmptyNode);
+        CPPUNIT_TEST(testConstructorWithString_EmptyDomain);
+        CPPUNIT_TEST(testConstructorWithString_EmptyDomainWithResource);
+        CPPUNIT_TEST(testConstructorWithString_DotDomain);
+        CPPUNIT_TEST(testConstructorWithString_DotDomainWithResource);
         CPPUNIT_TEST(testConstructorWithString_IllegalResource);
         CPPUNIT_TEST(testConstructorWithString_SpacesInNode);
         CPPUNIT_TEST(testConstructorWithStrings);
         CPPUNIT_TEST(testConstructorWithStrings_EmptyDomain);
         CPPUNIT_TEST(testConstructorWithStrings_EmptyResource);
         CPPUNIT_TEST(testIsBare);
@@ -59,12 +67,13 @@ class JIDTest : public CppUnit::TestFixture
         CPPUNIT_TEST(testHasResource_NoResource);
         CPPUNIT_TEST(testGetEscapedNode);
         CPPUNIT_TEST(testGetEscapedNode_XEP106Examples);
         CPPUNIT_TEST(testGetEscapedNode_BackslashAtEnd);
         CPPUNIT_TEST(testGetUnescapedNode);
         CPPUNIT_TEST(testGetUnescapedNode_XEP106Examples);
+        CPPUNIT_TEST(testStringPrepFailures);
         CPPUNIT_TEST_SUITE_END();
 
     public:
         JIDTest() {}
 
         void testConstructorWithString() {
@@ -116,16 +125,44 @@ class JIDTest : public CppUnit::TestFixture
             CPPUNIT_ASSERT_EQUAL(std::string("bar"), testling.getDomain());
             CPPUNIT_ASSERT_EQUAL(std::string(""), testling.getResource());
             CPPUNIT_ASSERT(testling.isBare());
             CPPUNIT_ASSERT(testling.isValid());
         }
 
+        void testConstructorWithString_OnlyDomainWithDot() {
+            JID testling("bar.");
+
+            CPPUNIT_ASSERT_EQUAL(std::string(""), testling.getNode());
+            CPPUNIT_ASSERT_EQUAL(std::string("bar"), testling.getDomain());
+            CPPUNIT_ASSERT_EQUAL(std::string(""), testling.getResource());
+            CPPUNIT_ASSERT(testling.isBare());
+            CPPUNIT_ASSERT(testling.isValid());
+        }
+
+        void testConstructorWithString_OnlyDomainDotStrippedOff() {
+            JID testling("foo.@bar./resource.");
+
+            CPPUNIT_ASSERT_EQUAL(std::string("foo."), testling.getNode());
+            CPPUNIT_ASSERT_EQUAL(std::string("bar"), testling.getDomain());
+            CPPUNIT_ASSERT_EQUAL(std::string("resource."), testling.getResource());
+            CPPUNIT_ASSERT(!testling.isBare());
+            CPPUNIT_ASSERT(testling.isValid());
+        }
+
+        void testConstructorWithString_InvalidOnlyDomainSingleDot() {
+            CPPUNIT_ASSERT(!JID(".").isValid());
+        }
+
         void testConstructorWithString_InvalidDomain() {
             CPPUNIT_ASSERT(!JID("foo@bar,baz").isValid());
         }
 
+        void testConstructorWithString_InvalidDomainEmptyLabel() {
+            CPPUNIT_ASSERT(!JID("foo@bar..").isValid());
+        }
+
         void testConstructorWithString_UpperCaseNode() {
             JID testling("Fo\xCE\xA9@bar");
 
             CPPUNIT_ASSERT_EQUAL(std::string("fo\xCF\x89"), testling.getNode());
             CPPUNIT_ASSERT_EQUAL(std::string("bar"), testling.getDomain());
             CPPUNIT_ASSERT(testling.isValid());
@@ -148,12 +185,42 @@ class JIDTest : public CppUnit::TestFixture
         void testConstructorWithString_EmptyNode() {
             JID testling("@bar");
 
             CPPUNIT_ASSERT(!testling.isValid());
         }
 
+        void testConstructorWithString_EmptyDomain() {
+            JID testling("bar@");
+
+            CPPUNIT_ASSERT(!testling.isValid());
+        }
+
+        void testStringPrepFailures() {
+            CPPUNIT_ASSERT_EQUAL(false, JID("foo@bar", "example.com").isValid());
+            CPPUNIT_ASSERT_EQUAL(false, JID("foo^", "example*com").isValid());
+            CPPUNIT_ASSERT_EQUAL(false, JID("foobar", "example^com").isValid());
+        }
+
+        void testConstructorWithString_EmptyDomainWithResource() {
+            JID testling("bar@/resource");
+
+            CPPUNIT_ASSERT(!testling.isValid());
+        }
+
+        void testConstructorWithString_DotDomain() {
+            JID testling("bar@.");
+
+            CPPUNIT_ASSERT(!testling.isValid());
+        }
+
+        void testConstructorWithString_DotDomainWithResource() {
+            JID testling("bar@./resource");
+
+            CPPUNIT_ASSERT(!testling.isValid());
+        }
+
         void testConstructorWithString_IllegalResource() {
             JID testling("foo@bar.com/\xd8\xb1\xd9\x85\xd9\x82\xd9\x87\x20\xd8\xaa\xd8\xb1\xd9\x86\xd8\xb3\x20");
 
             CPPUNIT_ASSERT(!testling.isValid());
         }