From e67a4284c0e9a2fb194335893872bc171e968029 Mon Sep 17 00:00:00 2001 From: Leorize Date: Thu, 16 Jul 2020 16:58:46 -0500 Subject: [PATCH] libbnetapi: Disallow instantiation of BUrlRequest subclasses directly This API change forces all creation of BUrlRequest to be done via BUrlProtocolRoster::MakeRequest(). This allows the structure of protocol addons to be altered without breaking ABI for client applications. Change-Id: I1785c9136c50d19eaa9e57cb9d259ed8d88a5b56 Reviewed-on: https://review.haiku-os.org/c/haiku/+/3080 Reviewed-by: waddlesplash --- headers/os/net/DataRequest.h | 10 ++- headers/os/net/FileRequest.h | 12 ++-- headers/os/net/GopherRequest.h | 10 ++- headers/os/net/HttpRequest.h | 16 +++-- .../server/AbstractServerProcess.cpp | 19 ++++-- .../haikudepot/server/WebAppInterface.cpp | 46 +++++++++---- src/tests/kits/net/service/DataTest.cpp | 12 ++-- src/tests/kits/net/service/HttpTest.cpp | 67 ++++++++++--------- 8 files changed, 121 insertions(+), 71 deletions(-) diff --git a/headers/os/net/DataRequest.h b/headers/os/net/DataRequest.h index 9e5f546971..8e4debf13e 100644 --- a/headers/os/net/DataRequest.h +++ b/headers/os/net/DataRequest.h @@ -11,17 +11,21 @@ #define _B_DATA_REQUEST_H_ +#include #include class BDataRequest: public BUrlRequest { public: + const BUrlResult& Result() const; +private: + friend class BUrlProtocolRoster; + BDataRequest(const BUrl& url, BUrlProtocolListener* listener = NULL, BUrlContext* context = NULL); - const BUrlResult& Result() const; -private: - status_t _ProtocolLoop(); + + status_t _ProtocolLoop(); private: BUrlResult fResult; }; diff --git a/headers/os/net/FileRequest.h b/headers/os/net/FileRequest.h index 4ee8cf25d3..6701860e40 100644 --- a/headers/os/net/FileRequest.h +++ b/headers/os/net/FileRequest.h @@ -10,19 +10,23 @@ #include +#include class BFileRequest : public BUrlRequest { public: - BFileRequest(const BUrl& url, - BUrlProtocolListener* listener = NULL, - BUrlContext* context = NULL); virtual ~BFileRequest(); const BUrlResult& Result() const; - void SetDisableListener(bool disable); + void SetDisableListener(bool disable); private: + friend class BUrlProtocolRoster; + + BFileRequest(const BUrl& url, + BUrlProtocolListener* listener = NULL, + BUrlContext* context = NULL); + status_t _ProtocolLoop(); private: BUrlResult fResult; diff --git a/headers/os/net/GopherRequest.h b/headers/os/net/GopherRequest.h index ad38f96ade..7e627c8078 100644 --- a/headers/os/net/GopherRequest.h +++ b/headers/os/net/GopherRequest.h @@ -9,13 +9,11 @@ #include #include +#include class BGopherRequest : public BNetworkRequest { public: - BGopherRequest(const BUrl& url, - BUrlProtocolListener* listener = NULL, - BUrlContext* context = NULL); virtual ~BGopherRequest(); status_t Stop(); @@ -23,6 +21,12 @@ public: void SetDisableListener(bool disable); private: + friend class BUrlProtocolRoster; + + BGopherRequest(const BUrl& url, + BUrlProtocolListener* listener = NULL, + BUrlContext* context = NULL); + status_t _ProtocolLoop(); void _SendRequest(); diff --git a/headers/os/net/HttpRequest.h b/headers/os/net/HttpRequest.h index 7704baedf9..0f0541155a 100644 --- a/headers/os/net/HttpRequest.h +++ b/headers/os/net/HttpRequest.h @@ -14,6 +14,7 @@ #include #include #include +#include namespace BPrivate { @@ -24,12 +25,6 @@ namespace BPrivate { class BHttpRequest : public BNetworkRequest { public: - BHttpRequest(const BUrl& url, - bool ssl = false, - const char* protocolName = "HTTP", - BUrlProtocolListener* listener = NULL, - BUrlContext* context = NULL); - BHttpRequest(const BHttpRequest& other); virtual ~BHttpRequest(); void SetMethod(const char* const method); @@ -64,6 +59,15 @@ public: static int16 StatusCodeClass(int16 code); private: + friend class BUrlProtocolRoster; + + BHttpRequest(const BUrl& url, + bool ssl = false, + const char* protocolName = "HTTP", + BUrlProtocolListener* listener = NULL, + BUrlContext* context = NULL); + BHttpRequest(const BHttpRequest& other); + void _ResetOptions(); status_t _ProtocolLoop(); status_t _MakeRequest(); diff --git a/src/apps/haikudepot/server/AbstractServerProcess.cpp b/src/apps/haikudepot/server/AbstractServerProcess.cpp index d02a267d98..d438f30f1f 100644 --- a/src/apps/haikudepot/server/AbstractServerProcess.cpp +++ b/src/apps/haikudepot/server/AbstractServerProcess.cpp @@ -344,14 +344,19 @@ AbstractServerProcess::DownloadToLocalFile(const BPath& targetFilePath, thread_id thread; - { - fRequest = dynamic_cast( - BUrlProtocolRoster::MakeRequest(url, &listener)); - fRequest->SetHeaders(headers); - fRequest->SetMaxRedirections(0); - fRequest->SetTimeout(TIMEOUT_MICROSECONDS); - thread = fRequest->Run(); + BUrlRequest* request = BUrlProtocolRoster::MakeRequest(url, &listener); + if (request == NULL) + return B_NO_MEMORY; + + fRequest = dynamic_cast(request); + if (fRequest == NULL) { + delete request; + return B_ERROR; } + fRequest->SetHeaders(headers); + fRequest->SetMaxRedirections(0); + fRequest->SetTimeout(TIMEOUT_MICROSECONDS); + thread = fRequest->Run(); wait_for_thread(thread, NULL); diff --git a/src/apps/haikudepot/server/WebAppInterface.cpp b/src/apps/haikudepot/server/WebAppInterface.cpp index 9a1cba7f07..62ecf2fcef 100644 --- a/src/apps/haikudepot/server/WebAppInterface.cpp +++ b/src/apps/haikudepot/server/WebAppInterface.cpp @@ -6,6 +6,7 @@ #include "WebAppInterface.h" +#include #include #include #include @@ -96,6 +97,21 @@ private: }; +static BHttpRequest* +make_http_request(const BUrl& url, BUrlProtocolListener* listener = NULL, + BUrlContext* context = NULL) +{ + BUrlRequest* request = BUrlProtocolRoster::MakeRequest(url, listener, + context); + BHttpRequest* httpRequest = dynamic_cast(request); + if (httpRequest == NULL) { + delete request; + return NULL; + } + return httpRequest; +} + + int WebAppInterface::fRequestIndex = 0; @@ -825,7 +841,6 @@ WebAppInterface::_SendJsonRequest(const char* domain, } BUrl url = ServerSettings::CreateFullUrl(BString("/__api/v1/") << domain); - bool isSecure = url.Protocol() == "https"; HDDEBUG("jrpc; will make request to [%s]", url.UrlString().String()); // If the request payload is logged then it must be copied to local memory @@ -846,9 +861,12 @@ WebAppInterface::_SendJsonRequest(const char* domain, headers.AddHeader("Content-Type", "application/json"); ServerSettings::AugmentHeaders(headers); - BHttpRequest request(url, isSecure, "HTTP", &listener, &context); - request.SetMethod(B_HTTP_POST); - request.SetHeaders(headers); + BHttpRequest* request = make_http_request(url, &listener, &context); + ObjectDeleter _(request); + if (request == NULL) + return B_ERROR; + request->SetMethod(B_HTTP_POST); + request->SetHeaders(headers); // Authentication via Basic Authentication // The other way would be to obtain a token and then use the Token Bearer @@ -860,16 +878,16 @@ WebAppInterface::_SendJsonRequest(const char* domain, context.AddAuthentication(url, authentication); } - request.AdoptInputData(requestData, requestDataSize); + request->AdoptInputData(requestData, requestDataSize); BMallocIO replyData; listener.SetDownloadIO(&replyData); - thread_id thread = request.Run(); + thread_id thread = request->Run(); wait_for_thread(thread, NULL); const BHttpResult& result = dynamic_cast( - request.Result()); + request->Result()); int32 statusCode = result.StatusCode(); @@ -930,7 +948,6 @@ WebAppInterface::_SendRawGetRequest(const BString urlPathComponents, BDataIO* stream) { BUrl url = ServerSettings::CreateFullUrl(urlPathComponents); - bool isSecure = url.Protocol() == "https"; ProtocolListener listener; listener.SetDownloadIO(stream); @@ -938,15 +955,18 @@ WebAppInterface::_SendRawGetRequest(const BString urlPathComponents, BHttpHeaders headers; ServerSettings::AugmentHeaders(headers); - BHttpRequest request(url, isSecure, "HTTP", &listener); - request.SetMethod(B_HTTP_GET); - request.SetHeaders(headers); + BHttpRequest *request = make_http_request(url, &listener); + ObjectDeleter _(request); + if (request == NULL) + return B_ERROR; + request->SetMethod(B_HTTP_GET); + request->SetHeaders(headers); - thread_id thread = request.Run(); + thread_id thread = request->Run(); wait_for_thread(thread, NULL); const BHttpResult& result = dynamic_cast( - request.Result()); + request->Result()); int32 statusCode = result.StatusCode(); diff --git a/src/tests/kits/net/service/DataTest.cpp b/src/tests/kits/net/service/DataTest.cpp index 0dcbfda657..5251e041d7 100644 --- a/src/tests/kits/net/service/DataTest.cpp +++ b/src/tests/kits/net/service/DataTest.cpp @@ -6,7 +6,9 @@ #include "DataTest.h" +#include #include +#include #include @@ -155,12 +157,14 @@ DataTest::_RunTest(BString url, const char* expected, size_t expectedLength) NextSubTest(); BUrl testUrl(url); - BDataRequest t(testUrl); + ObjectDeleter requestDeleter( + BUrlProtocolRoster::MakeRequest(testUrl, this)); + BDataRequest* request = dynamic_cast(requestDeleter.Get()); + CPPUNIT_ASSERT(request != NULL); fReceivedData.clear(); - t.SetListener(this); - t.Run(); + request->Run(); - while(t.IsRunning()) + while(request->IsRunning()) snooze(1000); CPPUNIT_ASSERT_EQUAL(expectedLength, fReceivedData.size()); diff --git a/src/tests/kits/net/service/HttpTest.cpp b/src/tests/kits/net/service/HttpTest.cpp index f27c80fe04..81d2c2eae6 100644 --- a/src/tests/kits/net/service/HttpTest.cpp +++ b/src/tests/kits/net/service/HttpTest.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include @@ -125,22 +126,23 @@ void SendAuthenticatedRequest( { TestListener listener(expectedResponseBody, expectedResponseHeaders); - BHttpRequest request(testUrl, testUrl.Protocol() == "https"); - request.SetContext(&context); - request.SetListener(&listener); + ObjectDeleter requestDeleter( + BUrlProtocolRoster::MakeRequest(testUrl, &listener, &context)); + BHttpRequest* request = dynamic_cast(requestDeleter.Get()); + CPPUNIT_ASSERT(request != NULL); - request.SetUserName("walter"); - request.SetPassword("secret"); + request->SetUserName("walter"); + request->SetPassword("secret"); - CPPUNIT_ASSERT(request.Run()); + CPPUNIT_ASSERT(request->Run()); - while (request.IsRunning()) + while (request->IsRunning()) snooze(1000); - CPPUNIT_ASSERT_EQUAL(B_OK, request.Status()); + CPPUNIT_ASSERT_EQUAL(B_OK, request->Status()); const BHttpResult &result = - dynamic_cast(request.Result()); + dynamic_cast(request->Result()); CPPUNIT_ASSERT_EQUAL(200, result.StatusCode()); CPPUNIT_ASSERT_EQUAL(BString("OK"), result.StatusText()); @@ -220,18 +222,19 @@ HttpTest::GetTest() TestListener listener(expectedResponseBody, expectedResponseHeaders); - BHttpRequest request(testUrl, testUrl.Protocol() == "https"); - request.SetContext(context); - request.SetListener(&listener); + ObjectDeleter requestDeleter( + BUrlProtocolRoster::MakeRequest(testUrl, &listener, context)); + BHttpRequest* request = dynamic_cast(requestDeleter.Get()); + CPPUNIT_ASSERT(request != NULL); - CPPUNIT_ASSERT(request.Run()); - while (request.IsRunning()) + CPPUNIT_ASSERT(request->Run()); + while (request->IsRunning()) snooze(1000); - CPPUNIT_ASSERT_EQUAL(B_OK, request.Status()); + CPPUNIT_ASSERT_EQUAL(B_OK, request->Status()); const BHttpResult& result - = dynamic_cast(request.Result()); + = dynamic_cast(request->Result()); CPPUNIT_ASSERT_EQUAL(200, result.StatusCode()); CPPUNIT_ASSERT_EQUAL(BString("OK"), result.StatusText()); @@ -282,19 +285,20 @@ HttpTest::ProxyTest() TestListener listener(expectedResponseBody, expectedResponseHeaders); - BHttpRequest request(testUrl); - request.SetContext(context); - request.SetListener(&listener); + ObjectDeleter requestDeleter( + BUrlProtocolRoster::MakeRequest(testUrl, &listener, context)); + BHttpRequest* request = dynamic_cast(requestDeleter.Get()); + CPPUNIT_ASSERT(request != NULL); - CPPUNIT_ASSERT(request.Run()); + CPPUNIT_ASSERT(request->Run()); - while (request.IsRunning()) + while (request->IsRunning()) snooze(1000); - CPPUNIT_ASSERT_EQUAL(B_OK, request.Status()); + CPPUNIT_ASSERT_EQUAL(B_OK, request->Status()); const BHttpResult& response - = dynamic_cast(request.Result()); + = dynamic_cast(request->Result()); CPPUNIT_ASSERT_EQUAL(200, response.StatusCode()); CPPUNIT_ASSERT_EQUAL(BString("OK"), response.StatusText()); CPPUNIT_ASSERT_EQUAL(169, response.Length()); @@ -373,9 +377,10 @@ HttpTest::UploadTest() BUrlContext context; - BHttpRequest request(testUrl, testUrl.Protocol() == "https"); - request.SetContext(&context); - request.SetListener(&listener); + ObjectDeleter requestDeleter( + BUrlProtocolRoster::MakeRequest(testUrl, &listener, &context)); + BHttpRequest* request = dynamic_cast(requestDeleter.Get()); + CPPUNIT_ASSERT(request != NULL); BHttpForm form; form.AddString("hello", "world"); @@ -383,17 +388,17 @@ HttpTest::UploadTest() B_OK, form.AddFile("_uploadfile", BPath(testFilePath.c_str()))); - request.SetPostFields(form); + request->SetPostFields(form); - CPPUNIT_ASSERT(request.Run()); + CPPUNIT_ASSERT(request->Run()); - while (request.IsRunning()) + while (request->IsRunning()) snooze(1000); - CPPUNIT_ASSERT_EQUAL(B_OK, request.Status()); + CPPUNIT_ASSERT_EQUAL(B_OK, request->Status()); const BHttpResult &result = - dynamic_cast(request.Result()); + dynamic_cast(request->Result()); CPPUNIT_ASSERT_EQUAL(200, result.StatusCode()); CPPUNIT_ASSERT_EQUAL(BString("OK"), result.StatusText()); CPPUNIT_ASSERT_EQUAL(913, result.Length());