🧰 Adding clang-tidy, cmake-format, and ASAN tests (#168)

* adding clang-tidy and asan tests

Playing with clang-tidy, we already had the rule removed in #166... running the static analyzer from CMake reproduced the report in many places... I suspect the macro and `new`/`delete` separation clang-tidy is just unable to understand.

Running the problematic examples + tests with the ASAN enabled reported no problems

* Apply suggestions from code review

* Update lint.yml

* applying linting rules

* fix bad rename

* formating

* clang-tidy on jsoncons tests

* and cmake-format to lint

* cmake format improve change detection

* Update lint.yml

* apply linting

.clang-tidy

@@ -19,7 +19,8 @@ Checks: '-*,
          portability-*,
          readability-*,
          -readability-magic-numbers,
-         -readability-braces-around-statements'
+         -readability-braces-around-statements,
+         -readability-uppercase-literal-suffix'
 
 CheckOptions:
   - key: readability-identifier-naming.TypedefCase

.github/actions/install/jsoncons/action.yml

@@ -11,7 +11,7 @@ runs:
     - run: |
         cd /tmp
         wget https://github.com/danielaparker/jsoncons/archive/v${{ inputs.version }}.tar.gz
-        tar -zvxf /tmp/v${{ inputs.version }}.tar.gz
+        tar -zxf /tmp/v${{ inputs.version }}.tar.gz
         cd jsoncons-${{ inputs.version }}
         cmake .
         sudo cmake --install .

.github/workflows/lint.yml

@@ -2,22 +2,81 @@ name: Lint CI
 
 on:
   push:
-    branches: [ master ]
+    branches: [master]
   pull_request:
-    branches: [ master ]
-    paths:
-      - 'include/jwt-cpp/**.h'
-      - 'tests/**.cpp'
-      - '.github/workflows/lint.yml'
+    branches: [master]
 
 jobs:
-  cpp:
+  clang-format:
     runs-on: ubuntu-20.04
     strategy:
       matrix:
         files: ["include/jwt-cpp/*.h", "tests/**.cpp"]
     steps:
-      - run: sudo apt-get install clang-format-10
+      - run: sudo apt-get install clang-format
       - uses: actions/checkout@v2
-      - run: clang-format-10 -i ${{ matrix.files }}
+      - run: clang-format -i ${{ matrix.files }}
       - run: exit $(git status -s | wc -l)
+
+  cmake-format:
+    runs-on: ubuntu-20.04
+    strategy:
+      matrix:
+        files: ["CMakeLists.txt", "**/CMakeLists.txt", "cmake/code-coverage.cmake"]
+    steps:
+      - uses: actions/setup-python@v2
+        with:
+          python-version: '3.x'
+      - run: pip install cmakelang
+      - uses: actions/checkout@v2
+      - run: ls ${{ matrix.files }}
+      - run: cmake-format --check $(ls ${{ matrix.files }})
+
+  clang-tidy:
+    runs-on: ubuntu-20.04
+    steps:
+      - run: sudo apt-get install clang-tidy
+      - uses: actions/checkout@v2
+      - uses: lukka/get-cmake@latest
+      - uses: ./.github/actions/install/gtest
+
+      - name: configure
+        run: |
+          mkdir build
+          cd build
+          cmake .. -DCMAKE_CXX_CLANG_TIDY="clang-tidy;-fix"
+
+      - name: run
+        run: |
+          cd build
+          make
+      - run: exit $(git status -s | wc -l)
+
+  asan: ## Based on https://gist.github.com/jlblancoc/44be9d4d466f0a973b1f3808a8e56782
+    runs-on: ubuntu-20.04
+    steps:
+      - uses: actions/checkout@v2
+      - uses: lukka/get-cmake@latest
+      - uses: ./.github/actions/install/gtest
+
+      - name: configure
+        run: |
+          mkdir build
+          cd build
+          cmake .. -DJWT_BUILD_TESTS=ON -DCMAKE_CXX_FLAGS="-fsanitize=address  -fsanitize=leak -g" \
+                                        -DCMAKE_C_FLAGS="-fsanitize=address  -fsanitize=leak -g" \
+                                        -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address  -fsanitize=leak" \
+                                        -DCMAKE_MODULE_LINKER_FLAGS="-fsanitize=address  -fsanitize=leak"
+
+      - name: build
+        run: |
+          cd build
+          make
+
+      - name: run
+        run: |
+          export ASAN_OPTIONS=fast_unwind_on_malloc=0
+          cd build
+          ./example/rsa-create
+          ./example/rsa-verify
+          ./tests/jwt-cpp-test

CMakeLists.txt

@@ -1,15 +1,12 @@
 cmake_minimum_required(VERSION 3.8)
 
-# HUNTER_ENABLED is always set if this package is included in a project using hunter (HunterGate sets it)
-# In this case we will use hunter as well to stay consistent. If not the use can supply it on configure
-# to force using hunter.
+# HUNTER_ENABLED is always set if this package is included in a project using hunter (HunterGate sets it) In this case
+# we will use hunter as well to stay consistent. If not the use can supply it on configure to force using hunter.
 if(HUNTER_ENABLED)
-	include("cmake/HunterGate.cmake")
-	HunterGate(
-		URL "https://github.com/cpp-pm/hunter/archive/v0.23.314.tar.gz"
-		SHA1 "95c47c92f68edb091b5d6d18924baabe02a6962a"
-	)
-	message(STATUS "jwt-cpp: using hunter for dependency resolution")
+  include("cmake/HunterGate.cmake")
+  huntergate(URL "https://github.com/cpp-pm/hunter/archive/v0.23.314.tar.gz" SHA1
+             "95c47c92f68edb091b5d6d18924baabe02a6962a")
+  message(STATUS "jwt-cpp: using hunter for dependency resolution")
 endif()
 
 project(jwt-cpp)
@@ -32,23 +29,22 @@ if(NOT JWT_SSL_LIBRARY IN_LIST JWT_SSL_LIBRARY_OPTIONS)
   message(FATAL_ERROR "JWT_SSL_LIBRARY must be one of ${JWT_SSL_LIBRARY_OPTIONS}")
 endif()
 
-# If Hunter is enabled, we configure it to resolve OpenSSL and warn
-# the user if he selected an option not supported by hunter.
-# We fall back to the system library in this case.
+# If Hunter is enabled, we configure it to resolve OpenSSL and warn the user if he selected an option not supported by
+# hunter. We fall back to the system library in this case.
 if(HUNTER_ENABLED)
-    if(${JWT_SSL_LIBRARY} MATCHES "OpenSSL")
-        hunter_add_package(OpenSSL)
-    elseif(${JWT_SSL_LIBRARY} MATCHES "LibreSSL")
-        message(WARNING "Hunter does not support LibreSSL yet, the system library will be used (if available)")
-    endif()
-    if(JWT_EXTERNAL_PICOJSON)
-        message(WARNING "Hunter does not support picojson yet, the system library will be used (if available)")
-    endif()
+  if(${JWT_SSL_LIBRARY} MATCHES "OpenSSL")
+    hunter_add_package(OpenSSL)
+  elseif(${JWT_SSL_LIBRARY} MATCHES "LibreSSL")
+    message(WARNING "Hunter does not support LibreSSL yet, the system library will be used (if available)")
+  endif()
+  if(JWT_EXTERNAL_PICOJSON)
+    message(WARNING "Hunter does not support picojson yet, the system library will be used (if available)")
+  endif()
 endif()
 if(${JWT_SSL_LIBRARY} MATCHES "OpenSSL")
-    find_package(OpenSSL 1.0.2 REQUIRED)
+  find_package(OpenSSL 1.0.2 REQUIRED)
 elseif(${JWT_SSL_LIBRARY} MATCHES "LibreSSL")
-    find_package(LibreSSL 3.0.0 REQUIRED)
+  find_package(LibreSSL 3.0.0 REQUIRED)
 endif()
 
 if(JWT_EXTERNAL_PICOJSON)

example/es256k.cpp

@@ -1,7 +1,7 @@
 #include <iostream>
 #include <jwt-cpp/jwt.h>
 
-int main(int argc, const char** argv) {
+int main() {
 	// openssl ecparam -name secp256k1 -genkey -noout -out ec-secp256k1-priv-key.pem
 	std::string es256k_priv_key = R"(-----BEGIN EC PRIVATE KEY-----
 MHQCAQEEIArnQWnspKtjiVuZuzuZ/l1Uqqq8gb2unLJ/6U/Saf4ioAcGBSuBBAAK

example/jwks-verify.cpp

@@ -1,9 +1,8 @@
 #include <iostream>
 #include <jwt-cpp/jwt.h>
 
-int main()
-{
-	std::string publicKey = R"({"keys": [{
+int main() {
+	std::string raw_jwks = R"({"keys": [{
 		"kid":"internal-gateway-jwt.api.sc.net",
 		"alg": "RS256",
     "kty": "RSA",
@@ -27,22 +26,31 @@ int main()
 	}
 ]})";
 
-	std::string token = "eyJraWQiOiJpbnRlcm5hbC1nYXRld2F5LWp3dC5hcGkuc2MubmV0IiwiYWxnIjoiUlMyNTYiLCJ0eXAiOiJKV1QifQ.eyJuYmYiOjE1Mzk3NjcwMTUsImlhdCI6MTUzOTc2Njk5MiwiaXNzIjoia29uZyIsImh0dHA6XC9cL3dzbzIub3JnXC9nYXRld2F5XC9zdWJzY3JpYmVyIjoidXZ0dXNlcjJAY2FyYm9uLnN1cGVyIiwib3JpZ2luYWxfaXNzIjoiaHR0cDpcL1wvd3NvMi5vcmdcL2dhdGV3YXkiLCJzdWIiOiJ1dnR1c2VyMkBjYXJib24uc3VwZXIiLCJodHRwOlwvXC93c28yLm9yZ1wvZ2F0ZXdheVwvZW5kdXNlciI6InV2dHVzZXIyQGNhcmJvbi5zdXBlciIsImp0aSI6IjI0NmJkZTlhLWQ4OGQtNGRlZC1hODhmLTRhMTNhOWJmODQ4ZiIsImh0dHA6XC9cL3dzbzIub3JnXC9nYXRld2F5XC9hcHBsaWNhdGlvbm5hbWUiOiJ1dnR1c2VyMl9hcHBfMSIsImV4cCI6MTUzOTc2NzkxNX0.foxbo6C30yr_wkF-5EkgtYUMG-4SXNfRsmewdT6MbE-RXVkIPkVk8kDP41yRXmnk4OxburCqawiGlzzEhfHoFf0qv0qZEmwEXSdcyRw-czZTs6ACjWYe8kejOCVmpvUrq01NgOhTwgVg6pv93BlcmNY--zytjx_9hlVm5SS1lZ0I21n45BIWu5JvBD51TZXEURb_XhL7RcF9I8mfzrRpB2fSHW38gj-nogsdOPA_y3S-hJKylmmaqmaQgTF-jP-gYr6eqKyGPVwc6fLZ5zqAup59SefdPEY23-WWmHzj968jlsDSEiCp_YiYTnF3tHVLFWDsrprYKwNb0_p95tBmPA";
+	std::string token =
+		"eyJraWQiOiJpbnRlcm5hbC1nYXRld2F5LWp3dC5hcGkuc2MubmV0IiwiYWxnIjoiUlMyNTYiLCJ0eXAiOiJKV1QifQ."
+		"eyJuYmYiOjE1Mzk3NjcwMTUsImlhdCI6MTUzOTc2Njk5MiwiaXNzIjoia29uZyIsImh0dHA6XC9cL3dzbzIub3JnXC9nYXRld2F5XC9zdWJzY3"
+		"JpYmVyIjoidXZ0dXNlcjJAY2FyYm9uLnN1cGVyIiwib3JpZ2luYWxfaXNzIjoiaHR0cDpcL1wvd3NvMi5vcmdcL2dhdGV3YXkiLCJzdWIiOiJ1"
+		"dnR1c2VyMkBjYXJib24uc3VwZXIiLCJodHRwOlwvXC93c28yLm9yZ1wvZ2F0ZXdheVwvZW5kdXNlciI6InV2dHVzZXIyQGNhcmJvbi5zdXBlci"
+		"IsImp0aSI6IjI0NmJkZTlhLWQ4OGQtNGRlZC1hODhmLTRhMTNhOWJmODQ4ZiIsImh0dHA6XC9cL3dzbzIub3JnXC9nYXRld2F5XC9hcHBsaWNh"
+		"dGlvbm5hbWUiOiJ1dnR1c2VyMl9hcHBfMSIsImV4cCI6MTUzOTc2NzkxNX0.foxbo6C30yr_wkF-5EkgtYUMG-4SXNfRsmewdT6MbE-"
+		"RXVkIPkVk8kDP41yRXmnk4OxburCqawiGlzzEhfHoFf0qv0qZEmwEXSdcyRw-czZTs6ACjWYe8kejOCVmpvUrq01NgOhTwgVg6pv93BlcmNY--"
+		"zytjx_9hlVm5SS1lZ0I21n45BIWu5JvBD51TZXEURb_XhL7RcF9I8mfzrRpB2fSHW38gj-nogsdOPA_y3S-hJKylmmaqmaQgTF-jP-"
+		"gYr6eqKyGPVwc6fLZ5zqAup59SefdPEY23-WWmHzj968jlsDSEiCp_YiYTnF3tHVLFWDsrprYKwNb0_p95tBmPA";
 
 	auto decoded_jwt = jwt::decode(token);
-	auto jwks = jwt::parse_jwks(publicKey);
+	auto jwks = jwt::parse_jwks(raw_jwks);
 	auto jwk = jwks.get_jwk(decoded_jwt.get_key_id());
 
 	auto issuer = decoded_jwt.get_issuer();
 	auto x5c = jwk.get_x5c_key_value();
 
 	if (!x5c.empty() && !issuer.empty()) {
-		auto verifier = jwt::verify()
-			.allow_algorithm(jwt::algorithm::rs256(jwt::helper::convert_base64_der_to_pem(x5c), "", "", ""))
-			.with_issuer(issuer)
-			.leeway(60UL); // value in seconds, add some to compensate timeout
+		auto verifier =
+			jwt::verify()
+				.allow_algorithm(jwt::algorithm::rs256(jwt::helper::convert_base64_der_to_pem(x5c), "", "", ""))
+				.with_issuer(issuer)
+				.leeway(60UL); // value in seconds, add some to compensate timeout
 
 		verifier.verify(decoded_jwt);
 	}
-
 }

example/print-claims.cpp

@@ -1,7 +1,7 @@
 #include <iostream>
 #include <jwt-cpp/jwt.h>
 
-int main(int argc, const char** argv) {
+int main() {
 	std::string token =
 		"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXUyJ9.eyJpc3MiOiJhdXRoMCJ9.AbIJTDMFc7yUa5MhvcP03nJPyCPzZtQcGEp-zWfOkEE";
 	auto decoded = jwt::decode(token);

example/private-claims.cpp

@@ -14,7 +14,7 @@ std::string make_pico_token() {
 
 	jwt::claim::set_t list{"once", "twice"};
 
-	std::vector<int64_t> big_numbers{727663072ull, 770979831ull, 427239169ull, 525936436ull};
+	std::vector<int64_t> big_numbers{727663072ULL, 770979831ULL, 427239169ULL, 525936436ULL};
 
 	const auto time = jwt::date::clock::now();
 
@@ -48,22 +48,15 @@ std::string make_nlohmann_token() {
 		static jwt::json::type get_type(const json& val) {
 			using jwt::json::type;
 
-			if (val.type() == json::value_t::boolean)
-				return type::boolean;
-			else if (val.type() == json::value_t::number_integer)
-				return type::integer;
-			else if (val.type() == json::value_t::number_unsigned) // nlohmann internally tracks two types of integers
-				return type::integer;
-			else if (val.type() == json::value_t::number_float)
-				return type::number;
-			else if (val.type() == json::value_t::string)
-				return type::string;
-			else if (val.type() == json::value_t::array)
-				return type::array;
-			else if (val.type() == json::value_t::object)
-				return type::object;
-			else
-				throw std::logic_error("invalid type");
+			if (val.type() == json::value_t::boolean) return type::boolean;
+			if (val.type() == json::value_t::number_integer) return type::integer;
+			// nlohmann internally tracks two types of integers
+			if (val.type() == json::value_t::number_unsigned) return type::integer;
+			if (val.type() == json::value_t::number_float) return type::number;
+			if (val.type() == json::value_t::string) return type::string;
+			if (val.type() == json::value_t::array) return type::array;
+			if (val.type() == json::value_t::object) return type::object;
+			throw std::logic_error("invalid type");
 		}
 
 		static json::object_t as_object(const json& val) {
@@ -115,7 +108,7 @@ std::string make_nlohmann_token() {
 
 	claim::set_t list{"once", "twice"};
 
-	std::vector<int64_t> big_numbers{727663072ull, 770979831ull, 427239169ull, 525936436ull};
+	std::vector<int64_t> big_numbers{727663072ULL, 770979831ULL, 427239169ULL, 525936436ULL};
 
 	const auto time = jwt::date::clock::now();
 
@@ -135,7 +128,7 @@ std::string make_nlohmann_token() {
 		.sign(jwt::algorithm::none{});
 }
 
-int main(int argc, const char** argv) {
+int main() {
 	const auto token = make_pico_token();
 	auto decoded = jwt::decode(token);
 

example/rsa-create.cpp

@@ -1,7 +1,7 @@
 #include <iostream>
 #include <jwt-cpp/jwt.h>
 
-int main(int argc, const char** argv) {
+int main() {
 	std::string rsa_priv_key = R"(-----BEGIN PRIVATE KEY-----
 MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQC4ZtdaIrd1BPIJ
 tfnF0TjIK5inQAXZ3XlCrUlJdP+XHwIRxdv1FsN12XyMYO/6ymLmo9ryoQeIrsXB

example/rsa-verify.cpp

@@ -1,7 +1,7 @@
 #include <iostream>
 #include <jwt-cpp/jwt.h>
 
-int main(int argc, const char** argv) {
+int main() {
 	std::string rsa_priv_key = R"(-----BEGIN PRIVATE KEY-----
 MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQC4ZtdaIrd1BPIJ
 tfnF0TjIK5inQAXZ3XlCrUlJdP+XHwIRxdv1FsN12XyMYO/6ymLmo9ryoQeIrsXB

tests/CMakeLists.txt

@@ -9,7 +9,7 @@ endif()
 enable_testing()
 include(GoogleTest)
 if(HUNTER_ENABLED)
-hunter_add_package(GTest)
+  hunter_add_package(GTest)
 endif()
 find_package(GTest REQUIRED)
 
@@ -30,11 +30,11 @@ target_compile_options(
   jwt-cpp-test PRIVATE $<$<CXX_COMPILER_ID:MSVC>:/W4> $<$<CXX_COMPILER_ID:GNU>:${JWT_TESTER_GCC_FLAGS}>
                        $<$<CXX_COMPILER_ID:Clang>:${JWT_TESTER_CLANG_FLAGS}>)
 if(HUNTER_ENABLED)
-target_link_libraries(jwt-cpp-test PRIVATE GTest::gtest GTest::gtest_main)
-# Define a compile define to bypass openssl error tests
-target_compile_options(jwt-cpp-test PRIVATE -DHUNTER_ENABLED=1)
+  target_link_libraries(jwt-cpp-test PRIVATE GTest::gtest GTest::gtest_main)
+  # Define a compile define to bypass openssl error tests
+  target_compile_options(jwt-cpp-test PRIVATE -DHUNTER_ENABLED=1)
 else()
-target_link_libraries(jwt-cpp-test PRIVATE GTest::GTest GTest::Main)
+  target_link_libraries(jwt-cpp-test PRIVATE GTest::GTest GTest::Main)
 endif()
 target_link_libraries(jwt-cpp-test PRIVATE jwt-cpp $<$<NOT:$<CXX_COMPILER_ID:MSVC>>:${CMAKE_DL_LIBS}>)
 

tests/ClaimTest.cpp

@@ -89,12 +89,12 @@ TEST(ClaimTest, SetAlgorithm) {
 }
 
 TEST(ClaimTest, AsInt) {
-	jwt::claim c(picojson::value((int64_t)10));
+	jwt::claim c(picojson::value(static_cast<int64_t>(10)));
 	ASSERT_EQ(c.as_int(), 10);
 }
 
 TEST(ClaimTest, AsDate) {
-	jwt::claim c(picojson::value((int64_t)10));
+	jwt::claim c(picojson::value(static_cast<int64_t>(10)));
 	ASSERT_EQ(c.as_date(), std::chrono::system_clock::from_time_t(10));
 }