Merge pull request #20 from IABTechLab/ant-UID2-1322-compile-warning-as-error

Treat compile warnings as errors

Author: atarassov-ttd

.github/workflows/cmake-macosx.yml .github/workflows/cmake-macos.yml.github/workflows/cmake-macosx.yml renamed to .github/workflows/cmake-macos.yml

@@ -1,4 +1,4 @@
-name: CMake Mac
+name: CMake macOS
 
 on: [push]
 
@@ -14,7 +14,7 @@ jobs:
 
     - name: Dev environment
       run: |
-        ${{github.workspace}}/tools/install-macosx-deps.sh
+        ${{github.workspace}}/tools/install-macos-deps.sh
 
     - name: Build and Test
       run: ${{github.workspace}}/tools/build.sh ${{matrix.build_type}}-${{matrix.compiler}}

CMakeLists.txt

@@ -1,12 +1,19 @@
-cmake_minimum_required (VERSION 3.12)
+cmake_minimum_required(VERSION 3.12)
 
-project ("uid2-client" VERSION 3.0.1)
+project("uid2-client" VERSION 3.0.1)
 
 configure_file(VERSION.in VERSION @ONLY)
 
-set (CMAKE_CXX_STANDARD 11)
+option(WARNING_AS_ERROR "Treat compiler warnings as errors" ON)
 
-if (${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
+set(CMAKE_CXX_STANDARD 11)
+set(CMAKE_CXX_FLAGS_DEBUG "-ggdb")
+add_compile_options(-Wall -Wextra)
+if(WARNING_AS_ERROR)
+	add_compile_options(-Werror)
+endif()
+
+if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
 	if (NOT DEFINED OPENSSL_ROOT_DIR)
 		set(OPENSSL_ROOT_DIR /usr/local/opt/openssl)
 	endif()

README.md

@@ -8,6 +8,11 @@ This SDK simplifies integration with UID2 for those using C++.
 
 This SDK requires C++ version 11.
 
+Supported platforms:
+
+ - Linux (tested on Ubuntu 22.04)
+ - MacOS (tested on macOS 12 Monterey and macOS 13 Ventura)
+
 Supported compilers:
 
  - clang (tested on versions 12 and 14)
@@ -28,10 +33,10 @@ sudo ./tools/install-ubuntu-devtools.sh
 sudo ./tools/install-ubuntu-deps.sh
 ```
 
-To set up dependencies on Mac OS X, make sure you have latest xcode installed, then:
+To set up dependencies on macOS, make sure you have latest xcode installed, then:
 
 ```
-./tools/install-macosx-deps.sh
+./tools/install-macos-deps.sh
 ```
 
 If you want to have clang-14 installed on Mac, run these additional commands:
@@ -92,6 +97,12 @@ Most modern IDEs should pick that up automatically. To reformat all the code, ru
 ./tools/devenv.sh ./tools/format.sh --fix
 ```
 
+By default, compiler warnings are treated as errors. To temporarily disable this behavior, set CMake `WARNING_AS_ERROR` option to `OFF`. For example:
+
+```
+cmake -DWARNING_AS_ERROR=OFF
+```
+
 Additionally the codebase is subject to clang-tidy checks. Some IDEs can pick that up automatically. Otherwise
 you can run the checks by running (requires docker):
 

lib/CMakeLists.txt

@@ -19,6 +19,8 @@ add_library(uid2client
 	${EXTERNAL_SOURCES}
 	${CMAKE_CURRENT_BINARY_DIR}/version.h)
 
+set_source_files_properties(${EXTERNAL_SOURCES} PROPERTIES COMPILE_FLAGS -Wno-error)
+
 target_include_directories(uid2client SYSTEM
 	PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/external
 	PRIVATE ${OPENSSL_INCLUDE_DIR})

lib/bigendianprocessor.h

@@ -76,8 +76,8 @@ class BigEndianByteReader {
     }
 
     const std::uint8_t* bytes_;
-    int position_;
     int size_;
+    int position_;
 };
 
 class BigEndianByteWriter {
@@ -140,7 +140,7 @@ class BigEndianByteWriter {
     }
 
     std::uint8_t* bytes_;
-    int position_;
     int size_;
+    int position_;
 };
 }  // namespace uid2

lib/uid2encryption.cpp

@@ -123,7 +123,7 @@ static DecryptionResult DecryptTokenV2(const std::vector<std::uint8_t>& encrypte
     idString.resize(idLength);
     identityPayloadReader.ReadBytes(const_cast<std::uint8_t*>(reinterpret_cast<const std::uint8_t*>(idString.data())), 0, idLength);
 
-    const std::int32_t privacyBits = identityPayloadReader.ReadInt32();  // NOLINT(clang-analyzer-deadcode.DeadStores)
+    /*const std::int32_t privacyBits = */ identityPayloadReader.ReadInt32();
     const Timestamp established = Timestamp::FromEpochMilli(identityPayloadReader.ReadInt64());
 
     if (checkValidity && expires < now) {
@@ -143,7 +143,7 @@ DecryptTokenV3(const std::vector<std::uint8_t>& encryptedId, const KeyContainer&
         return DecryptionResult::MakeError(DecryptionStatus::INVALID_IDENTITY_SCOPE);
     }
 
-    const auto version = reader.ReadByte();  // NOLINT(clang-analyzer-deadcode.DeadStores)
+    /*const auto version = */ reader.ReadByte();
 
     const std::int32_t masterKeyId = reader.ReadInt32();
     const auto* const masterKey = keys.Get(masterKeyId);
@@ -152,20 +152,20 @@ DecryptTokenV3(const std::vector<std::uint8_t>& encryptedId, const KeyContainer&
     }
 
     std::uint8_t masterPayload[256];
-    if (reader.GetRemainingSize() > sizeof(masterPayload)) {
+    if (reader.GetRemainingSize() > static_cast<int>(sizeof(masterPayload))) {
         return DecryptionResult::MakeError(DecryptionStatus::INVALID_PAYLOAD);
     }
     const int masterPayloadLen = DecryptGCM(reader.GetCurrentData(), reader.GetRemainingSize(), masterKey->secret_.data(), masterPayload);
 
     BigEndianByteReader masterPayloadReader(masterPayload, masterPayloadLen);
 
     const Timestamp expires = Timestamp::FromEpochMilli(masterPayloadReader.ReadInt64());
-    const Timestamp created = Timestamp::FromEpochMilli(masterPayloadReader.ReadInt64());  // NOLINT(clang-analyzer-deadcode.DeadStores)
+    /*const Timestamp created = */ Timestamp::FromEpochMilli(masterPayloadReader.ReadInt64());
 
-    const auto operatorSiteId = masterPayloadReader.ReadInt32();       // NOLINT(clang-analyzer-deadcode.DeadStores)
-    const auto operatorType = masterPayloadReader.ReadByte();          // NOLINT(clang-analyzer-deadcode.DeadStores)
-    const auto operatorVersion = masterPayloadReader.ReadInt32();      // NOLINT(clang-analyzer-deadcode.DeadStores)
-    const auto operatorClientKeyId = masterPayloadReader.ReadInt32();  // NOLINT(clang-analyzer-deadcode.DeadStores)
+    /*const auto operatorSiteId = */ masterPayloadReader.ReadInt32();
+    /*const auto operatorType = */ masterPayloadReader.ReadByte();
+    /*const auto operatorVersion = */ masterPayloadReader.ReadInt32();
+    /*const auto operatorClientKeyId = */ masterPayloadReader.ReadInt32();
 
     const auto siteKeyId = masterPayloadReader.ReadInt32();
     const auto* const siteKey = keys.Get(siteKeyId);
@@ -174,20 +174,20 @@ DecryptTokenV3(const std::vector<std::uint8_t>& encryptedId, const KeyContainer&
     }
 
     std::uint8_t sitePayload[128];
-    if (masterPayloadReader.GetRemainingSize() > sizeof(sitePayload)) {
+    if (masterPayloadReader.GetRemainingSize() > static_cast<int>(sizeof(sitePayload))) {
         return DecryptionResult::MakeError(DecryptionStatus::INVALID_PAYLOAD);
     }
     const auto sitePayloadLen = DecryptGCM(masterPayloadReader.GetCurrentData(), masterPayloadReader.GetRemainingSize(), siteKey->secret_.data(), sitePayload);
 
     BigEndianByteReader sitePayloadReader(sitePayload, sitePayloadLen);
 
     const auto siteId = sitePayloadReader.ReadInt32();
-    const auto publisherId = sitePayloadReader.ReadInt64();     // NOLINT(clang-analyzer-deadcode.DeadStores)
-    const auto publisherKeyId = sitePayloadReader.ReadInt32();  // NOLINT(clang-analyzer-deadcode.DeadStores)
+    /*const auto publisherId = */ sitePayloadReader.ReadInt64();
+    /*const auto publisherKeyId = */ sitePayloadReader.ReadInt32();
 
-    const auto privacyBits = sitePayloadReader.ReadInt32();  // NOLINT(clang-analyzer-deadcode.DeadStores)
+    /*const auto privacyBits = */ sitePayloadReader.ReadInt32();
     const Timestamp established = Timestamp::FromEpochMilli(sitePayloadReader.ReadInt64());
-    const Timestamp refreshed = Timestamp::FromEpochMilli(sitePayloadReader.ReadInt64());  // NOLINT(clang-analyzer-deadcode.DeadStores)
+    /*const Timestamp refreshed = */ Timestamp::FromEpochMilli(sitePayloadReader.ReadInt64());
 
     if (checkValidity && expires < now) {
         return DecryptionResult::MakeError(DecryptionStatus::EXPIRED_TOKEN, established, siteId, siteKey->siteId_);
@@ -314,7 +314,7 @@ static DecryptionDataResult DecryptDataV2(const std::vector<std::uint8_t>& encry
     }
 
     const auto encryptedAt = Timestamp::FromEpochMilli(reader.ReadInt64());
-    const int siteId = reader.ReadInt32();  // NOLINT(clang-analyzer-deadcode.DeadStores)
+    /*const int siteId = */ reader.ReadInt32();
     const std::int64_t keyId = reader.ReadInt32();
     const auto* const key = keys.Get(keyId);
     if (key == nullptr) {
@@ -351,7 +351,7 @@ static DecryptionDataResult DecryptDataV3(const std::vector<std::uint8_t>& encry
 
     BigEndianByteReader payloadReader(payload.data(), payloadLen);
     const auto encryptedAt = Timestamp::FromEpochMilli(payloadReader.ReadInt64());
-    const int siteId = payloadReader.ReadInt32();  // NOLINT(clang-analyzer-deadcode.DeadStores)
+    /*const int siteId = */ payloadReader.ReadInt32();
 
     return DecryptionDataResult::MakeSuccess({payloadReader.GetCurrentData(), payloadReader.GetCurrentData() + payloadReader.GetRemainingSize()}, encryptedAt);
 }

test/base_64_encoding_test.cpp

@@ -16,7 +16,7 @@ static void VerifyEncodingDecodingTables(
     std::set<unsigned int> decodedValues;
     EXPECT_EQ(expectedEncodingTable.size(), 64);
     EXPECT_EQ(expectedEncodingTable.size(), encodingTableLen);
-    for (int i = 0; i < expectedEncodingTable.size(); i++) {
+    for (int i = 0; i < static_cast<int>(expectedEncodingTable.size()); i++) {
         EXPECT_EQ(expectedEncodingTable[i], encodingTable[i]);
         const char chr = expectedEncodingTable[i];
         const auto decoderTableIndex = static_cast<unsigned char>(chr);
@@ -27,7 +27,7 @@ static void VerifyEncodingDecodingTables(
 
     // verify all the non-base64/base64url chars must have decoding value of 64 (b1000000)
     EXPECT_EQ(decodingTableLen, 256);
-    for (int i = 0; i < decodingTableLen; i++) {
+    for (unsigned int i = 0; i < decodingTableLen; i++) {
         if (decodedValues.find(i) == decodedValues.end()) {
             std::cout << i << std::endl;
             EXPECT_EQ(decodingTable[i], 64);

test/encryption_tests_v2.cpp

@@ -12,7 +12,6 @@ using namespace uid2;
 
 static std::vector<std::uint8_t> GetMasterSecret();
 static std::vector<std::uint8_t> GetSiteSecret();
-static std::vector<std::uint8_t> MakeKeySecret(std::uint8_t v);
 static std::string KeySetToJson(const std::vector<Key>& keys);
 static std::vector<std::uint8_t> Base64Decode(const std::string& str);
 
@@ -210,11 +209,6 @@ std::vector<std::uint8_t> GetSiteSecret()
     return {SITE_SECRET, SITE_SECRET + sizeof(SITE_SECRET)};
 }
 
-std::vector<std::uint8_t> MakeKeySecret(std::uint8_t v)
-{
-    return std::vector<std::uint8_t>(sizeof(SITE_SECRET), v);  // NOLINT
-}
-
 std::vector<std::uint8_t> Base64Decode(const std::string& str)
 {
     std::vector<std::uint8_t> result;

test/encryption_tests_v4.cpp

@@ -28,11 +28,8 @@ static std::string KeySetToJsonForSharing(const std::vector<Key>& keys);
 
 static std::string KeySetToJsonForSharingWithHeader(const std::string& defaultKeyset, int callerSiteId, const std::vector<Key>& keys);
 
-static std::vector<std::uint8_t> Base64Decode(const std::string& str);
-
 static const int MASTER_KEYSET_ID = 1;
 static const int DEFAULT_KEYSET_ID = 99999;
-static const int TOKEN_EXPIRY_SECONDS = 86400;
 static const std::int64_t MASTER_KEY_ID = 164;
 static const std::int64_t SITE_KEY_ID = 165;
 static const int SITE_ID = 9000;
@@ -84,7 +81,7 @@ void crossPlatformConsistencyCheck_Base64UrlTest(const std::vector<std::uint8_t>
     std::vector<std::uint8_t> decoded;
     uid2::UID2Base64UrlCoder::Decode(base64UrlEncodedStr, decoded);
     EXPECT_EQ(rawInputLen, decoded.size());
-    for (int i = 0; i < decoded.size(); i++) {
+    for (size_t i = 0; i < decoded.size(); i++) {
         EXPECT_EQ(rawInput[i], decoded[i]);
     }
 }
@@ -542,10 +539,3 @@ std::vector<std::uint8_t> MakeKeySecret(std::uint8_t v)
 {
     return std::vector<std::uint8_t>(sizeof(SITE_SECRET), v);  // NOLINT
 }
-
-std::vector<std::uint8_t> Base64Decode(const std::string& str)
-{
-    std::vector<std::uint8_t> result;
-    macaron::Base64::Decode(str, result);
-    return result;
-}