diff --git a/.github/workflows/linting-formatting.yml b/.github/workflows/linting-formatting.yml index b38e4f60c..d9cc12432 100644 --- a/.github/workflows/linting-formatting.yml +++ b/.github/workflows/linting-formatting.yml @@ -32,7 +32,7 @@ jobs: VALIDATE_ALL_CODEBASE: true GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: git diff - - uses: github/codeql-action/upload-sarif@662472033e021d55d94146f66f6058822b0b39fd # v3.27.0 + - uses: github/codeql-action/upload-sarif@4f3212b61783c3c68e8309a0f18a699764811cda # v3.27.1 if: ${{ success() || failure() }} with: sarif_file: megalinter-reports/megalinter-report.sarif diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 850f44119..738d7b6ac 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -31,6 +31,6 @@ jobs: results_format: sarif repo_token: ${{ secrets.SCORECARD_READ_TOKEN }} publish_results: true - - uses: github/codeql-action/upload-sarif@662472033e021d55d94146f66f6058822b0b39fd # v3.27.0 + - uses: github/codeql-action/upload-sarif@4f3212b61783c3c68e8309a0f18a699764811cda # v3.27.1 with: sarif_file: scorecards.sarif diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index 5559a1043..914cea948 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -82,7 +82,7 @@ jobs: - uses: hendrikmuhs/ccache-action@ed74d11c0b343532753ecead8a951bb09bb34bc9 # v1.2.14 with: key: ${{ github.job }} - - uses: github/codeql-action/init@662472033e021d55d94146f66f6058822b0b39fd # v3.27.0 + - uses: github/codeql-action/init@4f3212b61783c3c68e8309a0f18a699764811cda # v3.27.1 with: languages: cpp - uses: lukka/run-cmake@af1be47fd7c933593f687731bc6fdbee024d3ff4 # v10.8 @@ -90,4 +90,4 @@ jobs: configurePreset: "host" buildPreset: "host-Debug" configurePresetAdditionalArgs: "['-DCMAKE_C_COMPILER_LAUNCHER=ccache', '-DCMAKE_CXX_COMPILER_LAUNCHER=ccache']" - - uses: github/codeql-action/analyze@662472033e021d55d94146f66f6058822b0b39fd # v3.27.0 + - uses: github/codeql-action/analyze@4f3212b61783c3c68e8309a0f18a699764811cda # v3.27.1 diff --git a/infra/timer/TickOnInterruptTimerService.cpp b/infra/timer/TickOnInterruptTimerService.cpp index 322d23bfd..085a94ed6 100644 --- a/infra/timer/TickOnInterruptTimerService.cpp +++ b/infra/timer/TickOnInterruptTimerService.cpp @@ -1,5 +1,6 @@ #include "infra/timer/TickOnInterruptTimerService.hpp" #include "infra/event/EventDispatcher.hpp" +#include "infra/util/ReallyAssert.hpp" #include namespace infra @@ -8,6 +9,8 @@ namespace infra : TimerService(id) , resolution(resolution) { + really_assert(infra::EventDispatcher::InstanceSet()); + CalculateNextTrigger(); } diff --git a/services/network/HttpClientAuthentication.cpp b/services/network/HttpClientAuthentication.cpp index f65b95121..065ad846b 100644 --- a/services/network/HttpClientAuthentication.cpp +++ b/services/network/HttpClientAuthentication.cpp @@ -8,89 +8,89 @@ namespace services void HttpClientAuthentication::Get(infra::BoundedConstString requestTarget, HttpHeaders headers) { - Request(headers, [this, requestTarget]() + Request(headers, HttpVerb::get, requestTarget, [this]() { - Subject().Get(requestTarget, infra::MakeRange(headersWithAuthorization)); + Subject().Get(target, infra::MakeRange(headersWithAuthorization)); }); } void HttpClientAuthentication::Head(infra::BoundedConstString requestTarget, HttpHeaders headers) { - Request(headers, [this, requestTarget]() + Request(headers, HttpVerb::head, requestTarget, [this]() { - Subject().Head(requestTarget, infra::MakeRange(headersWithAuthorization)); + Subject().Head(target, infra::MakeRange(headersWithAuthorization)); }); } void HttpClientAuthentication::Connect(infra::BoundedConstString requestTarget, HttpHeaders headers) { - Request(headers, [this, requestTarget]() + Request(headers, HttpVerb::connect, requestTarget, [this]() { - Subject().Connect(requestTarget, infra::MakeRange(headersWithAuthorization)); + Subject().Connect(target, infra::MakeRange(headersWithAuthorization)); }); } void HttpClientAuthentication::Options(infra::BoundedConstString requestTarget, HttpHeaders headers) { - Request(headers, [this, requestTarget]() + Request(headers, HttpVerb::options, requestTarget, [this]() { - Subject().Options(requestTarget, infra::MakeRange(headersWithAuthorization)); + Subject().Options(target, infra::MakeRange(headersWithAuthorization)); }); } void HttpClientAuthentication::Post(infra::BoundedConstString requestTarget, infra::BoundedConstString content, HttpHeaders headers) { - Request(headers, [this, requestTarget, content]() + Request(headers, HttpVerb::post, requestTarget, [this, content]() { - Subject().Post(requestTarget, content, infra::MakeRange(headersWithAuthorization)); + Subject().Post(target, content, infra::MakeRange(headersWithAuthorization)); }); } void HttpClientAuthentication::Post(infra::BoundedConstString requestTarget, HttpHeaders headers) { - Request(headers, [this, requestTarget]() + Request(headers, HttpVerb::post, requestTarget, [this]() { - Subject().Post(requestTarget, infra::MakeRange(headersWithAuthorization)); + Subject().Post(target, infra::MakeRange(headersWithAuthorization)); }); } void HttpClientAuthentication::Put(infra::BoundedConstString requestTarget, infra::BoundedConstString content, HttpHeaders headers) { - Request(headers, [this, requestTarget, content]() + Request(headers, HttpVerb::put, requestTarget, [this, content]() { - Subject().Put(requestTarget, content, infra::MakeRange(headersWithAuthorization)); + Subject().Put(target, content, infra::MakeRange(headersWithAuthorization)); }); } void HttpClientAuthentication::Put(infra::BoundedConstString requestTarget, HttpHeaders headers) { - Request(headers, [this, requestTarget]() + Request(headers, HttpVerb::put, requestTarget, [this]() { - Subject().Put(requestTarget, infra::MakeRange(headersWithAuthorization)); + Subject().Put(target, infra::MakeRange(headersWithAuthorization)); }); } void HttpClientAuthentication::Patch(infra::BoundedConstString requestTarget, infra::BoundedConstString content, HttpHeaders headers) { - Request(headers, [this, requestTarget, content]() + Request(headers, HttpVerb::patch, requestTarget, [this, content]() { - Subject().Patch(requestTarget, content, infra::MakeRange(headersWithAuthorization)); + Subject().Patch(target, content, infra::MakeRange(headersWithAuthorization)); }); } void HttpClientAuthentication::Patch(infra::BoundedConstString requestTarget, HttpHeaders headers) { - Request(headers, [this, requestTarget]() + Request(headers, HttpVerb::patch, requestTarget, [this]() { - Subject().Patch(requestTarget, infra::MakeRange(headersWithAuthorization)); + Subject().Patch(target, infra::MakeRange(headersWithAuthorization)); }); } void HttpClientAuthentication::Delete(infra::BoundedConstString requestTarget, infra::BoundedConstString content, HttpHeaders headers) { - Request(headers, [this, requestTarget, content]() + Request(headers, HttpVerb::delete_, requestTarget, [this, content]() { - Subject().Delete(requestTarget, content, infra::MakeRange(headersWithAuthorization)); + Subject().Delete(target, content, infra::MakeRange(headersWithAuthorization)); }); } @@ -155,7 +155,8 @@ namespace services Observer().StatusAvailable(HttpStatusCode::Unauthorized); } - Observer().BodyComplete(); + if (HttpClient::IsAttached()) + Observer().BodyComplete(); } void HttpClientAuthentication::SendStreamAvailable(infra::SharedPtr&& writer) @@ -170,15 +171,18 @@ namespace services void HttpClientAuthentication::Detaching() { - HttpClient::Detach(); + if (HttpClient::IsAttached()) + HttpClient::Detach(); HttpClientObserver::Detaching(); } - void HttpClientAuthentication::Request(HttpHeaders headers, const infra::Function& newRequest) + void HttpClientAuthentication::Request(HttpHeaders headers, HttpVerb verb, infra::BoundedConstString target, const infra::Function& newRequest) { MakeHeaders(headers); unauthorized = false; request = newRequest; + this->verb = verb; + this->target = target; request(); } @@ -195,7 +199,10 @@ namespace services auto scheme = headerValue.substr(0, spaceIndex); auto challenge = headerValue.substr(std::min(headerValue.find_first_not_of(' ', spaceIndex), headerValue.size())); - Authenticate(scheme, challenge); + Authenticate(verb, target, scheme, challenge); + + headersWithAuthorization.pop_back(); + headersWithAuthorization.emplace_back("Authorization", AuthenticationHeader()); } HttpClientAuthenticationConnector::HttpClientAuthenticationConnector(HttpClientConnector& connector, HttpClientAuthentication& clientAuthentication) diff --git a/services/network/HttpClientAuthentication.hpp b/services/network/HttpClientAuthentication.hpp index 8f7bf186b..97e679782 100644 --- a/services/network/HttpClientAuthentication.hpp +++ b/services/network/HttpClientAuthentication.hpp @@ -39,18 +39,20 @@ namespace services void Detaching() override; protected: - virtual void Authenticate(infra::BoundedConstString scheme, infra::BoundedConstString value) = 0; + virtual void Authenticate(HttpVerb verb, infra::BoundedConstString target, infra::BoundedConstString scheme, infra::BoundedConstString value) = 0; virtual infra::BoundedConstString AuthenticationHeader() const = 0; virtual bool Retry() const = 0; virtual void Reset() = 0; private: - void Request(HttpHeaders headers, const infra::Function& newRequest); + void Request(HttpHeaders headers, HttpVerb verb, infra::BoundedConstString target, const infra::Function& newRequest); void MakeHeaders(HttpHeaders headers); void Authenticate(infra::BoundedConstString headerValue); private: infra::Function request; + HttpVerb verb; + infra::BoundedConstString target; bool unauthorized = false; infra::BoundedVector& headersWithAuthorization; }; diff --git a/services/network/test/TestHttpClientAuthentication.cpp b/services/network/test/TestHttpClientAuthentication.cpp index 2fe1816cf..6128d34e2 100644 --- a/services/network/test/TestHttpClientAuthentication.cpp +++ b/services/network/test/TestHttpClientAuthentication.cpp @@ -17,7 +17,7 @@ namespace using services::HttpClientAuthentication::HttpClientAuthentication; - MOCK_METHOD2(Authenticate, void(infra::BoundedConstString scheme, infra::BoundedConstString value)); + MOCK_METHOD4(Authenticate, void(services::HttpVerb verb, infra::BoundedConstString target, infra::BoundedConstString scheme, infra::BoundedConstString value)); MOCK_CONST_METHOD0(AuthenticationHeader, infra::BoundedConstString()); MOCK_CONST_METHOD0(Retry, bool()); MOCK_METHOD0(Reset, void()); @@ -37,7 +37,8 @@ class HttpClientAuthenticationTest ~HttpClientAuthenticationTest() override { - EXPECT_CALL(*httpClientObserver, Detaching()); + if (httpClientObserver->IsAttached()) + EXPECT_CALL(*httpClientObserver, Detaching()); } void CheckHeaders(services::HttpHeaders headersToCheck, infra::BoundedConstString headerValueToAdd) @@ -47,6 +48,16 @@ class HttpClientAuthenticationTest EXPECT_TRUE(infra::ContentsEqual(infra::MakeRange(copy), headersToCheck)); } + void Get() + { + EXPECT_CALL(httpClient, Get("target", testing::_)).WillOnce(testing::Invoke([this](infra::BoundedConstString requestTarget, services::HttpHeaders headers) + { + CheckHeaders(headers, "header contents"); + })); + EXPECT_CALL(clientAuthentication, AuthenticationHeader()).WillOnce(testing::Return("header contents")); + httpClientObserver->Subject().Get("target", infra::MakeRange(headers)); + } + infra::SharedOptional> httpClientObserver; testing::StrictMock> clientAuthentication; testing::StrictMock httpClient; @@ -56,12 +67,7 @@ class HttpClientAuthenticationTest TEST_F(HttpClientAuthenticationTest, Get_request_is_forwarded) { - EXPECT_CALL(httpClient, Get("target", testing::_)).WillOnce(testing::Invoke([this](infra::BoundedConstString requestTarget, services::HttpHeaders headers) - { - CheckHeaders(headers, "header contents"); - })); - EXPECT_CALL(clientAuthentication, AuthenticationHeader()).WillOnce(testing::Return("header contents")); - httpClientObserver->Subject().Get("target", infra::MakeRange(headers)); + Get(); } TEST_F(HttpClientAuthenticationTest, Head_request_is_forwarded) @@ -134,6 +140,26 @@ TEST_F(HttpClientAuthenticationTest, Put_request_is_forwarded_3) httpClientObserver->Subject().Put("target", infra::MakeRange(headers)); } +TEST_F(HttpClientAuthenticationTest, Patch_request_is_forwarded_1) +{ + EXPECT_CALL(httpClient, Patch("target", "contents", testing::_)).WillOnce(testing::Invoke([this](infra::BoundedConstString requestTarget, infra::BoundedConstString contents, services::HttpHeaders headers) + { + CheckHeaders(headers, "header contents"); + })); + EXPECT_CALL(clientAuthentication, AuthenticationHeader()).WillOnce(testing::Return("header contents")); + httpClientObserver->Subject().Patch("target", "contents", infra::MakeRange(headers)); +} + +TEST_F(HttpClientAuthenticationTest, Patch_request_is_forwarded_3) +{ + EXPECT_CALL(httpClient, Patch("target", testing::_)).WillOnce(testing::Invoke([this](infra::BoundedConstString requestTarget, services::HttpHeaders headers) + { + CheckHeaders(headers, "header contents"); + })); + EXPECT_CALL(clientAuthentication, AuthenticationHeader()).WillOnce(testing::Return("header contents")); + httpClientObserver->Subject().Patch("target", infra::MakeRange(headers)); +} + TEST_F(HttpClientAuthenticationTest, Delete_request_is_forwarded) { EXPECT_CALL(httpClient, Delete("target", "contents", testing::_)).WillOnce(testing::Invoke([this](infra::BoundedConstString requestTarget, infra::BoundedConstString contents, services::HttpHeaders headers) @@ -165,6 +191,8 @@ TEST_F(HttpClientAuthenticationTest, GetConnection_is_forwarded) TEST_F(HttpClientAuthenticationTest, normal_flow_is_forwarded) { + Get(); + testing::StrictMock writer; EXPECT_CALL(*httpClientObserver, FillContent(testing::Ref(writer))); httpClient.Observer().FillContent(writer); @@ -190,10 +218,13 @@ TEST_F(HttpClientAuthenticationTest, normal_flow_is_forwarded) TEST_F(HttpClientAuthenticationTest, unauthorized_is_not_retried) { + Get(); + httpClient.Observer().StatusAvailable(services::HttpStatusCode::Unauthorized); httpClient.Observer().HeaderAvailable({ "name", "value" }); - EXPECT_CALL(clientAuthentication, Authenticate("scheme", "value")); + EXPECT_CALL(clientAuthentication, AuthenticationHeader()).WillOnce(testing::Return("header contents")); + EXPECT_CALL(clientAuthentication, Authenticate(services::HttpVerb::get, "target", "scheme", "value")); httpClient.Observer().HeaderAvailable({ "WWW-Authenticate", "scheme value" }); testing::StrictMock reader; @@ -215,7 +246,8 @@ TEST_F(HttpClientAuthenticationTest, unauthorized_is_retried) httpClient.Observer().StatusAvailable(services::HttpStatusCode::Unauthorized); httpClient.Observer().HeaderAvailable({ "name", "value" }); - EXPECT_CALL(clientAuthentication, Authenticate("scheme", "value")); + EXPECT_CALL(clientAuthentication, AuthenticationHeader()).WillOnce(testing::Return("header contents")); + EXPECT_CALL(clientAuthentication, Authenticate(services::HttpVerb::get, "target", "scheme", "value")); httpClient.Observer().HeaderAvailable({ "WWW-Authenticate", "scheme value" }); testing::StrictMock reader; @@ -230,3 +262,27 @@ TEST_F(HttpClientAuthenticationTest, unauthorized_is_retried) })); httpClient.Observer().BodyComplete(); } + +TEST_F(HttpClientAuthenticationTest, detach_during_StatusAvailable) +{ + Get(); + + httpClient.Observer().StatusAvailable(services::HttpStatusCode::Unauthorized); + + httpClient.Observer().HeaderAvailable({ "name", "value" }); + EXPECT_CALL(clientAuthentication, AuthenticationHeader()).WillOnce(testing::Return("header contents")); + EXPECT_CALL(clientAuthentication, Authenticate(services::HttpVerb::get, "target", "scheme", "value")); + httpClient.Observer().HeaderAvailable({ "WWW-Authenticate", "scheme value" }); + + testing::StrictMock reader; + EXPECT_CALL(reader, Empty()).WillOnce(testing::Return(true)); + httpClient.Observer().BodyAvailable(infra::UnOwnedSharedPtr(reader)); + + EXPECT_CALL(clientAuthentication, Retry()).WillOnce(testing::Return(false)); + EXPECT_CALL(*httpClientObserver, StatusAvailable(services::HttpStatusCode::Unauthorized)).WillOnce(testing::Invoke([this](services::HttpStatusCode) + { + EXPECT_CALL(*httpClientObserver, Detaching()); + httpClientObserver->Detach(); + })); + httpClient.Observer().BodyComplete(); +} diff --git a/services/network/test_doubles/HttpClientAuthenticationStub.hpp b/services/network/test_doubles/HttpClientAuthenticationStub.hpp index f8bcf3f33..97aef1397 100644 --- a/services/network/test_doubles/HttpClientAuthenticationStub.hpp +++ b/services/network/test_doubles/HttpClientAuthenticationStub.hpp @@ -15,7 +15,7 @@ class HttpClientAuthenticationStub protected: // Implementation of HttpClientAuthentication - void Authenticate(infra::BoundedConstString scheme, infra::BoundedConstString value) override + void Authenticate(services::HttpVerb verb, infra::BoundedConstString target, infra::BoundedConstString scheme, infra::BoundedConstString value) override {} infra::BoundedConstString AuthenticationHeader() const override