diff --git a/ChangeLog b/ChangeLog index 535ad4917..93764d75d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -12,6 +12,7 @@ Bug fixes: - #6660 + #6582 Add missing XAtom for utf-8 handling with Xorg - #6814 The system asks to save twice. - #6817 Configure requires dns_sd.h for enterprise version +- #6821 Blocker bugs found by sonar Enhancements: - #6750 Integrate SonarCloud for static analysis and test coverage diff --git a/src/lib/synergy/ProtocolUtil.cpp b/src/lib/synergy/ProtocolUtil.cpp index de18dda21..0ce3df840 100644 --- a/src/lib/synergy/ProtocolUtil.cpp +++ b/src/lib/synergy/ProtocolUtil.cpp @@ -17,6 +17,7 @@ */ #include +#include #include "synergy/ProtocolUtil.h" #include "io/IStream.h" #include "base/Log.h" @@ -29,6 +30,57 @@ // ProtocolUtil // +namespace { + +void +writeInt(UInt32 Value, UInt32 Length, std::vector& Buffer) +{ + switch(Length) + { + case 1: + Buffer.push_back(static_cast(Value & 0xffU)); + break; + case 4: + Buffer.push_back(static_cast((Value >> 24U) & 0xffU)); + Buffer.push_back(static_cast((Value >> 16U) & 0xffU)); + Buffer.push_back(static_cast((Value >> 8U) & 0xffU)); + Buffer.push_back(static_cast( Value & 0xffU)); + break; + case 2: + Buffer.push_back(static_cast((Value >> 8U) & 0xffU)); + Buffer.push_back(static_cast( Value & 0xffU)); + break; + default: + assert(0 && "invalid integer format length"); + return; + } +} + +template +void +writeVectorInt(const std::vector* VectorData, std::vector& Buffer) +{ + if (VectorData) { + const std::vector& Vector = *VectorData; + writeInt((UInt32)Vector.size(), sizeof(UInt32), Buffer); + for (size_t i = 0; i < Vector.size(); ++i) { + writeInt(Vector[i], sizeof(T), Buffer); + } + } +} + +void +writeString(const String* StringData, std::vector& Buffer) +{ + const UInt32 len = (StringData != NULL) ? (UInt32)StringData->size() : 0; + writeInt(len, sizeof(len), Buffer); + if (len != 0) { + std::copy(StringData->begin(), StringData->end(), std::back_inserter(Buffer)); + } +} + +} //namespace + void ProtocolUtil::writef(synergy::IStream* stream, const char* fmt, ...) { @@ -83,18 +135,16 @@ ProtocolUtil::vwritef(synergy::IStream* stream, } // fill buffer - UInt8* buffer = new UInt8[size]; - writef(buffer, fmt, args); + std::vector Buffer; + writef(Buffer, fmt, args); try { // write buffer - stream->write(buffer, size); + stream->write(Buffer.data(), size); LOG((CLOG_DEBUG2 "wrote %d bytes", size)); - - delete[] buffer; } - catch (XBase&) { - delete[] buffer; + catch (const XBase& exception) { + LOG((CLOG_DEBUG2 "Exception <%s> during wrote %d bytes into stream", exception.what(), size)); throw; } } @@ -260,11 +310,11 @@ ProtocolUtil::getLength(const char* fmt, va_list args) return n; } -void -ProtocolUtil::writef(void* buffer, const char* fmt, va_list args) -{ - UInt8* dst = static_cast(buffer); + +void +ProtocolUtil::writef(std::vector& buffer, const char* fmt, va_list args) +{ while (*fmt) { if (*fmt == '%') { // format specifier. determine argument size. @@ -273,30 +323,7 @@ ProtocolUtil::writef(void* buffer, const char* fmt, va_list args) switch (*fmt) { case 'i': { const UInt32 v = va_arg(args, UInt32); - switch (len) { - case 1: - // 1 byte integer - *dst++ = static_cast(v & 0xff); - break; - - case 2: - // 2 byte integer - *dst++ = static_cast((v >> 8) & 0xff); - *dst++ = static_cast( v & 0xff); - break; - - case 4: - // 4 byte integer - *dst++ = static_cast((v >> 24) & 0xff); - *dst++ = static_cast((v >> 16) & 0xff); - *dst++ = static_cast((v >> 8) & 0xff); - *dst++ = static_cast( v & 0xff); - break; - - default: - assert(0 && "invalid integer format length"); - return; - } + writeInt(v, len, buffer); break; } @@ -304,52 +331,22 @@ ProtocolUtil::writef(void* buffer, const char* fmt, va_list args) switch (len) { case 1: { // 1 byte integers - const std::vector* list = - va_arg(args, const std::vector*); - const UInt32 n = (UInt32)list->size(); - *dst++ = static_cast((n >> 24) & 0xff); - *dst++ = static_cast((n >> 16) & 0xff); - *dst++ = static_cast((n >> 8) & 0xff); - *dst++ = static_cast( n & 0xff); - for (UInt32 i = 0; i < n; ++i) { - *dst++ = (*list)[i]; - } + const std::vector* list = va_arg(args, const std::vector*); + writeVectorInt(list, buffer); break; } case 2: { // 2 byte integers - const std::vector* list = - va_arg(args, const std::vector*); - const UInt32 n = (UInt32)list->size(); - *dst++ = static_cast((n >> 24) & 0xff); - *dst++ = static_cast((n >> 16) & 0xff); - *dst++ = static_cast((n >> 8) & 0xff); - *dst++ = static_cast( n & 0xff); - for (UInt32 i = 0; i < n; ++i) { - const UInt16 v = (*list)[i]; - *dst++ = static_cast((v >> 8) & 0xff); - *dst++ = static_cast( v & 0xff); - } + const std::vector* list = va_arg(args, const std::vector*); + writeVectorInt(list, buffer); break; } case 4: { // 4 byte integers - const std::vector* list = - va_arg(args, const std::vector*); - const UInt32 n = (UInt32)list->size(); - *dst++ = static_cast((n >> 24) & 0xff); - *dst++ = static_cast((n >> 16) & 0xff); - *dst++ = static_cast((n >> 8) & 0xff); - *dst++ = static_cast( n & 0xff); - for (UInt32 i = 0; i < n; ++i) { - const UInt32 v = (*list)[i]; - *dst++ = static_cast((v >> 24) & 0xff); - *dst++ = static_cast((v >> 16) & 0xff); - *dst++ = static_cast((v >> 8) & 0xff); - *dst++ = static_cast( v & 0xff); - } + const std::vector* list = va_arg(args, const std::vector*); + writeVectorInt(list, buffer); break; } @@ -363,15 +360,7 @@ ProtocolUtil::writef(void* buffer, const char* fmt, va_list args) case 's': { assert(len == 0); const String* src = va_arg(args, String*); - const UInt32 len = (src != NULL) ? (UInt32)src->size() : 0; - *dst++ = static_cast((len >> 24) & 0xff); - *dst++ = static_cast((len >> 16) & 0xff); - *dst++ = static_cast((len >> 8) & 0xff); - *dst++ = static_cast( len & 0xff); - if (len != 0) { - memcpy(dst, src->data(), len); - dst += len; - } + writeString(src, buffer); break; } @@ -379,18 +368,14 @@ ProtocolUtil::writef(void* buffer, const char* fmt, va_list args) assert(len == 0); const UInt32 len = va_arg(args, UInt32); const UInt8* src = va_arg(args, UInt8*); - *dst++ = static_cast((len >> 24) & 0xff); - *dst++ = static_cast((len >> 16) & 0xff); - *dst++ = static_cast((len >> 8) & 0xff); - *dst++ = static_cast( len & 0xff); - memcpy(dst, src, len); - dst += len; + writeInt(len, sizeof(len), buffer); + std::copy(src, src + len, std::back_inserter(buffer)); break; } case '%': assert(len == 0); - *dst++ = '%'; + buffer.push_back('%'); break; default: @@ -402,7 +387,7 @@ ProtocolUtil::writef(void* buffer, const char* fmt, va_list args) } else { // copy regular character - *dst++ = *fmt++; + buffer.push_back(*fmt++); } } } diff --git a/src/lib/synergy/ProtocolUtil.h b/src/lib/synergy/ProtocolUtil.h index a49da3096..c903366ad 100644 --- a/src/lib/synergy/ProtocolUtil.h +++ b/src/lib/synergy/ProtocolUtil.h @@ -79,7 +79,7 @@ private: const char* fmt, va_list); static UInt32 getLength(const char* fmt, va_list); - static void writef(void*, const char* fmt, va_list); + static void writef(std::vector&, const char* fmt, va_list); static UInt32 eatLength(const char** fmt); static void read(synergy::IStream*, void*, UInt32); diff --git a/src/test/unittests/synergy/GenericArgsParsingTests.cpp b/src/test/unittests/synergy/GenericArgsParsingTests.cpp index 32ee0db71..57d78e71c 100644 --- a/src/test/unittests/synergy/GenericArgsParsingTests.cpp +++ b/src/test/unittests/synergy/GenericArgsParsingTests.cpp @@ -47,10 +47,10 @@ TEST(GenericArgsParsingTests, parseGenericArgs_logLevelCmd_setLogLevel) const int argc = 3; const char* kLogLevelCmd[argc] = { "stub", "--debug", "DEBUG" }; - ArgParser argParser(NULL); lib::synergy::ArgsBase argsBase; + ArgParser argParser(NULL); argParser.setArgsBase(argsBase); - + argParser.parseGenericArgs(argc, kLogLevelCmd, i); String logFilter(argsBase.m_logFilter); @@ -65,10 +65,10 @@ TEST(GenericArgsParsingTests, parseGenericArgs_logFileCmd_saveLogFilename) const int argc = 3; const char* kLogFileCmd[argc] = { "stub", "--log", "mock_filename" }; - ArgParser argParser(NULL); lib::synergy::ArgsBase argsBase; + ArgParser argParser(NULL); argParser.setArgsBase(argsBase); - + argParser.parseGenericArgs(argc, kLogFileCmd, i); String logFile(argsBase.m_logFile); @@ -83,10 +83,10 @@ TEST(GenericArgsParsingTests, parseGenericArgs_logFileCmdWithSpace_saveLogFilena const int argc = 3; const char* kLogFileCmdWithSpace[argc] = { "stub", "--log", "mo ck_filename" }; - ArgParser argParser(NULL); lib::synergy::ArgsBase argsBase; + ArgParser argParser(NULL); argParser.setArgsBase(argsBase); - + argParser.parseGenericArgs(argc, kLogFileCmdWithSpace, i); String logFile(argsBase.m_logFile); @@ -101,10 +101,10 @@ TEST(GenericArgsParsingTests, parseGenericArgs_noDeamonCmd_daemonFalse) const int argc = 2; const char* kNoDeamonCmd[argc] = { "stub", "-f" }; - ArgParser argParser(NULL); lib::synergy::ArgsBase argsBase; + ArgParser argParser(NULL); argParser.setArgsBase(argsBase); - + argParser.parseGenericArgs(argc, kNoDeamonCmd, i); EXPECT_FALSE(argsBase.m_daemon); @@ -117,8 +117,8 @@ TEST(GenericArgsParsingTests, parseGenericArgs_deamonCmd_daemonTrue) const int argc = 2; const char* kDeamonCmd[argc] = { "stub", "--daemon" }; - ArgParser argParser(NULL); lib::synergy::ArgsBase argsBase; + ArgParser argParser(NULL); argParser.setArgsBase(argsBase); argParser.parseGenericArgs(argc, kDeamonCmd, i); @@ -165,8 +165,8 @@ TEST(GenericArgsParsingTests, parseGenericArgs_restartCmd_restartTrue) const int argc = 2; const char* kRestartCmd[argc] = { "stub", "--restart" }; - ArgParser argParser(NULL); lib::synergy::ArgsBase argsBase; + ArgParser argParser(NULL); argParser.setArgsBase(argsBase); argParser.parseGenericArgs(argc, kRestartCmd, i); @@ -181,10 +181,9 @@ TEST(GenericArgsParsingTests, parseGenericArgs_backendCmd_backendTrue) const int argc = 2; const char* kBackendCmd[argc] = { "stub", "-z" }; - ArgParser argParser(NULL); lib::synergy::ArgsBase argsBase; + ArgParser argParser(NULL); argParser.setArgsBase(argsBase); - argParser.parseGenericArgs(argc, kBackendCmd, i); EXPECT_EQ(true, argsBase.m_backend); @@ -197,10 +196,9 @@ TEST(GenericArgsParsingTests, parseGenericArgs_noHookCmd_noHookTrue) const int argc = 2; const char* kNoHookCmd[argc] = { "stub", "--no-hooks" }; - ArgParser argParser(NULL); lib::synergy::ArgsBase argsBase; + ArgParser argParser(NULL); argParser.setArgsBase(argsBase); - argParser.parseGenericArgs(argc, kNoHookCmd, i); EXPECT_EQ(true, argsBase.m_noHooks); @@ -215,11 +213,11 @@ TEST(GenericArgsParsingTests, parseGenericArgs_helpCmd_showHelp) const char* kHelpCmd[argc] = { "stub", "--help" }; NiceMock app; - ArgParser argParser(&app); lib::synergy::ArgsBase argsBase; + ArgParser argParser(&app); argParser.setArgsBase(argsBase); ON_CALL(app, help()).WillByDefault(Invoke(showMockHelp)); - + argParser.parseGenericArgs(argc, kHelpCmd, i); EXPECT_EQ(true, g_helpShowed); @@ -235,8 +233,8 @@ TEST(GenericArgsParsingTests, parseGenericArgs_versionCmd_showVersion) const char* kVersionCmd[argc] = { "stub", "--version" }; NiceMock app; - ArgParser argParser(&app); lib::synergy::ArgsBase argsBase; + ArgParser argParser(&app); argParser.setArgsBase(argsBase); ON_CALL(app, version()).WillByDefault(Invoke(showMockVersion)); @@ -252,8 +250,8 @@ TEST(GenericArgsParsingTests, parseGenericArgs_noTrayCmd_disableTrayTrue) const int argc = 2; const char* kNoTrayCmd[argc] = { "stub", "--no-tray" }; - ArgParser argParser(NULL); lib::synergy::ArgsBase argsBase; + ArgParser argParser(NULL); argParser.setArgsBase(argsBase); argParser.parseGenericArgs(argc, kNoTrayCmd, i); @@ -268,8 +266,8 @@ TEST(GenericArgsParsingTests, parseGenericArgs_ipcCmd_enableIpcTrue) const int argc = 2; const char* kIpcCmd[argc] = { "stub", "--ipc" }; - ArgParser argParser(NULL); lib::synergy::ArgsBase argsBase; + ArgParser argParser(NULL); argParser.setArgsBase(argsBase); argParser.parseGenericArgs(argc, kIpcCmd, i); @@ -285,8 +283,8 @@ TEST(GenericArgsParsingTests, parseGenericArgs_dragDropCmdOnNonLinux_enableDragD const int argc = 2; const char* kDragDropCmd[argc] = { "stub", "--enable-drag-drop" }; - ArgParser argParser(NULL); lib::synergy::ArgsBase argsBase; + ArgParser argParser(NULL); argParser.setArgsBase(argsBase); argParser.parseGenericArgs(argc, kDragDropCmd, i); @@ -303,8 +301,8 @@ TEST(GenericArgsParsingTests, parseGenericArgs_dragDropCmdOnLinux_enableDragDrop const int argc = 2; const char* kDragDropCmd[argc] = { "stub", "--enable-drag-drop" }; - ArgParser argParser(NULL); lib::synergy::ArgsBase argsBase; + ArgParser argParser(NULL); argParser.setArgsBase(argsBase); argParser.parseGenericArgs(argc, kDragDropCmd, i); diff --git a/src/test/unittests/synergy/ProtocolUtilTests.cpp b/src/test/unittests/synergy/ProtocolUtilTests.cpp index 5b7ec40cf..fea36a76e 100644 --- a/src/test/unittests/synergy/ProtocolUtilTests.cpp +++ b/src/test/unittests/synergy/ProtocolUtilTests.cpp @@ -24,15 +24,113 @@ using ::testing::_; using ::testing::Return; using ::testing::DoAll; using ::testing::SetArgPointee; +using ::testing::Pointee; using ::testing::Eq; using ::testing::StrEq; using ::testing::TypedEq; +using ::testing::ElementsAreArray; ACTION_P2(SetValueToVoidPointerArg0, value, size) { memcpy(arg0, value, size); } +MATCHER_P(EqVoidPointeeInt8, expected, "") +{ + const UInt8 Actual8 = (*static_cast(arg)); + return (expected == Actual8); +} + +MATCHER_P(EqVoidPointeeInt16, expected, "") +{ + const UInt16 Actual16 = (*static_cast(arg)); + return (expected == (Actual16 >> 8)); +} + +MATCHER_P(EqVoidPointeeInt32, expected, "") +{ + const UInt32 Actual32 = (*static_cast(arg)); + return (expected == (Actual32 >> 24)); +} + +MATCHER_P(EqVoidVectorInt1byte, expected, "") +{ + bool Result = true; + const UInt8* Actual = (static_cast(arg)) + sizeof (UInt32); + const size_t Size = *(Actual - 1); + + if (Size == expected.size()){ + for(size_t i = 0; i < expected.size(); ++i){ + if (expected[i] != Actual[i]){ + Result = false; + break; + } + } + } + else{ + Result = false; + } + + return Result; +} + +MATCHER_P(EqVoidVectorInt2bytes, expected, "") +{ + bool Result = true; + const UInt16* Actual = (static_cast(arg)) + sizeof (UInt16); + const size_t Size = *(Actual - 1) >> 8; + + if (Size == expected.size()){ + for(size_t i = 0; i < expected.size(); ++i){ + if (expected[i] != (Actual[i] >> 8) ){ + Result = false; + break; + } + } + } + else{ + Result = false; + } + + return Result; +} + +MATCHER_P(EqVoidVectorInt4bytes, expected, "") +{ + bool Result = true; + const UInt32* Actual = (static_cast(arg)) + 1; + const size_t Size = *(Actual - 1) >> 24; + + if (Size == expected.size()){ + for(size_t i = 0; i < expected.size(); ++i){ + if (expected[i] != (Actual[i] >> 24) ){ + Result = false; + break; + } + } + } + else{ + Result = false; + } + + return Result; +} + +MATCHER_P(EqVectorSymbols, expected, "") +{ + bool Result = true; + const UInt8* Actual = (static_cast(arg)); + + for(size_t i = 0; i < expected.size(); ++i){ + if (expected[i] != (Actual[i]) ){ + Result = false; + break; + } + } + + return Result; +} + ACTION(ThrowBadAlloc) { throw std::bad_alloc(); @@ -420,3 +518,102 @@ TEST_F(ProtocolUtilTests, readf_vector_int4bytes_and_string) EXPECT_EQ(Expected4Bytes, Actual4Bytes); } +class WriteIntTest : public ::testing::TestWithParam< std::tuple > +{ +public: + MockStream stream; + UInt8 Expected1Byte = 5; + UInt16 Expected2Bytes = 10; + UInt32 Expected4Bytes = 15; +}; + +TEST_P(WriteIntTest, write_int) +{ + const char* Format = std::get<0>(GetParam()); + const int DataSize = std::get<1>(GetParam()); + switch(DataSize) + { + case 2: + EXPECT_CALL(stream, write(EqVoidPointeeInt16(Expected2Bytes), Eq(DataSize))); + ProtocolUtil::writef(&stream, Format, Expected2Bytes); + break; + case 4: + EXPECT_CALL(stream, write(EqVoidPointeeInt32(Expected4Bytes), Eq(DataSize))); + ProtocolUtil::writef(&stream, Format, Expected4Bytes); + break; + default: + EXPECT_CALL(stream, write(EqVoidPointeeInt8(Expected1Byte), Eq(DataSize))); + ProtocolUtil::writef(&stream, Format, Expected1Byte); + break; + } +} + +INSTANTIATE_TEST_CASE_P( + WriteIntTest, + WriteIntTest, + ::testing::Values( + std::make_tuple("%1i", 1), + std::make_tuple("%2i", 2), + std::make_tuple("%4i", 4))); + +class WriteIntVectorTest : public ::testing::TestWithParam< std::tuple > +{ +public: + MockStream stream; + const std::vector Expected1Byte = {10, 20, 30}; + const std::vector Expected2Byte = {40, 50, 60}; + const std::vector Expected4Byte = {70, 80, 90}; +}; + +TEST_P(WriteIntVectorTest, write_vector_int) +{ + const char* Format = std::get<0>(GetParam()); + const int Type = std::get<1>(GetParam()); + + switch(Type){ + case 2: + EXPECT_CALL(stream, write(EqVoidVectorInt2bytes(Expected2Byte), Eq(Type * Expected2Byte.size() + sizeof(UInt32)))); + ProtocolUtil::writef(&stream, Format, &Expected2Byte); + break; + case 4: + EXPECT_CALL(stream, write(EqVoidVectorInt4bytes(Expected4Byte), Eq(Type * Expected4Byte.size() + sizeof(UInt32)))); + ProtocolUtil::writef(&stream, Format, &Expected4Byte); + break; + default: + EXPECT_CALL(stream, write(EqVoidVectorInt1byte(Expected1Byte), Eq(Expected1Byte.size() + sizeof(UInt32)))); + ProtocolUtil::writef(&stream, Format, &Expected1Byte); + break; + } +} + +INSTANTIATE_TEST_CASE_P( + WriteIntVectorTest, + WriteIntVectorTest, + ::testing::Values( + std::make_tuple("%1I", 1), + std::make_tuple("%2I", 2), + std::make_tuple("%4I", 4))); + +TEST_F(ProtocolUtilTests, write_string_test) +{ + const String Expected = "Expected"; + const std::vector ExpectedVector = {'E', 'x', 'p', 'e', 'c', 't', 'e', 'd'}; + EXPECT_CALL(stream, write(EqVoidVectorInt1byte(ExpectedVector), Expected.size() + sizeof (UInt32))); + ProtocolUtil::writef(&stream, "%s", &Expected); +} + +TEST_F(ProtocolUtilTests, write_raw_bytes_test) +{ + const UInt32 Size = 5; + const std::array Expected = {10, 20, 30, 40, 50}; + EXPECT_CALL(stream, write(EqVoidVectorInt1byte(Expected), Expected.size() + sizeof (UInt32))); + ProtocolUtil::writef(&stream, "%S", Size, &Expected); +} + +TEST_F(ProtocolUtilTests, write_symbols_from_format_test) +{ + const std::vector Expected = {'%', '1', '2', '3', '4', '5'}; + EXPECT_CALL(stream, write(EqVectorSymbols(Expected), Expected.size())); + ProtocolUtil::writef(&stream, "%%12345"); +} +