smw/deskflow
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

SYNERGY-221 Blocker bugs (#6821)

Browse source 
* SYNERGY-221 Blocker bugs

* SYNERGY-221 Compilation fix

* Update ChangeLog

* SYNERGY-221 Exception message has been fixed.

* Update src/lib/synergy/ProtocolUtil.cpp

Co-authored-by: Jnewbon <48688400+Jnewbon@users.noreply.github.com>

* SYNERGY-221 Fix code smells

Co-authored-by: Jnewbon <48688400+Jnewbon@users.noreply.github.com>
This commit is contained in:
SerhiiGadzhilov 2020-10-30 19:21:03 +03:00 committed by GitHub
parent 26f2f2c283
commit d9d833fcb8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 291 additions and 110 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
ChangeLog
View file

@ -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

161
src/lib/synergy/ProtocolUtil.cpp
View file

@ -17,6 +17,7 @@
*/
#include <array>
#include <iterator>
#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<UInt8>& Buffer)
{
switch(Length)
{
case 1:
Buffer.push_back(static_cast<UInt8>(Value & 0xffU));
break;
case 4:
Buffer.push_back(static_cast<UInt8>((Value >> 24U) & 0xffU));
Buffer.push_back(static_cast<UInt8>((Value >> 16U) & 0xffU));
Buffer.push_back(static_cast<UInt8>((Value >> 8U) & 0xffU));
Buffer.push_back(static_cast<UInt8>( Value & 0xffU));
break;
case 2:
Buffer.push_back(static_cast<UInt8>((Value >> 8U) & 0xffU));
Buffer.push_back(static_cast<UInt8>( Value & 0xffU));
break;
default:
assert(0 && "invalid integer format length");
return;
}
}
template <typename T>
void
writeVectorInt(const std::vector<T>* VectorData, std::vector<UInt8>& Buffer)
{
if (VectorData) {
const std::vector<T>& 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<UInt8>& 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<UInt8> 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<UInt8*>(buffer);
void
ProtocolUtil::writef(std::vector<UInt8>& 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<UInt8>(v & 0xff);
break;
case 2:
// 2 byte integer
*dst++ = static_cast<UInt8>((v >> 8) & 0xff);
*dst++ = static_cast<UInt8>( v & 0xff);
break;
case 4:
// 4 byte integer
*dst++ = static_cast<UInt8>((v >> 24) & 0xff);
*dst++ = static_cast<UInt8>((v >> 16) & 0xff);
*dst++ = static_cast<UInt8>((v >> 8) & 0xff);
*dst++ = static_cast<UInt8>( 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<UInt8>* list =
va_arg(args, const std::vector<UInt8>*);
const UInt32 n = (UInt32)list->size();
*dst++ = static_cast<UInt8>((n >> 24) & 0xff);
*dst++ = static_cast<UInt8>((n >> 16) & 0xff);
*dst++ = static_cast<UInt8>((n >> 8) & 0xff);
*dst++ = static_cast<UInt8>( n & 0xff);
for (UInt32 i = 0; i < n; ++i) {
*dst++ = (*list)[i];
}
const std::vector<UInt8>* list = va_arg(args, const std::vector<UInt8>*);
writeVectorInt(list, buffer);
break;
}
case 2: {
// 2 byte integers
const std::vector<UInt16>* list =
va_arg(args, const std::vector<UInt16>*);
const UInt32 n = (UInt32)list->size();
*dst++ = static_cast<UInt8>((n >> 24) & 0xff);
*dst++ = static_cast<UInt8>((n >> 16) & 0xff);
*dst++ = static_cast<UInt8>((n >> 8) & 0xff);
*dst++ = static_cast<UInt8>( n & 0xff);
for (UInt32 i = 0; i < n; ++i) {
const UInt16 v = (*list)[i];
*dst++ = static_cast<UInt8>((v >> 8) & 0xff);
*dst++ = static_cast<UInt8>( v & 0xff);
}
const std::vector<UInt16>* list = va_arg(args, const std::vector<UInt16>*);
writeVectorInt(list, buffer);
break;
}
case 4: {
// 4 byte integers
const std::vector<UInt32>* list =
va_arg(args, const std::vector<UInt32>*);
const UInt32 n = (UInt32)list->size();
*dst++ = static_cast<UInt8>((n >> 24) & 0xff);
*dst++ = static_cast<UInt8>((n >> 16) & 0xff);
*dst++ = static_cast<UInt8>((n >> 8) & 0xff);
*dst++ = static_cast<UInt8>( n & 0xff);
for (UInt32 i = 0; i < n; ++i) {
const UInt32 v = (*list)[i];
*dst++ = static_cast<UInt8>((v >> 24) & 0xff);
*dst++ = static_cast<UInt8>((v >> 16) & 0xff);
*dst++ = static_cast<UInt8>((v >> 8) & 0xff);
*dst++ = static_cast<UInt8>( v & 0xff);
}
const std::vector<UInt32>* list = va_arg(args, const std::vector<UInt32>*);
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<UInt8>((len >> 24) & 0xff);
*dst++ = static_cast<UInt8>((len >> 16) & 0xff);
*dst++ = static_cast<UInt8>((len >> 8) & 0xff);
*dst++ = static_cast<UInt8>( 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<UInt8>((len >> 24) & 0xff);
*dst++ = static_cast<UInt8>((len >> 16) & 0xff);
*dst++ = static_cast<UInt8>((len >> 8) & 0xff);
*dst++ = static_cast<UInt8>( 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++);
}
}
}

2
src/lib/synergy/ProtocolUtil.h
View file

@ -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<UInt8>&, const char* fmt, va_list);
static UInt32 eatLength(const char** fmt);
static void read(synergy::IStream*, void*, UInt32);

40
src/test/unittests/synergy/GenericArgsParsingTests.cpp
View file

@ -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<MockApp> 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<MockApp> 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);

197
src/test/unittests/synergy/ProtocolUtilTests.cpp
View file

@ -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<const UInt8*>(arg));
return (expected == Actual8);
}
MATCHER_P(EqVoidPointeeInt16, expected, "")
{
const UInt16 Actual16 = (*static_cast<const UInt16*>(arg));
return (expected == (Actual16 >> 8));
}
MATCHER_P(EqVoidPointeeInt32, expected, "")
{
const UInt32 Actual32 = (*static_cast<const UInt32*>(arg));
return (expected == (Actual32 >> 24));
}
MATCHER_P(EqVoidVectorInt1byte, expected, "")
{
bool Result = true;
const UInt8* Actual = (static_cast<const UInt8*>(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<const UInt16*>(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<const UInt32*>(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<const UInt8*>(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<const char*, int> >
{
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<const char*, int> >
{
public:
MockStream stream;
const std::vector<UInt8> Expected1Byte = {10, 20, 30};
const std::vector<UInt16> Expected2Byte = {40, 50, 60};
const std::vector<UInt32> 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<UInt8> 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<UInt8, Size> 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<UInt8> Expected = {'%', '1', '2', '3', '4', '5'};
EXPECT_CALL(stream, write(EqVectorSymbols(Expected), Expected.size()));
ProtocolUtil::writef(&stream, "%%12345");
}