From fc4ac803aad4fd0341e0d5bb5d11ffc75aef869c Mon Sep 17 00:00:00 2001 From: yuyoyuppe Date: Wed, 26 Aug 2020 15:03:53 +0300 Subject: [PATCH] common: do not accept invalid input in VersionHelper and add negative unit tests --- .../UnitTestsVersionHelper.cpp | 93 ++++++++++++++++--- src/common/VersionHelper.cpp | 34 +++---- src/common/VersionHelper.h | 8 +- src/common/common.vcxproj | 1 + src/common/common.vcxproj.filters | 7 +- src/common/d2d_svg.cpp | 4 +- src/common/pch.h | 1 + src/common/string_utils.h | 32 +++++++ 8 files changed, 144 insertions(+), 36 deletions(-) create mode 100644 src/common/string_utils.h diff --git a/src/common/UnitTests-CommonLib/UnitTestsVersionHelper.cpp b/src/common/UnitTests-CommonLib/UnitTestsVersionHelper.cpp index 0c4c0ad9dd..79424a4827 100644 --- a/src/common/UnitTests-CommonLib/UnitTestsVersionHelper.cpp +++ b/src/common/UnitTests-CommonLib/UnitTestsVersionHelper.cpp @@ -4,11 +4,20 @@ using namespace Microsoft::VisualStudio::CppUnitTestFramework; +namespace Microsoft::VisualStudio::CppUnitTestFramework +{ + template<> + inline std::wstring ToString(const VersionHelper& v) + { + return v.toWstring(); + } +} + namespace UnitTestsVersionHelper { - const int MAJOR_VERSION_0 = 0; - const int MINOR_VERSION_12 = 12; - const int REVISION_VERSION_0 = 0; + const size_t MAJOR_VERSION_0 = 0; + const size_t MINOR_VERSION_12 = 12; + const size_t REVISION_VERSION_0 = 0; TEST_CLASS (UnitTestsVersionHelper) { @@ -23,9 +32,9 @@ namespace UnitTestsVersionHelper } TEST_METHOD (integerConstructorShouldProperlyInitializationWithDifferentVersionNumbers) { - const int testcaseMajor = 2; - const int testcaseMinor = 25; - const int testcaseRevision = 1; + const size_t testcaseMajor = 2; + const size_t testcaseMinor = 25; + const size_t testcaseRevision = 1; VersionHelper sut(testcaseMajor, testcaseMinor, testcaseRevision); Assert::AreEqual(testcaseMajor, sut.major); @@ -36,17 +45,77 @@ namespace UnitTestsVersionHelper { VersionHelper sut("v0.12.3"); - Assert::AreEqual(0, sut.major); - Assert::AreEqual(12, sut.minor); - Assert::AreEqual(3, sut.revision); + Assert::AreEqual(0ull, sut.major); + Assert::AreEqual(12ull, sut.minor); + Assert::AreEqual(3ull, sut.revision); } TEST_METHOD (stringConstructorShouldProperlyInitializationWithDifferentVersionNumbers) { VersionHelper sut("v2.25.1"); - Assert::AreEqual(2, sut.major); - Assert::AreEqual(25, sut.minor); - Assert::AreEqual(1, sut.revision); + Assert::AreEqual(2ull, sut.major); + Assert::AreEqual(25ull, sut.minor); + Assert::AreEqual(1ull, sut.revision); + } + TEST_METHOD (emptyStringNotAccepted) + { + Assert::ExpectException([] { + VersionHelper sut(""); + }); + } + TEST_METHOD (invalidStringNotAccepted1) + { + Assert::ExpectException([] { + VersionHelper sut("v2a."); + }); + } + TEST_METHOD (invalidStringNotAccepted2) + { + Assert::ExpectException([] { + VersionHelper sut("12abc2vv.0"); + }); + } + TEST_METHOD (invalidStringNotAccepted3) + { + Assert::ExpectException([] { + VersionHelper sut("123"); + }); + } + TEST_METHOD (invalidStringNotAccepted4) + { + Assert::ExpectException([] { + VersionHelper sut("v1v2v3"); + }); + } + TEST_METHOD (invalidStringNotAccepted5) + { + Assert::ExpectException([] { + VersionHelper sut("v.1.2.3v"); + }); + } + TEST_METHOD (partialVersionStringNotAccepted1) + { + Assert::ExpectException([] { + VersionHelper sut("v1."); + }); + } + TEST_METHOD (partialVersionStringNotAccepted2) + { + Assert::ExpectException([] { + VersionHelper sut("v1.2"); + }); + } + TEST_METHOD (partialVersionStringNotAccepted3) + { + Assert::ExpectException([] { + VersionHelper sut("v1.2."); + }); + } + TEST_METHOD (parsedWithoutLeadingV) + { + VersionHelper expected{ 12ull, 13ull, 111ull }; + VersionHelper actual("12.13.111"); + Assert::AreEqual(actual, expected); } TEST_METHOD (whenMajorVersionIsGreaterComparisonOperatorShouldReturnProperValue) { diff --git a/src/common/VersionHelper.cpp b/src/common/VersionHelper.cpp index fefdb82432..9317a7c849 100644 --- a/src/common/VersionHelper.cpp +++ b/src/common/VersionHelper.cpp @@ -1,30 +1,32 @@ #include "pch.h" #include "VersionHelper.h" +#include "string_utils.h" + #include #include VersionHelper::VersionHelper(std::string str) { - std::replace(str.begin(), str.end(), '.', ' '); - std::replace(str.begin(), str.end(), 'v', ' '); - std::stringstream ss; + // Remove whitespaces chars and a leading 'v' + str = left_trim(trim(str), "v"); + // Replace '.' with spaces + replace_chars(str, ".", ' '); - ss << str; - - std::string temp; - ss >> temp; - std::stringstream(temp) >> major; - ss >> temp; - std::stringstream(temp) >> minor; - ss >> temp; - std::stringstream(temp) >> revision; + std::istringstream ss{ str }; + ss >> major; + ss >> minor; + ss >> revision; + if (ss.fail() || !ss.eof()) + { + throw std::logic_error("VersionHelper: couldn't parse the supplied version string"); + } } -VersionHelper::VersionHelper(int major, int minor, int revision) : - major(major), - minor(minor), - revision(revision) +VersionHelper::VersionHelper(const size_t major, const size_t minor, const size_t revision) : + major{ major }, + minor{ minor }, + revision{ revision } { } diff --git a/src/common/VersionHelper.h b/src/common/VersionHelper.h index 6f1f22e59c..9d3a24c571 100644 --- a/src/common/VersionHelper.h +++ b/src/common/VersionHelper.h @@ -6,13 +6,13 @@ struct VersionHelper { VersionHelper(std::string str); - VersionHelper(int major, int minor, int revision); + VersionHelper(const size_t major, const size_t minor, const size_t revision); auto operator<=>(const VersionHelper&) const = default; - int major; - int minor; - int revision; + size_t major; + size_t minor; + size_t revision; std::wstring toWstring() const; }; diff --git a/src/common/common.vcxproj b/src/common/common.vcxproj index 6b1e73e94f..97a46b404e 100644 --- a/src/common/common.vcxproj +++ b/src/common/common.vcxproj @@ -138,6 +138,7 @@ + diff --git a/src/common/common.vcxproj.filters b/src/common/common.vcxproj.filters index 8b47962b66..c9abbe7cb4 100644 --- a/src/common/common.vcxproj.filters +++ b/src/common/common.vcxproj.filters @@ -131,13 +131,16 @@ Header Files - + Header Files Header Files + + Header Files + @@ -223,4 +226,4 @@ - + \ No newline at end of file diff --git a/src/common/d2d_svg.cpp b/src/common/d2d_svg.cpp index 3e8a9b92ee..a839eef9ba 100644 --- a/src/common/d2d_svg.cpp +++ b/src/common/d2d_svg.cpp @@ -34,10 +34,10 @@ D2DSVG& D2DSVG::resize(int x, int y, int width, int height, float fill, float ma transform = transform * D2D1::Matrix3x2F::Translation((width - svg_width) / 2.0f, (height - svg_height) / 2.0f); float h_scale = fill * height / svg_height; float v_scale = fill * width / svg_width; - used_scale = min(h_scale, v_scale); + used_scale = std::min(h_scale, v_scale); if (max_scale > 0) { - used_scale = min(used_scale, max_scale); + used_scale = std::min(used_scale, max_scale); } transform = transform * D2D1::Matrix3x2F::Scale(used_scale, used_scale, D2D1::Point2F(width / 2.0f, height / 2.0f)); transform = transform * D2D1::Matrix3x2F::Translation((float)x, (float)y); diff --git a/src/common/pch.h b/src/common/pch.h index 8b9652ae35..c396885a03 100644 --- a/src/common/pch.h +++ b/src/common/pch.h @@ -1,3 +1,4 @@ +#define NOMINMAX #include #include #include diff --git a/src/common/string_utils.h b/src/common/string_utils.h new file mode 100644 index 0000000000..7a0c121b94 --- /dev/null +++ b/src/common/string_utils.h @@ -0,0 +1,32 @@ +#pragma once + +#include +#include +#include + +constexpr inline std::string_view default_trim_arg = " \t\r\n"; + +inline std::string_view left_trim(std::string_view s, const std::string_view chars_to_trim = default_trim_arg) +{ + s.remove_prefix(std::min(s.find_first_not_of(chars_to_trim), size(s))); + return s; +} + +inline std::string_view right_trim(std::string_view s, const std::string_view chars_to_trim = default_trim_arg) +{ + s.remove_suffix(std::min(size(s) - s.find_last_not_of(chars_to_trim) - 1, size(s))); + return s; +} + +inline std::string_view trim(std::string_view s, const std::string_view chars_to_trim = default_trim_arg) +{ + return left_trim(right_trim(s, chars_to_trim), chars_to_trim); +} + +inline void replace_chars(std::string& s, const std::string_view chars_to_replace, const char replacement_char) +{ + for (const char c : chars_to_replace) + { + std::replace(begin(s), end(s), c, replacement_char); + } +}