From 626bcbd289a762995e5680be310b444d4ac33f26 Mon Sep 17 00:00:00 2001 From: Bartosz Wieczorek Date: Fri, 6 Oct 2023 11:34:39 +0200 Subject: [PATCH] remove custom allocated resources from curl implementation --- PAM/ssh/include/rublon/core_handler.hpp | 2 +- PAM/ssh/include/rublon/curl.hpp | 6 +- PAM/ssh/tests/core_handler_tests.cpp | 15 +++-- PAM/ssh/tests/http_mock.hpp | 73 +++++++++++++++---------- 4 files changed, 55 insertions(+), 41 deletions(-) diff --git a/PAM/ssh/include/rublon/core_handler.hpp b/PAM/ssh/include/rublon/core_handler.hpp index ce3d48d..c1b46c5 100644 --- a/PAM/ssh/include/rublon/core_handler.hpp +++ b/PAM/ssh/include/rublon/core_handler.hpp @@ -132,7 +132,7 @@ class CoreHandler : public CoreHandlerInterface< CoreHandler< HttpHandler > > { std::pmr::string uri{url + path.data(), &memoryResource}; return http - .request(uri, request) // + .request(uri, request, response) // .and_then(validateSignature) .and_then(validateResponse) .or_else(handleError); diff --git a/PAM/ssh/include/rublon/curl.hpp b/PAM/ssh/include/rublon/curl.hpp index cf7801f..ebd9994 100644 --- a/PAM/ssh/include/rublon/curl.hpp +++ b/PAM/ssh/include/rublon/curl.hpp @@ -65,8 +65,8 @@ class CURL { public: CURL() : curl{std::unique_ptr< ::CURL, void (*)(::CURL *) >(curl_easy_init(), curl_easy_cleanup)} {} - - tl::expected< Response, HttpError > request(std::string_view uri, const Request & request) const { + + tl::expected< std::reference_wrapper, HttpError > request(std::string_view uri, const Request & request, Response&response) const { memory::MonotonicStackResource< 8 * 1024 > stackResource; std::pmr::string response_data{&stackResource}; @@ -110,8 +110,6 @@ class CURL { long size; curl_easy_getinfo(curl.get(), CURLINFO_HEADER_SIZE, &size); - Response response{memory::default_resource()}; - details::headers(response_data, response.headers); response.body = response_data.substr(size); diff --git a/PAM/ssh/tests/core_handler_tests.cpp b/PAM/ssh/tests/core_handler_tests.cpp index 2a3dec2..6a36b1c 100644 --- a/PAM/ssh/tests/core_handler_tests.cpp +++ b/PAM/ssh/tests/core_handler_tests.cpp @@ -21,6 +21,7 @@ class CoreHandlerTestable : public CoreHandler< HttpHandlerMock > { static RapidJSONPMRAlloc alloc{mr}; return CoreHandler< HttpHandlerMock >::request(alloc, path, body); } + }; class CoreHandlerTests : public testing::Test { @@ -31,6 +32,7 @@ class CoreHandlerTests : public testing::Test { CoreHandlerTestable sut; HttpHandlerMock & http; + rublon::Response _res{std::pmr::get_default_resource()}; Document doc; }; @@ -38,31 +40,31 @@ class CoreHandlerTests : public testing::Test { using namespace testing; TEST_F(CoreHandlerTests, coreShouldSetConnectionErrorOnBrokenConnection) { - EXPECT_CALL(http, request(_, _)).WillOnce(Return(RawHttpResponse{}.withTimeoutError())); + EXPECT_CALL(http, request(_, _)).WillOnce(Return(RawHttpResponse{_res}.withTimeoutError())); EXPECT_THAT(sut.request("", doc), // AllOf(Not(HasValue()), Unexpected(HttpError{}))); } TEST_F(CoreHandlerTests, coreShouldCheckSignatureAndReturnBadSignatureBeforeAnythingElse) { - EXPECT_CALL(http, request(_, _)).WillOnce(Return(RawHttpResponse{}.initMessage().withBrokenBody().withBrokenSignature())); + EXPECT_CALL(http, request(_, _)).WillOnce(Return(RawHttpResponse{_res}.initMessage().withBrokenBody().withBrokenSignature())); EXPECT_THAT(sut.request("", doc), // AllOf(Not(HasValue()), Unexpected(CoreHandlerError{CoreHandlerError::BadSigature}))); } TEST_F(CoreHandlerTests, coreShouldSetBrokenDataWhenResultIsNotAvailable) { - EXPECT_CALL(http, request(_, _)).WillOnce(Return(RawHttpResponse{}.initMessage().withBrokenBody())); + EXPECT_CALL(http, request(_, _)).WillOnce(Return(RawHttpResponse{_res}.initMessage().withBrokenBody())); EXPECT_THAT(sut.request("", doc), // AllOf(Not(HasValue()), Unexpected(CoreHandlerError{CoreHandlerError::BrokenData}))); } TEST_F(CoreHandlerTests, coreSignatureIsBeingChecked) { - EXPECT_CALL(http, request(Eq(conf.parameters.apiServer + "/path"), _)).WillOnce(Return(RawHttpResponse{}.initMessage())); + EXPECT_CALL(http, request(Eq(conf.parameters.apiServer + "/path"), _)).WillOnce(Return(RawHttpResponse{_res}.initMessage())); EXPECT_THAT(sut.request("/path", doc), // AllOf(HasValue(), IsObject(), HasKey("/result/tid"))); } TEST_F(CoreHandlerTests, onHttpProblemsAccessShouldBeDeniedByDefault) { - EXPECT_CALL(http, request(_, _)).WillOnce(Return(RawHttpResponse{}.initMessage().withServiceUnavailableError())); + EXPECT_CALL(http, request(_, _)).WillOnce(Return(RawHttpResponse{_res}.initMessage().withServiceUnavailableError())); EXPECT_THAT(sut.request("/path", doc), // AllOf(Not(HasValue()), Unexpected(PamDeny{}))); } @@ -73,11 +75,12 @@ class CoreHandlerWithBypassTests : public testing::Test { } CoreHandlerTestable sut; + rublon::Response _res{memory::default_resource()}; HttpHandlerMock & http; }; TEST_F(CoreHandlerWithBypassTests, onHttpProblemsAccessShouldBeBypassedWhenEnabled) { - EXPECT_CALL(http, request(_, _)).WillOnce(Return(RawHttpResponse{}.initMessage().withServiceUnavailableError())); + EXPECT_CALL(http, request(_, _)).WillOnce(Return(RawHttpResponse{_res}.initMessage().withServiceUnavailableError())); EXPECT_THAT(sut.request("/path", Document{}), // AllOf(Not(HasValue()), Unexpected(PamBaypass{}))); } diff --git a/PAM/ssh/tests/http_mock.hpp b/PAM/ssh/tests/http_mock.hpp index b316adb..b640fd7 100644 --- a/PAM/ssh/tests/http_mock.hpp +++ b/PAM/ssh/tests/http_mock.hpp @@ -136,65 +136,65 @@ class CodeVerificationResponse { template < typename Generator > class ResponseBase { public: - ResponseBase & initMessage() { + Generator & initMessage() { _coreGenerator = InitCoreResponse{}; - return *this; + return static_cast< Generator & >(*this); } - ResponseBase & selectMethodMessage() { + Generator & selectMethodMessage() { _coreGenerator = MethodSelectCoreResponse{}; - return *this; + return static_cast< Generator & >(*this); } - ResponseBase & codeConfirmationMessage() { + Generator & codeConfirmationMessage() { _coreGenerator = CodeVerificationResponse{}; - return *this; + return static_cast< Generator & >(*this); } - ResponseBase & withBrokenBody() { + Generator & withBrokenBody() { options.generateBrokenBody = true; - return *this; + return static_cast< Generator & >(*this); } - ResponseBase & withCoreException() { + Generator & withCoreException() { options.coreException = "some exception"; - return *this; + return static_cast< Generator & >(*this); } - ResponseBase & withBrokenSignature() { + Generator & withBrokenSignature() { options.generateBrokenSigneture = true; - return *this; + return static_cast< Generator & >(*this); } - ResponseBase & withWrongCodeResponse() { + Generator & withWrongCodeResponse() { options.generateBrokenCode = true; - return *this; + return static_cast< Generator & >(*this); } - ResponseBase & withTimeoutError() { + Generator & withTimeoutError() { options.error = rublon::Error{rublon::HttpError{}}; - return *this; + return static_cast< Generator & >(*this); } - ResponseBase & withServiceUnavailableError() { + Generator & withServiceUnavailableError() { options.error = rublon::Error{rublon::HttpError{rublon::HttpError::Error, 405}}; - return *this; + return static_cast< Generator & >(*this); } template < typename... T > - ResponseBase & withMethods(T... methods) { + Generator & withMethods(T... methods) { (options.methods.insert(methods), ...); - return *this; + return static_cast< Generator & >(*this); } - ResponseBase & withConnectionError() { + Generator & withConnectionError() { // options.error = rublon::Error{rublon::CoreHandlerError{rublon::CoreHandlerError::ConnectionError}}; - return *this; + return static_cast< Generator & >(*this); } - - ResponseBase & withBasSignature() { + + Generator & withBasSignature() { options.error = rublon::Error{rublon::CoreHandlerError{rublon::CoreHandlerError::BadSigature}}; - return *this; + return static_cast< Generator & >(*this); } operator auto() { @@ -221,15 +221,20 @@ class RawCoreResponse : public ResponseBase< RawCoreResponse > { }; class RawHttpResponse : public ResponseBase< RawHttpResponse > { + rublon::Response & response; + + public: + RawHttpResponse(rublon::Response & r) + : ResponseBase< RawHttpResponse >(), + response{r} { + + }; static auto signResponse(rublon::Response & res, ResponseMockOptions opts) { const auto & sign = opts.generateBrokenSigneture ? std::array< char, 65 >{} : rublon::signData(res.body, conf.parameters.secretKey.c_str()); res.headers["x-rublon-signature"] = sign.data(); } - - public: - tl::expected< rublon::Response, rublon::Error > generate() { - rublon::Response response{std::pmr::get_default_resource()}; + tl::expected< std::reference_wrapper< rublon::Response >, rublon::Error > generate() { response.body = std::visit([&](const auto generator) { return generator.generate(options); }, _coreGenerator); signResponse(response, options); return response; @@ -241,5 +246,13 @@ class HttpHandlerMock { template < typename... Args > HttpHandlerMock(const Args &...) {} - MOCK_METHOD(( tl::expected< rublon::Response, rublon::Error > ), request, ( std::string_view, const rublon::Request & ), (const)); + MOCK_METHOD(( tl::expected< std::reference_wrapper< rublon::Response >, rublon::Error > ), + request, + ( std::string_view, const rublon::Request & ), + (const)); + + tl::expected< std::reference_wrapper< rublon::Response >, rublon::Error > + request(std::string_view uri, const rublon::Request & req, rublon::Response &) const { + return request(uri, req); + } };