8 files changed, 113 insertions, 78 deletions
diff --git a/Swiften/IDN/ICUConverter.cpp b/Swiften/IDN/ICUConverter.cpp
index d6b0827..37ce708 100644
--- a/Swiften/IDN/ICUConverter.cpp
+++ b/Swiften/IDN/ICUConverter.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2016 Isode Limited.
+ * Copyright (c) 2012-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -19,6 +19,8 @@ using namespace Swift;
 using boost::numeric_cast;
 
 namespace {
+    static constexpr auto maxStringPrepLength = 1023;
+
     typedef std::vector<UChar, SafeAllocator<UChar> > ICUString;
 
     const char* toConstCharArray(const std::string& input) {
@@ -93,15 +95,8 @@ namespace {
         ICUString icuInput = convertToICUString(s);
         ICUString icuResult;
         UParseError parseError;
-        icuResult.resize(icuInput.size());
+        icuResult.resize(maxStringPrepLength);
         int32_t icuResultLength = usprep_prepare(icuProfile.get(), vecptr(icuInput), numeric_cast<int32_t>(icuInput.size()), vecptr(icuResult), numeric_cast<int32_t>(icuResult.size()), USPREP_ALLOW_UNASSIGNED, &parseError, &status);
-        icuResult.resize(numeric_cast<size_t>(icuResultLength));
-        if (status == U_BUFFER_OVERFLOW_ERROR) {
-            status = U_ZERO_ERROR;
-            icuResult.resize(numeric_cast<size_t>(icuResultLength));
-            icuResultLength = usprep_prepare(icuProfile.get(), vecptr(icuInput), numeric_cast<int32_t>(icuInput.size()), vecptr(icuResult), numeric_cast<int32_t>(icuResult.size()), USPREP_ALLOW_UNASSIGNED, &parseError, &status);
-            icuResult.resize(numeric_cast<size_t>(icuResultLength));
-        }
         if (U_FAILURE(status)) {
             return std::vector<char, SafeAllocator<char> >();
         }
diff --git a/Swiften/IDN/ICUConverter.h b/Swiften/IDN/ICUConverter.h
index 0a0b0d3..b0f5d85 100644
--- a/Swiften/IDN/ICUConverter.h
+++ b/Swiften/IDN/ICUConverter.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2016 Isode Limited.
+ * Copyright (c) 2012-2017 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -9,15 +9,14 @@
 #include <string>
 
 #include <Swiften/Base/API.h>
-#include <Swiften/Base/Override.h>
 #include <Swiften/IDN/IDNConverter.h>
 
 namespace Swift {
     class SWIFTEN_API ICUConverter : public IDNConverter {
         public:
-            virtual std::string getStringPrepared(const std::string& s, StringPrepProfile profile) SWIFTEN_OVERRIDE;
-            virtual SafeByteArray getStringPrepared(const SafeByteArray& s, StringPrepProfile profile) SWIFTEN_OVERRIDE;
+            virtual std::string getStringPrepared(const std::string& s, StringPrepProfile profile) override;
+            virtual SafeByteArray getStringPrepared(const SafeByteArray& s, StringPrepProfile profile) override;
 
-            virtual boost::optional<std::string> getIDNAEncoded(const std::string& s) SWIFTEN_OVERRIDE;
+            virtual boost::optional<std::string> getIDNAEncoded(const std::string& s) override;
     };
 }
diff --git a/Swiften/IDN/LibIDNConverter.cpp b/Swiften/IDN/LibIDNConverter.cpp
index 0c01352..e2a87be 100644
--- a/Swiften/IDN/LibIDNConverter.cpp
+++ b/Swiften/IDN/LibIDNConverter.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2016 Isode Limited.
+ * Copyright (c) 2012-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -24,7 +24,7 @@ extern "C" {
 using namespace Swift;
 
 namespace {
-    static const int MAX_STRINGPREP_SIZE = 1024;
+    static const size_t MAX_STRINGPREP_SIZE = 1024;
 
     const Stringprep_profile* getLibIDNProfile(IDNConverter::StringPrepProfile profile) {
         switch(profile) {
@@ -44,7 +44,8 @@ namespace {
             return ContainerType();
         }
 
-        input.resize(MAX_STRINGPREP_SIZE);
+        // Ensure we have enough space for stringprepping, and that input is always NUL terminated
+        input.resize(std::max(MAX_STRINGPREP_SIZE, input.size() + 1));
         if (stringprep(&input[0], MAX_STRINGPREP_SIZE, static_cast<Stringprep_profile_flags>(0), getLibIDNProfile(profile)) == 0) {
             return input;
         }
@@ -77,6 +78,9 @@ boost::optional<std::string> LibIDNConverter::getIDNAEncoded(const std::string&
     if (idna_to_ascii_8z(domain.c_str(), &output, IDNA_USE_STD3_ASCII_RULES) == IDNA_SUCCESS) {
         std::string result(output);
         free(output);
+        if (result.size() > 255) {
+            return boost::optional<std::string>();
+        }
         return result;
     }
     else {
diff --git a/Swiften/IDN/LibIDNConverter.h b/Swiften/IDN/LibIDNConverter.h
index 3f1d1f7..5553ab3 100644
--- a/Swiften/IDN/LibIDNConverter.h
+++ b/Swiften/IDN/LibIDNConverter.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2016 Isode Limited.
+ * Copyright (c) 2012-2017 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -9,16 +9,15 @@
 #include <string>
 
 #include <Swiften/Base/API.h>
-#include <Swiften/Base/Override.h>
 #include <Swiften/IDN/IDNConverter.h>
 
 namespace Swift {
     class SWIFTEN_API LibIDNConverter : public IDNConverter {
         public:
-            virtual std::string getStringPrepared(const std::string& s, StringPrepProfile profile) SWIFTEN_OVERRIDE;
-            virtual SafeByteArray getStringPrepared(const SafeByteArray& s, StringPrepProfile profile) SWIFTEN_OVERRIDE;
+            virtual std::string getStringPrepared(const std::string& s, StringPrepProfile profile) override;
+            virtual SafeByteArray getStringPrepared(const SafeByteArray& s, StringPrepProfile profile) override;
 
-            virtual boost::optional<std::string> getIDNAEncoded(const std::string& s) SWIFTEN_OVERRIDE;
+            virtual boost::optional<std::string> getIDNAEncoded(const std::string& s) override;
     };
 }
 
diff --git a/Swiften/IDN/PlatformIDNConverter.cpp b/Swiften/IDN/PlatformIDNConverter.cpp
index c85d3b6..3564bb9 100644
--- a/Swiften/IDN/PlatformIDNConverter.cpp
+++ b/Swiften/IDN/PlatformIDNConverter.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2016 Isode Limited.
+ * Copyright (c) 2012-2019 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -13,16 +13,17 @@
 
 namespace Swift {
 
-IDNConverter* PlatformIDNConverter::create() {
+std::unique_ptr<IDNConverter> PlatformIDNConverter::create() {
 #if defined(HAVE_LIBIDN)
-    return new LibIDNConverter();
+    return std::make_unique<LibIDNConverter>();
 #elif defined(HAVE_ICU)
-    return new ICUConverter();
+    return std::make_unique<ICUConverter>();
 #else
 #if defined(NEED_IDN)
 #error "No IDN implementation"
+#else
+    return {};
 #endif
-    return nullptr;
 #endif
 }
 
diff --git a/Swiften/IDN/PlatformIDNConverter.h b/Swiften/IDN/PlatformIDNConverter.h
index 704f7a7..3b9a275 100644
--- a/Swiften/IDN/PlatformIDNConverter.h
+++ b/Swiften/IDN/PlatformIDNConverter.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012 Isode Limited.
+ * Copyright (c) 2012-2019 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -8,10 +8,12 @@
 
 #include <Swiften/Base/API.h>
 
+#include <memory>
+
 namespace Swift {
     class IDNConverter;
 
     namespace PlatformIDNConverter {
-        SWIFTEN_API IDNConverter* create();
+        SWIFTEN_API std::unique_ptr<IDNConverter> create();
     }
 }
diff --git a/Swiften/IDN/SConscript b/Swiften/IDN/SConscript
index 28596f7..0afad0e 100644
--- a/Swiften/IDN/SConscript
+++ b/Swiften/IDN/SConscript
@@ -23,6 +23,7 @@ swiften_env.Append(SWIFTEN_OBJECTS = [objects])
 if env["TEST"] :
     test_env = myenv.Clone()
     test_env.UseFlags(swiften_env["CPPUNIT_FLAGS"])
+    test_env.UseFlags(myenv.get("GOOGLETEST_FLAGS", ""))
     env.Append(UNITTEST_OBJECTS = test_env.SwiftenObject([
                 File("UnitTest/IDNConverterTest.cpp"),
                 File("UnitTest/UTF8ValidatorTest.cpp")
diff --git a/Swiften/IDN/UnitTest/IDNConverterTest.cpp b/Swiften/IDN/UnitTest/IDNConverterTest.cpp
index 508a28c..77a1ece 100644
--- a/Swiften/IDN/UnitTest/IDNConverterTest.cpp
+++ b/Swiften/IDN/UnitTest/IDNConverterTest.cpp
@@ -1,64 +1,98 @@
 /*
- * Copyright (c) 2010-2016 Isode Limited.
+ * Copyright (c) 2010-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #include <memory>
 
-#include <cppunit/extensions/HelperMacros.h>
-#include <cppunit/extensions/TestFactoryRegistry.h>
+#include <gtest/gtest.h>
 
 #include <Swiften/IDN/IDNConverter.h>
 #include <Swiften/IDN/PlatformIDNConverter.h>
 
 using namespace Swift;
 
-class IDNConverterTest : public CppUnit::TestFixture {
-        CPPUNIT_TEST_SUITE(IDNConverterTest);
-        CPPUNIT_TEST(testStringPrep);
-        CPPUNIT_TEST(testStringPrep_Empty);
-        CPPUNIT_TEST(testGetEncoded);
-        CPPUNIT_TEST(testGetEncoded_International);
-        CPPUNIT_TEST(testGetEncoded_Invalid);
-        CPPUNIT_TEST_SUITE_END();
-
-    public:
-        void setUp() {
-            testling = std::shared_ptr<IDNConverter>(PlatformIDNConverter::create());
-        }
-
-        void testStringPrep() {
-            std::string result = testling->getStringPrepared("tron\xc3\x87on", IDNConverter::NamePrep);
-
-            CPPUNIT_ASSERT_EQUAL(std::string("tron\xc3\xa7on"), result);
-        }
-
-        void testStringPrep_Empty() {
-            CPPUNIT_ASSERT_EQUAL(std::string(""), testling->getStringPrepared("", IDNConverter::NamePrep));
-            CPPUNIT_ASSERT_EQUAL(std::string(""), testling->getStringPrepared("", IDNConverter::XMPPNodePrep));
-            CPPUNIT_ASSERT_EQUAL(std::string(""), testling->getStringPrepared("", IDNConverter::XMPPResourcePrep));
-        }
-
-        void testGetEncoded() {
-            boost::optional<std::string> result = testling->getIDNAEncoded("www.swift.im");
-            CPPUNIT_ASSERT(!!result);
-            CPPUNIT_ASSERT_EQUAL(std::string("www.swift.im"), *result);
-        }
-
-        void testGetEncoded_International() {
-            boost::optional<std::string> result = testling->getIDNAEncoded("www.tron\xc3\x87on.com");
-            CPPUNIT_ASSERT(!!result);
-            CPPUNIT_ASSERT_EQUAL(std::string("www.xn--tronon-zua.com"), *result);
-        }
-
-        void testGetEncoded_Invalid() {
-            boost::optional<std::string> result = testling->getIDNAEncoded("www.foo,bar.com");
-            CPPUNIT_ASSERT(!result);
-        }
-
-    private:
-        std::shared_ptr<IDNConverter> testling;
+class IDNConverterTest : public ::testing::Test {
+
+protected:
+    virtual void SetUp() {
+        testling_ = std::shared_ptr<IDNConverter>(PlatformIDNConverter::create());
+    }
+
+    std::shared_ptr<IDNConverter> testling_;
 };
 
-CPPUNIT_TEST_SUITE_REGISTRATION(IDNConverterTest);
+TEST_F(IDNConverterTest, testStringPrep) {
+    std::string result = testling_->getStringPrepared("tron\xc3\x87on", IDNConverter::NamePrep);
+
+    ASSERT_EQ(std::string("tron\xc3\xa7on"), result);
+}
+
+TEST_F(IDNConverterTest, testStringPrep_Empty) {
+    ASSERT_EQ(std::string(""), testling_->getStringPrepared("", IDNConverter::NamePrep));
+    ASSERT_EQ(std::string(""), testling_->getStringPrepared("", IDNConverter::XMPPNodePrep));
+    ASSERT_EQ(std::string(""), testling_->getStringPrepared("", IDNConverter::XMPPResourcePrep));
+}
+
+TEST_F(IDNConverterTest, testStringPrep_MaximumOutputSize) {
+    const std::string input(1023, 'x');
+    ASSERT_EQ(input, testling_->getStringPrepared(input, IDNConverter::NamePrep));
+    ASSERT_EQ(input, testling_->getStringPrepared(input, IDNConverter::XMPPNodePrep));
+    ASSERT_EQ(input, testling_->getStringPrepared(input, IDNConverter::XMPPResourcePrep));
+}
+
+TEST_F(IDNConverterTest, testStringPrep_TooLong) {
+    const std::string input(1024, 'x');
+    ASSERT_THROW(testling_->getStringPrepared(input, IDNConverter::NamePrep), std::exception);
+    ASSERT_THROW(testling_->getStringPrepared(input, IDNConverter::XMPPNodePrep), std::exception);
+    ASSERT_THROW(testling_->getStringPrepared(input, IDNConverter::XMPPResourcePrep), std::exception);
+}
+
+TEST_F(IDNConverterTest, testStringPrep_ShrinkingBelow1023) {
+    std::string input;
+    std::string expected;
+    // The four byte \u03b1\u0313 UTF-8 string will shrink to the three byte \u1f00
+    for (auto i = 0; i < 300; ++i) {
+        input +="\xce\xb1\xcc\x93"; // UTF-8 repesentation of U+03B1 U+0313
+        expected += "\xe1\xbc\x80"; // UTF-8 representation of U+1F00
+    }
+    ASSERT_EQ(expected, testling_->getStringPrepared(input, IDNConverter::NamePrep));
+    ASSERT_EQ(expected, testling_->getStringPrepared(input, IDNConverter::XMPPNodePrep));
+    ASSERT_EQ(expected, testling_->getStringPrepared(input, IDNConverter::XMPPResourcePrep));
+}
+
+TEST_F(IDNConverterTest, testGetEncoded) {
+    boost::optional<std::string> result = testling_->getIDNAEncoded("www.swift.im");
+    ASSERT_TRUE(!!result);
+    ASSERT_EQ(std::string("www.swift.im"), *result);
+}
+
+TEST_F(IDNConverterTest, testGetEncoded_International) {
+    boost::optional<std::string> result = testling_->getIDNAEncoded("www.tron\xc3\x87on.com");
+    ASSERT_TRUE(result);
+    ASSERT_EQ(std::string("www.xn--tronon-zua.com"), *result);
+}
+
+TEST_F(IDNConverterTest, testGetEncoded_Invalid) {
+    boost::optional<std::string> result = testling_->getIDNAEncoded("www.foo,bar.com");
+    ASSERT_FALSE(result);
+}
+
+TEST_F(IDNConverterTest, testRFC1035LengthRestrictions) {
+    // label size check, 63 octets or less
+    ASSERT_TRUE(testling_->getIDNAEncoded(std::string(63, 'a') + ".example"));
+    ASSERT_TRUE(testling_->getIDNAEncoded(std::string(63, 'a') + "." + std::string(63, 'a') + ".example"));
+    ASSERT_FALSE(testling_->getIDNAEncoded(std::string(64, 'a') + "." + std::string(63, 'a') + ".example"));
+    ASSERT_FALSE(testling_->getIDNAEncoded(std::string(63, 'a') + "." + std::string(64, 'a') + ".example"));
+    ASSERT_FALSE(testling_->getIDNAEncoded(std::string(0, 'a') + "." + std::string(63, 'a') + ".example"));
+    ASSERT_FALSE(testling_->getIDNAEncoded(std::string(63, 'a') + "." + std::string(0, 'a') + ".example"));
+
+    // domain name 255 octets or less
+    ASSERT_TRUE(testling_->getIDNAEncoded(std::string(63, 'a') + ".example"));
+    ASSERT_TRUE(testling_->getIDNAEncoded(std::string(63, 'a') + "." + std::string(63, 'a') + ".example"));
+    ASSERT_TRUE(testling_->getIDNAEncoded(std::string(63, 'a') + "." + std::string(63, 'a') + "." + std::string(63, 'a') + ".example"));
+    ASSERT_TRUE(testling_->getIDNAEncoded(std::string(63, 'a') + "." + std::string(63, 'a') + "." + std::string(63, 'a') + "." + std::string(55, 'a') + ".example"));
+    ASSERT_FALSE(testling_->getIDNAEncoded(std::string(63, 'a') + "." + std::string(63, 'a') + "." + std::string(63, 'a') + "." + std::string(56, 'a') + ".example"));
+    ASSERT_FALSE(testling_->getIDNAEncoded(std::string(63, 'a') + "." + std::string(56, 'a') + "." + std::string(63, 'a') + "." + std::string(63, 'a') + ".example"));
+}