From fbc02ab3b96fb46231458d2e283cfdd191185fb5 Mon Sep 17 00:00:00 2001 From: Tobias Markmann <tm@ayena.de> Date: Wed, 22 Jun 2016 13:55:05 +0200 Subject: Treat JIDs with empty resources as invalid Cleaned up JID.cpp in process. Test-Information: Adjusted tests accordingly. All tests pass pass on OS X 10.11.5. Change-Id: I774344c8f7378dafe9249217503c082f46890457 diff --git a/Swiften/JID/JID.cpp b/Swiften/JID/JID.cpp index 54084d6..c82674d 100644 --- a/Swiften/JID/JID.cpp +++ b/Swiften/JID/JID.cpp @@ -6,23 +6,21 @@ #define SWIFTEN_CACHE_JID_PREP +#include <sstream> +#include <string> #include <vector> -#include <list> -#include <string> #ifdef SWIFTEN_CACHE_JID_PREP #include <mutex> -#include <boost/unordered_map.hpp> +#include <unordered_map> #endif -#include <boost/assign/list_of.hpp> -#include <boost/algorithm/string/find_format.hpp> -#include <boost/algorithm/string/finder.hpp> + #include <boost/optional.hpp> -#include <sstream> #include <Swiften/Base/String.h> -#include <Swiften/JID/JID.h> #include <Swiften/IDN/IDNConverter.h> +#include <Swiften/JID/JID.h> + #ifndef SWIFTEN_JID_NO_DEFAULT_IDN_CONVERTER #include <memory> #include <Swiften/IDN/PlatformIDNConverter.h> @@ -31,7 +29,7 @@ using namespace Swift; #ifdef SWIFTEN_CACHE_JID_PREP -typedef boost::unordered_map<std::string, std::string> PrepCache; +typedef std::unordered_map<std::string, std::string> PrepCache; static std::mutex namePrepCacheMutex; static PrepCache nodePrepCache; @@ -39,7 +37,7 @@ static PrepCache domainPrepCache; static PrepCache resourcePrepCache; #endif -static const std::list<char> escapedChars = boost::assign::list_of(' ')('"')('&')('\'')('/')('<')('>')('@')(':'); +static const std::vector<char> escapedChars = {' ', '"', '&', '\'', '/', '<', '>', '@', ':'}; static IDNConverter* idnConverter = nullptr; @@ -67,67 +65,6 @@ static bool getEscapeSequenceValue(const std::string& sequence, unsigned char& v return (!s.fail() && !s.bad() && (value == 0x5C || std::find(escapedChars.begin(), escapedChars.end(), value) != escapedChars.end())); } -// Disabling this code for now, since GCC4.5+boost1.42 (on ubuntu) seems to -// result in a bug. Replacing it with naive code. -#if 0 -struct UnescapedCharacterFinder { - template<typename Iterator> boost::iterator_range<Iterator> operator()(Iterator begin, Iterator end) { - for (; begin != end; ++begin) { - if (std::find(escapedChars.begin(), escapedChars.end(), *begin) != escapedChars.end()) { - return boost::iterator_range<Iterator>(begin, begin + 1); - } - else if (*begin == '\\') { - // Check if we have an escaped dissalowed character sequence - Iterator innerBegin = begin + 1; - if (innerBegin != end && innerBegin + 1 != end) { - Iterator innerEnd = innerBegin + 2; - unsigned char value; - if (getEscapeSequenceValue(std::string(innerBegin, innerEnd), value)) { - return boost::iterator_range<Iterator>(begin, begin + 1); - } - } - } - } - return boost::iterator_range<Iterator>(end, end); - } -}; - -struct UnescapedCharacterFormatter { - template<typename FindResult> std::string operator()(const FindResult& match) const { - std::ostringstream s; - s << '\\' << std::hex << static_cast<int>(*match.begin()); - return s.str(); - } -}; - -struct EscapedCharacterFinder { - template<typename Iterator> boost::iterator_range<Iterator> operator()(Iterator begin, Iterator end) { - for (; begin != end; ++begin) { - if (*begin == '\\') { - Iterator innerEnd = begin + 1; - for (size_t i = 0; i < 2 && innerEnd != end; ++i, ++innerEnd) { - } - unsigned char value; - if (getEscapeSequenceValue(std::string(begin + 1, innerEnd), value)) { - return boost::iterator_range<Iterator>(begin, innerEnd); - } - } - } - return boost::iterator_range<Iterator>(end, end); - } -}; - -struct EscapedCharacterFormatter { - template<typename FindResult> std::string operator()(const FindResult& match) const { - unsigned char value; - if (getEscapeSequenceValue(std::string(match.begin() + 1, match.end()), value)) { - return std::string(reinterpret_cast<const char*>(&value), 1); - } - return boost::copy_range<std::string>(match); - } -}; -#endif - namespace Swift { JID::JID(const char* jid) : valid_(true) { @@ -182,6 +119,11 @@ void JID::nameprepAndSetComponents(const std::string& node, const std::string& d valid_ = false; return; } + + if (hasResource_ && resource.empty()) { + valid_ = false; + return; + } #ifndef SWIFTEN_CACHE_JID_PREP node_ = idnConverter->getStringPrepared(node, IDNConverter::XMPPNodePrep); domain_ = idnConverter->getStringPrepared(domain, IDNConverter::NamePrep); @@ -286,7 +228,6 @@ std::string JID::getEscapedNode(const std::string& node) { result += *i; } return result; - //return boost::find_format_all_copy(node, UnescapedCharacterFinder(), UnescapedCharacterFormatter()); } std::string JID::getUnescapedNode() const { @@ -307,7 +248,6 @@ std::string JID::getUnescapedNode() const { ++j; } return result; - //return boost::find_format_all_copy(node_, EscapedCharacterFinder(), EscapedCharacterFormatter()); } void JID::setIDNConverter(IDNConverter* converter) { diff --git a/Swiften/JID/UnitTest/JIDTest.cpp b/Swiften/JID/UnitTest/JIDTest.cpp index 432e7cb..ca3e5ae 100644 --- a/Swiften/JID/UnitTest/JIDTest.cpp +++ b/Swiften/JID/UnitTest/JIDTest.cpp @@ -95,7 +95,7 @@ class JIDTest : public CppUnit::TestFixture void testConstructorWithString_EmptyResource() { JID testling("bar/"); - CPPUNIT_ASSERT(testling.isValid()); + CPPUNIT_ASSERT(!testling.isValid()); CPPUNIT_ASSERT(!testling.isBare()); } @@ -212,8 +212,7 @@ class JIDTest : public CppUnit::TestFixture JID testling("bar/"); CPPUNIT_ASSERT_EQUAL(std::string(""), testling.toBare().getNode()); - CPPUNIT_ASSERT_EQUAL(std::string("bar"), testling.toBare().getDomain()); - CPPUNIT_ASSERT(testling.toBare().isBare()); + CPPUNIT_ASSERT(!testling.isValid()); } void testToString() { @@ -237,7 +236,7 @@ class JIDTest : public CppUnit::TestFixture void testToString_EmptyResource() { JID testling("foo@bar/"); - CPPUNIT_ASSERT_EQUAL(std::string("foo@bar/"), testling.toString()); + CPPUNIT_ASSERT_EQUAL(false, testling.isValid()); } void testCompare_SmallerNode() { @@ -300,14 +299,14 @@ class JIDTest : public CppUnit::TestFixture JID testling1("x@y/"); JID testling2("x@y"); - CPPUNIT_ASSERT_EQUAL(1, testling1.compare(testling2, JID::WithResource)); + CPPUNIT_ASSERT_EQUAL(-1, testling1.compare(testling2, JID::WithResource)); } void testCompare_EmptyResourceAndNoResource() { JID testling1("x@y"); JID testling2("x@y/"); - CPPUNIT_ASSERT_EQUAL(-1, testling1.compare(testling2, JID::WithResource)); + CPPUNIT_ASSERT_EQUAL(1, testling1.compare(testling2, JID::WithResource)); } void testEquals() { @@ -359,9 +358,17 @@ class JIDTest : public CppUnit::TestFixture } void testHasResource_NoResource() { - JID testling("x@y"); + { + JID testling("x@y"); - CPPUNIT_ASSERT(testling.isBare()); + CPPUNIT_ASSERT(testling.isBare()); + CPPUNIT_ASSERT_EQUAL(true, testling.isValid()); + } + + { + JID testling("node@domain/"); + CPPUNIT_ASSERT_EQUAL(false, testling.isValid()); + } } void testGetEscapedNode() { -- cgit v0.10.2-6-g49f6