From 43e293e66400209880fc18da30cd54c47e8acb16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julio=20C=C3=A9sar=20Rocha?= Date: Thu, 9 Dec 2021 17:21:43 -0800 Subject: [PATCH 1/4] Avoid capturing raw `this` pointer in WebSocket module (#9247) * Use implicit (brace) constructor for getMethods() * Define CxxModuleHolder * Make m_asyncStorageManager shared_ptr * Change files * Initialize m_holder * Implement WebSocketModule::SharedState * Move ResourceFactory into shared state * Revert AsyncStorage changes * Add URL argument to IWebSocketResource::Connect * Drop URL from factory method * Update BeastWebSocketResource * Handle malformed Uris within ::Connect() * Remove urlString argument from IWebSocketResource::Make * Use constexpr for commont test URLs * Remove redundant move in to_hstring * Add/remove std::move where applicable Co-authored-by: Nick Gerleman --- ...-e21dbe3d-18ab-42a0-a4a3-f96fb8c0997f.json | 7 + vnext/Desktop.DLL/react-native-win32.x64.def | 2 +- vnext/Desktop.DLL/react-native-win32.x86.def | 2 +- .../WebSocketIntegrationTest.cpp | 32 +-- .../WebSocketResourcePerformanceTests.cpp | 4 +- .../Desktop.UnitTests/BaseWebSocketTests.cpp | 24 +- vnext/Desktop.UnitTests/WebSocketMocks.cpp | 7 +- vnext/Desktop.UnitTests/WebSocketMocks.h | 4 +- .../Desktop.UnitTests/WebSocketModuleTest.cpp | 2 +- .../WinRTWebSocketResourceUnitTest.cpp | 14 +- vnext/Desktop/BeastWebSocketResource.cpp | 103 ++++++- vnext/Desktop/BeastWebSocketResource.h | 85 +++++- vnext/Desktop/WebSocketResourceFactory.cpp | 21 +- .../Modules/CreateModules.cpp | 4 +- vnext/Shared/IWebSocketResource.h | 12 +- vnext/Shared/InspectorPackagerConnection.cpp | 5 +- vnext/Shared/Modules/WebSocketModule.cpp | 253 +++++++++--------- vnext/Shared/Modules/WebSocketModule.h | 34 ++- vnext/Shared/WinRTWebSocketResource.cpp | 40 ++- vnext/Shared/WinRTWebSocketResource.h | 8 +- 20 files changed, 430 insertions(+), 233 deletions(-) create mode 100644 change/react-native-windows-e21dbe3d-18ab-42a0-a4a3-f96fb8c0997f.json diff --git a/change/react-native-windows-e21dbe3d-18ab-42a0-a4a3-f96fb8c0997f.json b/change/react-native-windows-e21dbe3d-18ab-42a0-a4a3-f96fb8c0997f.json new file mode 100644 index 00000000000..93547280d96 --- /dev/null +++ b/change/react-native-windows-e21dbe3d-18ab-42a0-a4a3-f96fb8c0997f.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "Avoid capturing raw `this` pointer in WS and AsyncStorage'", + "packageName": "react-native-windows", + "email": "julio.rocha@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/vnext/Desktop.DLL/react-native-win32.x64.def b/vnext/Desktop.DLL/react-native-win32.x64.def index 0bc740ebf47..bf32f6fc7c6 100644 --- a/vnext/Desktop.DLL/react-native-win32.x64.def +++ b/vnext/Desktop.DLL/react-native-win32.x64.def @@ -22,7 +22,7 @@ EXPORTS ?GetConstants@ViewManagerBase@react@facebook@@UEBA?AUdynamic@folly@@XZ ?InitializeLogging@react@facebook@@YAX$$QEAV?$function@$$A6AXW4RCTLogLevel@react@facebook@@PEBD@Z@std@@@Z ?InitializeTracing@react@facebook@@YAXPEAUINativeTraceHandler@12@@Z -?Make@IWebSocketResource@React@Microsoft@@SA?AV?$shared_ptr@UIWebSocketResource@React@Microsoft@@@std@@$$QEAV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@5@@Z +?Make@IWebSocketResource@React@Microsoft@@SA?AV?$shared_ptr@UIWebSocketResource@React@Microsoft@@@std@@XZ ?Make@JSBigAbiString@react@facebook@@SA?AV?$unique_ptr@$$CBUJSBigAbiString@react@facebook@@U?$default_delete@$$CBUJSBigAbiString@react@facebook@@@std@@@std@@$$QEAV?$unique_ptr@U?$IAbiArray@D@AbiSafe@@UAbiObjectDeleter@2@@5@@Z ?at@dynamic@folly@@QEGBAAEBU12@V?$Range@PEBD@2@@Z ?atImpl@dynamic@folly@@AEGBAAEBU12@AEBU12@@Z diff --git a/vnext/Desktop.DLL/react-native-win32.x86.def b/vnext/Desktop.DLL/react-native-win32.x86.def index f77e326570d..d3487e29742 100644 --- a/vnext/Desktop.DLL/react-native-win32.x86.def +++ b/vnext/Desktop.DLL/react-native-win32.x86.def @@ -22,7 +22,7 @@ EXPORTS ?GetConstants@ViewManagerBase@react@facebook@@UBE?AUdynamic@folly@@XZ ?InitializeLogging@react@facebook@@YGX$$QAV?$function@$$A6GXW4RCTLogLevel@react@facebook@@PBD@Z@std@@@Z ?InitializeTracing@react@facebook@@YGXPAUINativeTraceHandler@12@@Z -?Make@IWebSocketResource@React@Microsoft@@SG?AV?$shared_ptr@UIWebSocketResource@React@Microsoft@@@std@@$$QAV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@5@@Z +?Make@IWebSocketResource@React@Microsoft@@SG?AV?$shared_ptr@UIWebSocketResource@React@Microsoft@@@std@@XZ ?Make@JSBigAbiString@react@facebook@@SG?AV?$unique_ptr@$$CBUJSBigAbiString@react@facebook@@U?$default_delete@$$CBUJSBigAbiString@react@facebook@@@std@@@std@@$$QAV?$unique_ptr@U?$IAbiArray@D@AbiSafe@@UAbiObjectDeleter@2@@5@@Z ?moduleNames@ModuleRegistry@react@facebook@@QAE?AV?$vector@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@V?$allocator@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@2@@std@@XZ ?at@dynamic@folly@@QGBEABU12@V?$Range@PBD@2@@Z diff --git a/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp b/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp index 05f719b8085..a540ad82bf0 100644 --- a/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp +++ b/vnext/Desktop.IntegrationTests/WebSocketIntegrationTest.cpp @@ -39,7 +39,7 @@ TEST_CLASS (WebSocketIntegrationTest) string scheme = "ws"; if (isSecure) scheme += "s"; - auto ws = IWebSocketResource::Make(scheme + "://localhost:5556/"); + auto ws = IWebSocketResource::Make(); promise sentSizePromise; ws->SetOnSend([&sentSizePromise](size_t size) { @@ -61,7 +61,7 @@ TEST_CLASS (WebSocketIntegrationTest) server->Start(); string sent = "prefix"; auto expectedSize = sent.size(); - ws->Connect(); + ws->Connect(scheme + "://localhost:5556/"); ws->Send(std::move(sent)); // Block until response is received. Fail in case of a remote endpoint failure. @@ -85,7 +85,7 @@ TEST_CLASS (WebSocketIntegrationTest) TEST_METHOD(ConnectClose) { auto server = make_shared(5556); - auto ws = IWebSocketResource::Make("ws://localhost:5556/"); + auto ws = IWebSocketResource::Make(); Assert::IsFalse(nullptr == ws); bool connected = false; bool closed = false; @@ -105,7 +105,7 @@ TEST_CLASS (WebSocketIntegrationTest) }); server->Start(); - ws->Connect(); + ws->Connect("ws://localhost:5556/"); ws->Close(CloseCode::Normal, "Closing"); server->Stop(); @@ -124,7 +124,7 @@ TEST_CLASS (WebSocketIntegrationTest) // IWebSocketResource scope. Ensures object is closed implicitly by destructor. { - auto ws = IWebSocketResource::Make("ws://localhost:5556"); + auto ws = IWebSocketResource::Make(); ws->SetOnConnect([&connected]() { connected = true; @@ -138,7 +138,7 @@ TEST_CLASS (WebSocketIntegrationTest) errorMessage = error.Message; }); - ws->Connect(); + ws->Connect("ws://localhost:5556"); ws->Close();//TODO: Either remove or rename test. } @@ -156,7 +156,7 @@ TEST_CLASS (WebSocketIntegrationTest) auto server = make_shared(5556); server->Start(); - auto ws = IWebSocketResource::Make("ws://localhost:5556"); + auto ws = IWebSocketResource::Make(); promise pingPromise; ws->SetOnPing([&pingPromise]() { @@ -168,7 +168,7 @@ TEST_CLASS (WebSocketIntegrationTest) errorString = err.Message; }); - ws->Connect(); + ws->Connect("ws://localhost:5556"); ws->Ping(); auto pingFuture = pingPromise.get_future(); pingFuture.wait(); @@ -189,14 +189,14 @@ TEST_CLASS (WebSocketIntegrationTest) TEST_METHOD(SendReceiveLargeMessage) { auto server = make_shared(5556); server->SetMessageFactory([](string &&message) { return message + "_response"; }); - auto ws = IWebSocketResource::Make("ws://localhost:5556/"); + auto ws = IWebSocketResource::Make(); promise response; ws->SetOnMessage([&response](size_t size, const string &message, bool isBinary) { response.set_value(message); }); string errorMessage; ws->SetOnError([&errorMessage](Error err) { errorMessage = err.Message; }); server->Start(); - ws->Connect(); + ws->Connect("ws://localhost:5556"); char digits[] = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'}; #define LEN 4096 + 4096 * 2 + 1 @@ -246,12 +246,12 @@ TEST_CLASS (WebSocketIntegrationTest) auto cookie = response[boost::beast::http::field::cookie].to_string(); server->SetMessageFactory([cookie](string &&) { return cookie; }); }); - auto ws = IWebSocketResource::Make("ws://localhost:5556/"); + auto ws = IWebSocketResource::Make(); promise response; ws->SetOnMessage([&response](size_t size, const string &message, bool isBinary) { response.set_value(message); }); server->Start(); - ws->Connect({}, {{L"Cookie", "JSESSIONID=AD9A320CC4034641997FF903F1D10906"}}); + ws->Connect("ws://localhost:5556/", {}, {{L"Cookie", "JSESSIONID=AD9A320CC4034641997FF903F1D10906"}}); ws->Send(""); auto future = response.get_future(); @@ -285,7 +285,7 @@ TEST_CLASS (WebSocketIntegrationTest) return message; }); - auto ws = IWebSocketResource::Make("ws://localhost:5556/"); + auto ws = IWebSocketResource::Make(); std::vector messages{ "AQ==", // [ 01 ] @@ -307,7 +307,7 @@ TEST_CLASS (WebSocketIntegrationTest) }); server->Start(); - ws->Connect(); + ws->Connect("ws://localhost:5556"); // Send all but the last message. // Compare result with the next message in the sequence. @@ -336,7 +336,7 @@ TEST_CLASS (WebSocketIntegrationTest) { return message; }); - auto ws = IWebSocketResource::Make("ws://localhost:5556/"); + auto ws = IWebSocketResource::Make(); string expected = "ABCDEFGHIJ"; string result(expected.size(), '0'); @@ -356,7 +356,7 @@ TEST_CLASS (WebSocketIntegrationTest) }); server->Start(); - ws->Connect(); + ws->Connect("ws://localhost:5556"); // Consecutive immediate writes should be enqueued. // The WebSocket library (WinRT or Beast) can't handle multiple write operations diff --git a/vnext/Desktop.IntegrationTests/WebSocketResourcePerformanceTests.cpp b/vnext/Desktop.IntegrationTests/WebSocketResourcePerformanceTests.cpp index 628576fc5b5..ff708605652 100644 --- a/vnext/Desktop.IntegrationTests/WebSocketResourcePerformanceTests.cpp +++ b/vnext/Desktop.IntegrationTests/WebSocketResourcePerformanceTests.cpp @@ -71,7 +71,7 @@ TEST_CLASS (WebSocketResourcePerformanceTest) { { vector> resources; for (int i = 0; i < resourceTotal; i++) { - auto ws = IWebSocketResource::Make("ws://localhost:5555/"); // TODO: Switch to port 5556 + auto ws = IWebSocketResource::Make(); ws->SetOnMessage([this, &threadCount, &errorFound](size_t size, const string &message, bool isBinary) { if (errorFound) return; @@ -103,7 +103,7 @@ TEST_CLASS (WebSocketResourcePerformanceTest) { errorFound = true; errorMessage = error.Message; }); - ws->Connect(); + ws->Connect("ws://localhost:5555"); // TODO: Switch to port 5556 resources.push_back(std::move(ws)); } // Create and store WS resources. diff --git a/vnext/Desktop.UnitTests/BaseWebSocketTests.cpp b/vnext/Desktop.UnitTests/BaseWebSocketTests.cpp index ae3aa109be4..c5e68faa19a 100644 --- a/vnext/Desktop.UnitTests/BaseWebSocketTests.cpp +++ b/vnext/Desktop.UnitTests/BaseWebSocketTests.cpp @@ -15,6 +15,12 @@ using std::make_unique; using std::promise; using std::string; +namespace { + +constexpr char testUrl[] = "ws://localhost"; + +} // namespace + namespace Microsoft::React::Test { using CloseCode = IWebSocketResource::CloseCode; @@ -30,7 +36,7 @@ TEST_CLASS(BaseWebSocketTest){ END_TEST_CLASS_ATTRIBUTE() TEST_METHOD(CreateAndSetHandlers){ - auto ws = make_unique(Url("ws://localhost")); + auto ws = make_unique(Url(testUrl)); Assert::IsFalse(nullptr == ws); ws->SetOnConnect([]() {}); @@ -44,11 +50,11 @@ TEST_CLASS(BaseWebSocketTest){ TEST_METHOD(ConnectSucceeds) { string errorMessage; bool connected = false; - auto ws = make_unique(Url("ws://localhost")); + auto ws = make_unique(Url(testUrl)); ws->SetOnError([&errorMessage](Error err) { errorMessage = err.Message; }); ws->SetOnConnect([&connected]() { connected = true; }); - ws->Connect({}, {}); + ws->Connect(testUrl, {}, {}); ws->Close(CloseCode::Normal, {}); Assert::AreEqual({}, errorMessage); @@ -58,14 +64,14 @@ TEST_CLASS(BaseWebSocketTest){ TEST_METHOD(ConnectFails) { string errorMessage; bool connected = false; - auto ws = make_unique(Url("ws://localhost")); + auto ws = make_unique(Url(testUrl)); ws->SetOnError([&errorMessage](Error err) { errorMessage = err.Message; }); ws->SetOnConnect([&connected]() { connected = true; }); ws->SetConnectResult([]() -> error_code { return make_error_code(errc::state_not_recoverable); }); - ws->Connect({}, {}); + ws->Connect(testUrl, {}, {}); ws->Close(CloseCode::Normal, {}); Assert::AreNotEqual({}, errorMessage); @@ -78,14 +84,14 @@ TEST_CLASS(BaseWebSocketTest){ TEST_METHOD(HandshakeFails) { string errorMessage; bool connected = false; - auto ws = make_unique(Url("ws://localhost")); + auto ws = make_unique(Url(testUrl)); ws->SetOnError([&errorMessage](Error err) { errorMessage = err.Message; }); ws->SetOnConnect([&connected]() { connected = true; }); ws->SetHandshakeResult([](string, string) -> error_code { return make_error_code(errc::state_not_recoverable); }); - ws->Connect({}, {}); + ws->Connect(testUrl, {}, {}); ws->Close(CloseCode::Normal, {}); Assert::AreNotEqual({}, errorMessage); @@ -99,7 +105,7 @@ TEST_CLASS(BaseWebSocketTest){ string errorMessage; promise connected; bool closed = false; - auto ws = make_unique(Url("ws://localhost")); + auto ws = make_unique(Url(testUrl)); ws->SetOnError([&errorMessage](Error err) { errorMessage = err.Message; }); ws->SetOnConnect([&connected]() { connected.set_value(); }); ws->SetOnClose( @@ -108,7 +114,7 @@ TEST_CLASS(BaseWebSocketTest){ ws->SetCloseResult( []() -> error_code { return make_error_code(errc::success); }); - ws->Connect({}, {}); + ws->Connect(testUrl, {}, {}); connected.get_future().wait(); ws->Close(CloseCode::Normal, "Normal"); diff --git a/vnext/Desktop.UnitTests/WebSocketMocks.cpp b/vnext/Desktop.UnitTests/WebSocketMocks.cpp index 6d5f2b646ac..00301705917 100644 --- a/vnext/Desktop.UnitTests/WebSocketMocks.cpp +++ b/vnext/Desktop.UnitTests/WebSocketMocks.cpp @@ -10,10 +10,13 @@ MockWebSocketResource::~MockWebSocketResource() {} #pragma region IWebSocketResource overrides -void MockWebSocketResource::Connect(const Protocols &protocols, const Options &options) noexcept /*override*/ +void MockWebSocketResource::Connect( + string &&url, + const Protocols &protocols, + const Options &options) noexcept /*override*/ { if (Mocks.Connect) - return Mocks.Connect(protocols, options); + return Mocks.Connect(std::move(url), protocols, options); } void MockWebSocketResource::Ping() noexcept /*override*/ diff --git a/vnext/Desktop.UnitTests/WebSocketMocks.h b/vnext/Desktop.UnitTests/WebSocketMocks.h index 962af1eda72..70b972c374a 100644 --- a/vnext/Desktop.UnitTests/WebSocketMocks.h +++ b/vnext/Desktop.UnitTests/WebSocketMocks.h @@ -7,7 +7,7 @@ struct MockWebSocketResource : public IWebSocketResource { ~MockWebSocketResource(); struct Mocks { - std::function Connect; + std::function Connect; std::function Ping; std::function Send; std::function SendBinary; @@ -25,7 +25,7 @@ struct MockWebSocketResource : public IWebSocketResource { #pragma region IWebSocketResource overrides - void Connect(const Protocols &, const Options &) noexcept override; + void Connect(std::string &&url, const Protocols &, const Options &) noexcept override; void Ping() noexcept override; diff --git a/vnext/Desktop.UnitTests/WebSocketModuleTest.cpp b/vnext/Desktop.UnitTests/WebSocketModuleTest.cpp index 076f707b056..dd7b423efb0 100644 --- a/vnext/Desktop.UnitTests/WebSocketModuleTest.cpp +++ b/vnext/Desktop.UnitTests/WebSocketModuleTest.cpp @@ -74,7 +74,7 @@ TEST_CLASS (WebSocketModuleTest) { module->setInstance(instance); module->SetResourceFactory([](const string &) { auto rc = make_shared(); - rc->Mocks.Connect = [rc](const IWebSocketResource::Protocols &, const IWebSocketResource::Options &) { + rc->Mocks.Connect = [rc](string &&, const IWebSocketResource::Protocols &, const IWebSocketResource::Options &) { rc->OnConnect(); }; diff --git a/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp b/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp index 71dcb31cdc5..f13ba0964ca 100644 --- a/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp +++ b/vnext/Desktop.UnitTests/WinRTWebSocketResourceUnitTest.cpp @@ -32,6 +32,8 @@ IAsyncAction ThrowAsync() { co_return; } +constexpr char testUrl[] = "ws://host:0"; + } // namespace namespace Microsoft::React::Test { @@ -51,12 +53,11 @@ TEST_CLASS (WinRTWebSocketResourceUnitTest) { mws->Mocks.Close = [](uint16_t, const hstring &) {}; // Test APIs - auto rc = - make_shared(std::move(imws), MockDataWriter{}, Uri{L"ws://host:0"}, CertExceptions{}); + auto rc = make_shared(std::move(imws), MockDataWriter{}, CertExceptions{}); rc->SetOnConnect([&connected]() { connected = true; }); rc->SetOnError([&errorMessage](Error &&error) { errorMessage = error.Message; }); - rc->Connect({}, {}); + rc->Connect(testUrl, {}, {}); rc->Close(CloseCode::Normal, {}); Assert::AreEqual({}, errorMessage); @@ -77,12 +78,11 @@ TEST_CLASS (WinRTWebSocketResourceUnitTest) { mws->Mocks.Close = [](uint16_t, const hstring &) {}; // Test APIs - auto rc = - make_shared(std::move(imws), MockDataWriter{}, Uri{L"ws://host:0"}, CertExceptions{}); + auto rc = make_shared(std::move(imws), MockDataWriter{}, CertExceptions{}); rc->SetOnConnect([&connected]() { connected = true; }); rc->SetOnError([&errorMessage](Error &&error) { errorMessage = error.Message; }); - rc->Connect({}, {}); + rc->Connect(testUrl, {}, {}); rc->Close(CloseCode::Normal, {}); Assert::AreEqual({"[0x80004005] Expected Failure"}, errorMessage); @@ -95,7 +95,7 @@ TEST_CLASS (WinRTWebSocketResourceUnitTest) { auto lambda = [&rc]() mutable { rc = make_shared( - winrt::make(), MockDataWriter{}, Uri{L"ws://host:0"}, CertExceptions{}); + winrt::make(), MockDataWriter{}, CertExceptions{}); }; Assert::ExpectException(lambda); diff --git a/vnext/Desktop/BeastWebSocketResource.cpp b/vnext/Desktop/BeastWebSocketResource.cpp index 4eb476400e8..2951bd61562 100644 --- a/vnext/Desktop/BeastWebSocketResource.cpp +++ b/vnext/Desktop/BeastWebSocketResource.cpp @@ -30,6 +30,90 @@ using std::unique_ptr; namespace Microsoft::React { +#pragma region BeastWebSocketResource + +BeastWebSocketResource::BeastWebSocketResource() noexcept {} + +#pragma region IWebSocketResource members + +void BeastWebSocketResource::Connect( + std::string &&urlString, + const Protocols &protocols, + const Options &options) noexcept { + Url url(std::move(urlString)); + + if (url.scheme == "ws") { + if (url.port.empty()) + url.port = "80"; + + m_concreteResource = std::make_shared(std::move(url)); + } else if (url.scheme == "wss") { + if (url.port.empty()) + url.port = "443"; + + m_concreteResource = std::make_shared(std::move(url)); + } + + m_concreteResource->SetOnClose(std::move(m_closeHandler)); + m_concreteResource->SetOnConnect(std::move(m_connectHandler)); + m_concreteResource->SetOnError(std::move(m_errorHandler)); + m_concreteResource->SetOnMessage(std::move(m_readHandler)); + m_concreteResource->SetOnPing(std::move(m_pingHandler)); + m_concreteResource->SetOnSend(std::move(m_writeHandler)); +} + +void BeastWebSocketResource::Close(CloseCode code, const string &reason) noexcept { + m_concreteResource->Close(code, reason); +} + +void BeastWebSocketResource::Send(string &&message) noexcept { + m_concreteResource->Send(std::move(message)); +} + +void BeastWebSocketResource::SendBinary(string &&base64String) noexcept { + m_concreteResource->SendBinary(std::move(base64String)); +} + +void BeastWebSocketResource::Ping() noexcept { + m_concreteResource->Ping(); +} + +#pragma endregion IWebSocketResource members + +#pragma region Handler setters + +void BeastWebSocketResource::SetOnConnect(function &&handler) noexcept { + m_connectHandler = std::move(handler); +} + +void BeastWebSocketResource::SetOnPing(function &&handler) noexcept { + m_pingHandler = std::move(handler); +} + +void BeastWebSocketResource::SetOnSend(function &&handler) noexcept { + m_writeHandler = std::move(handler); +} + +void BeastWebSocketResource::SetOnMessage(function &&handler) noexcept { + m_readHandler = std::move(handler); +} + +void BeastWebSocketResource::SetOnClose(function &&handler) noexcept { + m_closeHandler = std::move(handler); +} + +void BeastWebSocketResource::SetOnError(function &&handler) noexcept { + m_errorHandler = std::move(handler); +} + +IWebSocketResource::ReadyState BeastWebSocketResource::GetReadyState() const noexcept { + return m_concreteResource->GetReadyState(); +} + +#pragma endregion Handler setters + +#pragma endregion BeastWebSocketResource + namespace Beast { #pragma region BaseWebSocketResource members @@ -314,13 +398,16 @@ websocket::close_code BaseWebSocketResource::ToBeastCloseCo #pragma region IWebSocketResource members template -void BaseWebSocketResource::Connect(const Protocols &protocols, const Options &options) noexcept { +void BaseWebSocketResource::Connect( + std::string &&url, + const Protocols &protocols, + const Options &options) noexcept { // "Cannot call Connect more than once"); assert(ReadyState::Connecting == m_readyState); // TODO: Enable? // m_stream->set_option(websocket::stream_base::timeout::suggested(role_type::client)); - m_stream->set_option(websocket::stream_base::decorator([options = std::move(options)](websocket::request_type &req) { + m_stream->set_option(websocket::stream_base::decorator([options = options](websocket::request_type &req) { // Collect headers for (const auto &header : options) { req.insert(Microsoft::Common::Unicode::Utf16ToUtf8(header.first), header.second); @@ -433,34 +520,34 @@ void BaseWebSocketResource::Ping() noexcept { template void BaseWebSocketResource::SetOnConnect(function &&handler) noexcept { - m_connectHandler = handler; + m_connectHandler = std::move(handler); } template void BaseWebSocketResource::SetOnPing(function &&handler) noexcept { - m_pingHandler = handler; + m_pingHandler = std::move(handler); } template void BaseWebSocketResource::SetOnSend(function &&handler) noexcept { - m_writeHandler = handler; + m_writeHandler = std::move(handler); } template void BaseWebSocketResource::SetOnMessage( function &&handler) noexcept { - m_readHandler = handler; + m_readHandler = std::move(handler); } template void BaseWebSocketResource::SetOnClose( function &&handler) noexcept { - m_closeHandler = handler; + m_closeHandler = std::move(handler); } template void BaseWebSocketResource::SetOnError(function &&handler) noexcept { - m_errorHandler = handler; + m_errorHandler = std::move(handler); } template diff --git a/vnext/Desktop/BeastWebSocketResource.h b/vnext/Desktop/BeastWebSocketResource.h index a471c1ec6b2..aba52553e21 100644 --- a/vnext/Desktop/BeastWebSocketResource.h +++ b/vnext/Desktop/BeastWebSocketResource.h @@ -13,7 +13,9 @@ #include "IWebSocketResource.h" #include "Utils.h" -namespace Microsoft::React::Beast { +namespace Microsoft::React { + +namespace Beast { template < typename SocketLayer = boost::beast::tcp_stream, @@ -156,7 +158,7 @@ class BaseWebSocketResource : public IWebSocketResource { /// /// /// - void Connect(const Protocols &protocols, const Options &options) noexcept override; + void Connect(std::string &&url, const Protocols &protocols, const Options &options) noexcept override; /// /// @@ -341,4 +343,81 @@ class TestWebSocketResource : public BaseWebSocketResource< } // namespace Test -} // namespace Microsoft::React::Beast +} // namespace Beast + +class BeastWebSocketResource : public IWebSocketResource, public std::enable_shared_from_this { + std::function m_connectHandler; + std::function m_pingHandler; + std::function m_writeHandler; + std::function m_readHandler; + std::function m_closeHandler; + std::function m_errorHandler; + + std::shared_ptr m_concreteResource; + + public: + BeastWebSocketResource() noexcept; + +#pragma region IWebSocketResource + + /// + /// + /// + void Connect(std::string &&url, const Protocols &protocols, const Options &options) noexcept override; + + /// + /// + /// + void Ping() noexcept override; + + /// + /// + /// + void Send(std::string &&message) noexcept override; + + /// + /// + /// + void SendBinary(std::string &&base64String) noexcept override; + + /// + /// + /// + void Close(CloseCode code, const std::string &reason) noexcept override; + + ReadyState GetReadyState() const noexcept override; + + /// + /// + /// + void SetOnConnect(std::function &&handler) noexcept override; + + /// + /// + /// + void SetOnPing(std::function &&handler) noexcept override; + + /// + /// + /// + void SetOnSend(std::function &&handler) noexcept override; + + /// + /// + /// + void SetOnMessage(std::function &&handler) noexcept override; + + /// + /// + /// + void SetOnClose(std::function &&handler) noexcept override; + + /// + /// + /// + void SetOnError(std::function &&handler) noexcept override; + +#pragma endregion IWebSocketResource +}; + +} // namespace Microsoft::React diff --git a/vnext/Desktop/WebSocketResourceFactory.cpp b/vnext/Desktop/WebSocketResourceFactory.cpp index a9e782f20cf..3a89a0b6f94 100644 --- a/vnext/Desktop/WebSocketResourceFactory.cpp +++ b/vnext/Desktop/WebSocketResourceFactory.cpp @@ -7,7 +7,6 @@ #include #include "BeastWebSocketResource.h" -using std::make_shared; using std::shared_ptr; using std::string; @@ -15,24 +14,10 @@ namespace Microsoft::React { #pragma region IWebSocketResource static members /*static*/ -shared_ptr IWebSocketResource::Make(string &&urlString) { +shared_ptr IWebSocketResource::Make() { #if ENABLE_BEAST if (GetRuntimeOptionBool("UseBeastWebSocket")) { - Url url(std::move(urlString)); - - if (url.scheme == "ws") { - if (url.port.empty()) - url.port = "80"; - - return make_shared(std::move(url)); - } else if (url.scheme == "wss") { - if (url.port.empty()) - url.port = "443"; - - return make_shared(std::move(url)); - } else { - throw std::invalid_argument((string("Incorrect URL scheme: ") + url.scheme).c_str()); - } + return std::make_shared(); } else { #endif // ENABLE_BEAST std::vector certExceptions; @@ -42,7 +27,7 @@ shared_ptr IWebSocketResource::Make(string &&urlString) { certExceptions.emplace_back( winrt::Windows::Security::Cryptography::Certificates::ChainValidationResult::InvalidName); } - return make_shared(std::move(urlString), std::move(certExceptions)); + return std::make_shared(std::move(certExceptions)); #if ENABLE_BEAST } #endif // ENABLE_BEAST diff --git a/vnext/Microsoft.ReactNative/Modules/CreateModules.cpp b/vnext/Microsoft.ReactNative/Modules/CreateModules.cpp index f5d0b565247..8b2a22e9b14 100644 --- a/vnext/Microsoft.ReactNative/Modules/CreateModules.cpp +++ b/vnext/Microsoft.ReactNative/Modules/CreateModules.cpp @@ -19,9 +19,9 @@ using winrt::Microsoft::ReactNative::implementation::QuirkSettings; namespace Microsoft::React { -std::shared_ptr IWebSocketResource::Make(std::string &&urlString) { +std::shared_ptr IWebSocketResource::Make() { std::vector certExceptions; - return std::make_shared(std::move(urlString), std::move(certExceptions)); + return std::make_shared(std::move(certExceptions)); } } // namespace Microsoft::React diff --git a/vnext/Shared/IWebSocketResource.h b/vnext/Shared/IWebSocketResource.h index 75a3f10b72d..6c27c72755f 100644 --- a/vnext/Shared/IWebSocketResource.h +++ b/vnext/Shared/IWebSocketResource.h @@ -79,17 +79,17 @@ struct IWebSocketResource { /// /// Creates an IWebSocketResource instance. /// - /// - /// WebSocket URL address the instance will connect to. - /// The address's scheme can be either ws:// or wss://. - /// - static std::shared_ptr Make(std::string &&url); + static std::shared_ptr Make(); virtual ~IWebSocketResource() noexcept {} /// /// Establishes a continuous connection with the remote endpoint. /// + /// + /// WebSocket URL address the instance will connect to. + /// The address's scheme can be either ws:// or wss://. + /// /// /// Currently unused /// @@ -97,7 +97,7 @@ struct IWebSocketResource { /// HTTP header fields passed by the remote endpoint, to be used in the /// handshake process. /// - virtual void Connect(const Protocols &protocols = {}, const Options &options = {}) noexcept = 0; + virtual void Connect(std::string &&url, const Protocols &protocols = {}, const Options &options = {}) noexcept = 0; /// /// Sends a ping frame to the remote endpoint. diff --git a/vnext/Shared/InspectorPackagerConnection.cpp b/vnext/Shared/InspectorPackagerConnection.cpp index 84d91003776..481067b0dc7 100644 --- a/vnext/Shared/InspectorPackagerConnection.cpp +++ b/vnext/Shared/InspectorPackagerConnection.cpp @@ -204,8 +204,7 @@ winrt::fire_and_forget InspectorPackagerConnection::connectAsync() { co_await winrt::resume_background(); std::vector certExceptions; - m_packagerWebSocketConnection = - std::make_shared(m_url, std::move(certExceptions)); + m_packagerWebSocketConnection = std::make_shared(std::move(certExceptions)); m_packagerWebSocketConnection->SetOnError([](const Microsoft::React::IWebSocketResource::Error &err) { facebook::react::tracing::error(err.Message.c_str()); @@ -270,7 +269,7 @@ winrt::fire_and_forget InspectorPackagerConnection::connectAsync() { Microsoft::React::IWebSocketResource::Protocols protocols; Microsoft::React::IWebSocketResource::Options options; - m_packagerWebSocketConnection->Connect(protocols, options); + m_packagerWebSocketConnection->Connect(std::string{m_url}, protocols, options); co_return; } diff --git a/vnext/Shared/Modules/WebSocketModule.cpp b/vnext/Shared/Modules/WebSocketModule.cpp index e8ae6adf704..3e70ebe8c67 100644 --- a/vnext/Shared/Modules/WebSocketModule.cpp +++ b/vnext/Shared/Modules/WebSocketModule.cpp @@ -14,7 +14,9 @@ #include using namespace facebook::xplat; -using namespace folly; + +using facebook::react::Instance; +using folly::dynamic; using Microsoft::Common::Unicode::Utf16ToUtf8; using Microsoft::Common::Unicode::Utf8ToUtf16; @@ -24,17 +26,111 @@ using std::string; using std::weak_ptr; namespace { +using Microsoft::React::IWebSocketResource; +using Microsoft::React::WebSocketModule; + constexpr char moduleName[] = "WebSocketModule"; + +static void SendEvent(weak_ptr weakInstance, string &&eventName, dynamic &&args) { + if (auto instance = weakInstance.lock()) { + instance->callJSFunction("RCTDeviceEventEmitter", "emit", dynamic::array(std::move(eventName), std::move(args))); + } +} + +static shared_ptr +GetOrCreateWebSocket(int64_t id, string &&url, weak_ptr weakState) { + auto state = weakState.lock(); + if (!state) { + return nullptr; + } + + auto itr = state->ResourceMap.find(id); + if (itr == state->ResourceMap.end()) { + if (!state->Module) { + return nullptr; + } + auto weakInstance = state->Module->getInstance(); + + shared_ptr ws; + try { + ws = state->ResourceFactory(std::move(url)); + } catch (const winrt::hresult_error &e) { + std::stringstream ss; + ss << "[" << std::hex << std::showbase << std::setw(8) << static_cast(e.code()) << "] " + << winrt::to_string(e.message()); + + SendEvent(weakInstance, "webSocketFailed", dynamic::object("id", id)("message", std::move(ss.str()))); + + return nullptr; + } catch (const std::exception &e) { + SendEvent(weakInstance, "webSocketFailed", dynamic::object("id", id)("message", e.what())); + + return nullptr; + } catch (...) { + SendEvent( + weakInstance, + "webSocketFailed", + dynamic::object("id", id)("message", "Unidentified error creating IWebSocketResource")); + + return nullptr; + } + + ws->SetOnError([id, weakInstance](const IWebSocketResource::Error &err) { + auto strongInstance = weakInstance.lock(); + if (!strongInstance) + return; + + auto errorObj = dynamic::object("id", id)("message", err.Message); + SendEvent(weakInstance, "websocketFailed", std::move(errorObj)); + }); + ws->SetOnConnect([id, weakInstance]() { + auto strongInstance = weakInstance.lock(); + if (!strongInstance) + return; + + auto args = dynamic::object("id", id); + SendEvent(weakInstance, "websocketOpen", std::move(args)); + }); + ws->SetOnMessage([id, weakInstance](size_t length, const string &message, bool isBinary) { + auto strongInstance = weakInstance.lock(); + if (!strongInstance) + return; + + auto args = dynamic::object("id", id)("data", message)("type", isBinary ? "binary" : "text"); + SendEvent(weakInstance, "websocketMessage", std::move(args)); + }); + ws->SetOnClose([id, weakInstance](IWebSocketResource::CloseCode code, const string &reason) { + auto strongInstance = weakInstance.lock(); + if (!strongInstance) + return; + + auto args = dynamic::object("id", id)("code", static_cast(code))("reason", reason); + SendEvent(weakInstance, "websocketClosed", std::move(args)); + }); + + state->ResourceMap.emplace(id, ws); + return ws; + } + + return itr->second; +} + } // anonymous namespace namespace Microsoft::React { -WebSocketModule::WebSocketModule() - : m_resourceFactory{[](string &&url) { return IWebSocketResource::Make(std::move(url)); }} {} +WebSocketModule::WebSocketModule() : m_sharedState{std::make_shared()} { + m_sharedState->ResourceFactory = [](string &&url) { return IWebSocketResource::Make(); }; + m_sharedState->Module = this; +} + +WebSocketModule::~WebSocketModule() noexcept /*override*/ { + m_sharedState->Module = nullptr; +} void WebSocketModule::SetResourceFactory( std::function(const string &)> &&resourceFactory) { - m_resourceFactory = std::move(resourceFactory); + m_sharedState->ResourceFactory = std::move(resourceFactory); } string WebSocketModule::getName() { @@ -50,9 +146,9 @@ std::vector WebSocketModule::getMeth { return { - Method( + { "connect", - [this](dynamic args) // const string& url, dynamic protocols, dynamic options, int64_t id + [weakState = weak_ptr(m_sharedState)](dynamic args) // const string& url, dynamic protocols, dynamic options, int64_t id { IWebSocketResource::Protocols protocols; dynamic protocolsDynamic = jsArgAsDynamic(args, 1); @@ -75,20 +171,21 @@ std::vector WebSocketModule::getMeth } } - weak_ptr weakWs = this->GetOrCreateWebSocket(jsArgAsInt(args, 3), jsArgAsString(args, 0)); + weak_ptr weakWs = GetOrCreateWebSocket(jsArgAsInt(args, 3), jsArgAsString(args, 0), weakState); if (auto sharedWs = weakWs.lock()) { - sharedWs->Connect(protocols, options); + sharedWs->Connect(jsArgAsString(args, 0), protocols, options); } - }), - Method( + } + }, + { "close", - [this](dynamic args) // [int64_t code, string reason,] int64_t id + [weakState = weak_ptr(m_sharedState)](dynamic args) // [int64_t code, string reason,] int64_t id { // See react-native\Libraries\WebSocket\WebSocket.js:_close if (args.size() == 3) // WebSocketModule.close(statusCode, closeReason, this._socketId); { - weak_ptr weakWs = this->GetOrCreateWebSocket(jsArgAsInt(args, 2)); + weak_ptr weakWs = GetOrCreateWebSocket(jsArgAsInt(args, 2), {}, weakState); if (auto sharedWs = weakWs.lock()) { sharedWs->Close(static_cast(jsArgAsInt(args, 0)), jsArgAsString(args, 1)); @@ -96,7 +193,7 @@ std::vector WebSocketModule::getMeth } else if (args.size() == 1) // WebSocketModule.close(this._socketId); { - weak_ptr weakWs = this->GetOrCreateWebSocket(jsArgAsInt(args, 0)); + weak_ptr weakWs = GetOrCreateWebSocket(jsArgAsInt(args, 0), {}, weakState); if (auto sharedWs = weakWs.lock()) { sharedWs->Close(IWebSocketResource::CloseCode::Normal, {}); @@ -104,137 +201,53 @@ std::vector WebSocketModule::getMeth } else { - auto errorObj = dynamic::object("id", -1)("message", "Incorrect number of parameters"); - this->SendEvent("websocketFailed", std::move(errorObj)); + auto state = weakState.lock(); + if (state && state->Module) { + auto errorObj = dynamic::object("id", -1)("message", "Incorrect number of parameters"); + SendEvent(state->Module->getInstance(), "websocketFailed", std::move(errorObj)); + } } - }), - Method( + } + }, + { "send", - [this](dynamic args) // const string& message, int64_t id + [weakState = weak_ptr(m_sharedState)](dynamic args) // const string& message, int64_t id { - weak_ptr weakWs = this->GetOrCreateWebSocket(jsArgAsInt(args, 1)); + weak_ptr weakWs = GetOrCreateWebSocket(jsArgAsInt(args, 1), {}, weakState); if (auto sharedWs = weakWs.lock()) { sharedWs->Send(jsArgAsString(args, 0)); } - }), - Method( + } + }, + { "sendBinary", - [this](dynamic args) // const string& base64String, int64_t id + [weakState = weak_ptr(m_sharedState)](dynamic args) // const string& base64String, int64_t id { - weak_ptr weakWs = this->GetOrCreateWebSocket(jsArgAsInt(args, 1)); + weak_ptr weakWs = GetOrCreateWebSocket(jsArgAsInt(args, 1), {}, weakState); if (auto sharedWs = weakWs.lock()) { sharedWs->SendBinary(jsArgAsString(args, 0)); } - }), - Method( + } + }, + { "ping", - [this](dynamic args) // int64_t id + [weakState = weak_ptr(m_sharedState)](dynamic args) // int64_t id { - weak_ptr weakWs = this->GetOrCreateWebSocket(jsArgAsInt(args, 0)); + weak_ptr weakWs = GetOrCreateWebSocket(jsArgAsInt(args, 0), {}, weakState); if (auto sharedWs = weakWs.lock()) { sharedWs->Ping(); } - }) + } + } }; } // getMethods // clang-format on -#pragma region private members - -void WebSocketModule::SendEvent(string &&eventName, dynamic &&args) { - auto weakInstance = this->getInstance(); - if (auto instance = weakInstance.lock()) { - instance->callJSFunction("RCTDeviceEventEmitter", "emit", dynamic::array(std::move(eventName), std::move(args))); - } -} - -// clang-format off -shared_ptr WebSocketModule::GetOrCreateWebSocket(int64_t id, string&& url) -{ - auto itr = m_webSockets.find(id); - if (itr == m_webSockets.end()) - { - shared_ptr ws; - try - { - ws = m_resourceFactory(std::move(url)); - } - catch (const winrt::hresult_error& e) - { - std::stringstream ss; - ss << "[" << std::hex << std::showbase << std::setw(8) << static_cast(e.code()) << "] " << - winrt::to_string(e.message()); - - SendEvent("webSocketFailed", dynamic::object("id", id)("message", std::move(ss.str()))); - - return nullptr; - } - catch (const std::exception& e) - { - SendEvent("webSocketFailed", dynamic::object("id", id)("message", e.what())); - - return nullptr; - } - catch (...) - { - SendEvent("webSocketFailed", dynamic::object("id", id)("message", "Unidentified error creating IWebSocketResource")); - - return nullptr; - } - - auto weakInstance = this->getInstance(); - ws->SetOnError([this, id, weakInstance](const IWebSocketResource::Error& err) - { - auto strongInstance = weakInstance.lock(); - if (!strongInstance) - return; - - auto errorObj = dynamic::object("id", id)("message", err.Message); - this->SendEvent("websocketFailed", std::move(errorObj)); - }); - ws->SetOnConnect([this, id, weakInstance]() - { - auto strongInstance = weakInstance.lock(); - if (!strongInstance) - return; - - auto args = dynamic::object("id", id); - this->SendEvent("websocketOpen", std::move(args)); - }); - ws->SetOnMessage([this, id, weakInstance](size_t length, const string& message, bool isBinary) - { - auto strongInstance = weakInstance.lock(); - if (!strongInstance) - return; - - auto args = dynamic::object("id", id)("data", message)("type", isBinary ? "binary" : "text"); - this->SendEvent("websocketMessage", std::move(args)); - }); - ws->SetOnClose([this, id, weakInstance](IWebSocketResource::CloseCode code, const string& reason) - { - auto strongInstance = weakInstance.lock(); - if (!strongInstance) - return; - - auto args = dynamic::object("id", id)("code", static_cast(code))("reason", reason); - this->SendEvent("websocketClosed", std::move(args)); - }); - - m_webSockets.emplace(id, ws); - return ws; - } - - return itr->second; -} -// clang-format on - -#pragma endregion private members - /*extern*/ std::unique_ptr CreateWebSocketModule() noexcept { - return make_unique(); + return std::make_unique(); } } // namespace Microsoft::React diff --git a/vnext/Shared/Modules/WebSocketModule.h b/vnext/Shared/Modules/WebSocketModule.h index 25fa33493b7..e9ecc504d50 100644 --- a/vnext/Shared/Modules/WebSocketModule.h +++ b/vnext/Shared/Modules/WebSocketModule.h @@ -18,6 +18,26 @@ class WebSocketModule : public facebook::xplat::module::CxxModule { WebSocketModule(); + ~WebSocketModule() noexcept override; + + struct SharedState { + /// + /// Keeps IWebSocketResource instances identified by id. + /// As defined in WebSocket.js. + /// + std::map> ResourceMap{}; + + /// + /// Generates IWebSocketResource instances, defaulting to IWebSocketResource::Make. + /// + std::function(std::string &&)> ResourceFactory; + + /// + /// Keeps a raw reference to the module object to lazily retrieve the React Instance as needed. + /// + CxxModule *Module{nullptr}; + }; + #pragma region CxxModule overrides /// @@ -41,16 +61,6 @@ class WebSocketModule : public facebook::xplat::module::CxxModule { void SetResourceFactory(std::function(const std::string &)> &&resourceFactory); private: - /// - /// Notifies an event to the current React Instance. - /// - void SendEvent(std::string &&eventName, folly::dynamic &¶meters); - - /// - /// Creates or retrieves a raw IWebSocketResource pointer. - /// - std::shared_ptr GetOrCreateWebSocket(std::int64_t id, std::string &&url = {}); - /// /// Keeps IWebSocketResource instances identified by id. /// As defined in WebSocket.js. @@ -58,9 +68,9 @@ class WebSocketModule : public facebook::xplat::module::CxxModule { std::map> m_webSockets; /// - /// Generates IWebSocketResource instances, defaulting to IWebSocketResource::Make. + /// Keeps members that can be accessed threads other than this module's owner accessible. /// - std::function(std::string &&)> m_resourceFactory; + std::shared_ptr m_sharedState; }; } // namespace Microsoft::React diff --git a/vnext/Shared/WinRTWebSocketResource.cpp b/vnext/Shared/WinRTWebSocketResource.cpp index 8d98c9e8213..84d173d504d 100644 --- a/vnext/Shared/WinRTWebSocketResource.cpp +++ b/vnext/Shared/WinRTWebSocketResource.cpp @@ -90,20 +90,14 @@ namespace Microsoft::React { // private WinRTWebSocketResource::WinRTWebSocketResource( IMessageWebSocket &&socket, - Uri &&uri, vector &&certExceptions) - : WinRTWebSocketResource( - std::move(socket), - DataWriter{socket.OutputStream()}, - std::move(uri), - std::move(certExceptions)) {} + : WinRTWebSocketResource(std::move(socket), DataWriter{socket.OutputStream()}, std::move(certExceptions)) {} WinRTWebSocketResource::WinRTWebSocketResource( IMessageWebSocket &&socket, IDataWriter &&writer, - Uri &&uri, vector &&certExceptions) - : m_uri{std::move(uri)}, m_socket{std::move(socket)}, m_writer{std::move(writer)} { + : m_socket{std::move(socket)}, m_writer{std::move(writer)} { m_socket.MessageReceived({this, &WinRTWebSocketResource::OnMessageReceived}); for (const auto &certException : certExceptions) { @@ -111,8 +105,8 @@ WinRTWebSocketResource::WinRTWebSocketResource( } } -WinRTWebSocketResource::WinRTWebSocketResource(const string &urlString, vector &&certExceptions) - : WinRTWebSocketResource(MessageWebSocket{}, Uri{winrt::to_hstring(urlString)}, std::move(certExceptions)) {} +WinRTWebSocketResource::WinRTWebSocketResource(vector &&certExceptions) + : WinRTWebSocketResource(MessageWebSocket{}, std::move(certExceptions)) {} WinRTWebSocketResource::~WinRTWebSocketResource() noexcept /*override*/ { @@ -123,13 +117,14 @@ WinRTWebSocketResource::~WinRTWebSocketResource() noexcept /*override*/ #pragma region Private members -IAsyncAction WinRTWebSocketResource::PerformConnect() noexcept { +IAsyncAction WinRTWebSocketResource::PerformConnect(Uri &&uri) noexcept { auto self = shared_from_this(); + auto coUri = std::move(uri); co_await resume_background(); try { - auto async = self->m_socket.ConnectAsync(self->m_uri); + auto async = self->m_socket.ConnectAsync(coUri); co_await lessthrow_await_adapter{async}; @@ -334,7 +329,7 @@ void WinRTWebSocketResource::Synchronize() noexcept { #pragma region IWebSocketResource -void WinRTWebSocketResource::Connect(const Protocols &protocols, const Options &options) noexcept { +void WinRTWebSocketResource::Connect(string &&url, const Protocols &protocols, const Options &options) noexcept { m_readyState = ReadyState::Connecting; for (const auto &header : options) { @@ -348,7 +343,24 @@ void WinRTWebSocketResource::Connect(const Protocols &protocols, const Options & } m_connectRequested = true; - PerformConnect(); + + Uri uri{nullptr}; + try { + uri = Uri{winrt::to_hstring(url)}; + } catch (hresult_error const &e) { + if (m_errorHandler) { + m_errorHandler({HResultToString(e), ErrorType::Connection}); + } + + // Abort - Mark connection as concluded. + SetEvent(m_connectPerformed.get()); + m_connectPerformedPromise.set_value(); + m_connectRequested = false; + + return; + } + + PerformConnect(std::move(uri)); } void WinRTWebSocketResource::Ping() noexcept { diff --git a/vnext/Shared/WinRTWebSocketResource.h b/vnext/Shared/WinRTWebSocketResource.h index 777d78de4fc..f75ae809614 100644 --- a/vnext/Shared/WinRTWebSocketResource.h +++ b/vnext/Shared/WinRTWebSocketResource.h @@ -17,7 +17,6 @@ namespace Microsoft::React { class WinRTWebSocketResource : public IWebSocketResource, public std::enable_shared_from_this { - winrt::Windows::Foundation::Uri m_uri; winrt::Windows::Networking::Sockets::IMessageWebSocket m_socket; // TODO: Use or remove. winrt::Windows::Networking::Sockets::IMessageWebSocket::MessageReceived_revoker m_revoker; @@ -45,10 +44,9 @@ class WinRTWebSocketResource : public IWebSocketResource, public std::enable_sha WinRTWebSocketResource( winrt::Windows::Networking::Sockets::IMessageWebSocket &&socket, - winrt::Windows::Foundation::Uri &&uri, std::vector &&certExceptions); - winrt::Windows::Foundation::IAsyncAction PerformConnect() noexcept; + winrt::Windows::Foundation::IAsyncAction PerformConnect(winrt::Windows::Foundation::Uri &&uri) noexcept; winrt::fire_and_forget PerformPing() noexcept; winrt::fire_and_forget PerformWrite(std::string &&message, bool isBinary) noexcept; winrt::fire_and_forget PerformClose() noexcept; @@ -62,11 +60,9 @@ class WinRTWebSocketResource : public IWebSocketResource, public std::enable_sha WinRTWebSocketResource( winrt::Windows::Networking::Sockets::IMessageWebSocket &&socket, winrt::Windows::Storage::Streams::IDataWriter &&writer, - winrt::Windows::Foundation::Uri &&uri, std::vector &&certExceptions); WinRTWebSocketResource( - const std::string &urlString, std::vector &&certExceptions); ~WinRTWebSocketResource() noexcept override; @@ -76,7 +72,7 @@ class WinRTWebSocketResource : public IWebSocketResource, public std::enable_sha /// /// /// - void Connect(const Protocols &protocols, const Options &options) noexcept override; + void Connect(std::string &&url, const Protocols &protocols, const Options &options) noexcept override; /// /// From 14bbdbea94c773dcfabc57eb172359e0b0a80f78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julio=20C=C3=A9sar=20Rocha?= Date: Fri, 17 Dec 2021 11:56:16 -0800 Subject: [PATCH 2/4] Use shared pointer in WebSocket MessageReceived (#9293) * Set MessageReceived to self-capturing lambda * clang format * Change files --- ...-857f8e44-c7be-4025-827d-902572d0c205.json | 7 +++ vnext/Shared/WinRTWebSocketResource.cpp | 61 +++++++++---------- vnext/Shared/WinRTWebSocketResource.h | 3 - 3 files changed, 36 insertions(+), 35 deletions(-) create mode 100644 change/react-native-windows-857f8e44-c7be-4025-827d-902572d0c205.json diff --git a/change/react-native-windows-857f8e44-c7be-4025-827d-902572d0c205.json b/change/react-native-windows-857f8e44-c7be-4025-827d-902572d0c205.json new file mode 100644 index 00000000000..caa65d7d036 --- /dev/null +++ b/change/react-native-windows-857f8e44-c7be-4025-827d-902572d0c205.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "Use shared pointer in MessageReceived", + "packageName": "react-native-windows", + "email": "julio.rocha@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/vnext/Shared/WinRTWebSocketResource.cpp b/vnext/Shared/WinRTWebSocketResource.cpp index 84d173d504d..7985e01ed00 100644 --- a/vnext/Shared/WinRTWebSocketResource.cpp +++ b/vnext/Shared/WinRTWebSocketResource.cpp @@ -98,8 +98,6 @@ WinRTWebSocketResource::WinRTWebSocketResource( IDataWriter &&writer, vector &&certExceptions) : m_socket{std::move(socket)}, m_writer{std::move(writer)} { - m_socket.MessageReceived({this, &WinRTWebSocketResource::OnMessageReceived}); - for (const auto &certException : certExceptions) { m_socket.Control().IgnorableServerCertificateErrors().Append(certException); } @@ -288,36 +286,6 @@ fire_and_forget WinRTWebSocketResource::PerformClose() noexcept { m_closePerformed.Set(); } -void WinRTWebSocketResource::OnMessageReceived( - IWebSocket const &sender, - IMessageWebSocketMessageReceivedEventArgs const &args) { - try { - string response; - IDataReader reader = args.GetDataReader(); - auto len = reader.UnconsumedBufferLength(); - if (args.MessageType() == SocketMessageType::Utf8) { - reader.UnicodeEncoding(UnicodeEncoding::Utf8); - vector data(len); - reader.ReadBytes(data); - - response = string(CheckedReinterpretCast(data.data()), data.size()); - } else { - auto buffer = reader.ReadBuffer(len); - winrt::hstring data = CryptographicBuffer::EncodeToBase64String(buffer); - - response = winrt::to_string(std::wstring_view(data)); - } - - if (m_readHandler) { - m_readHandler(response.length(), response, args.MessageType() == SocketMessageType::Binary); - } - } catch (hresult_error const &e) { - if (m_errorHandler) { - m_errorHandler({HResultToString(e), ErrorType::Receive}); - } - } -} - void WinRTWebSocketResource::Synchronize() noexcept { // Ensure sequence of other operations if (m_connectRequested) { @@ -330,6 +298,35 @@ void WinRTWebSocketResource::Synchronize() noexcept { #pragma region IWebSocketResource void WinRTWebSocketResource::Connect(string &&url, const Protocols &protocols, const Options &options) noexcept { + m_socket.MessageReceived( + [self = shared_from_this()](IWebSocket const &sender, IMessageWebSocketMessageReceivedEventArgs const &args) { + try { + string response; + IDataReader reader = args.GetDataReader(); + auto len = reader.UnconsumedBufferLength(); + if (args.MessageType() == SocketMessageType::Utf8) { + reader.UnicodeEncoding(UnicodeEncoding::Utf8); + vector data(len); + reader.ReadBytes(data); + + response = string(CheckedReinterpretCast(data.data()), data.size()); + } else { + auto buffer = reader.ReadBuffer(len); + winrt::hstring data = CryptographicBuffer::EncodeToBase64String(buffer); + + response = winrt::to_string(std::wstring_view(data)); + } + + if (self->m_readHandler) { + self->m_readHandler(response.length(), response, args.MessageType() == SocketMessageType::Binary); + } + } catch (hresult_error const &e) { + if (self->m_errorHandler) { + self->m_errorHandler({HResultToString(e), ErrorType::Receive}); + } + } + }); + m_readyState = ReadyState::Connecting; for (const auto &header : options) { diff --git a/vnext/Shared/WinRTWebSocketResource.h b/vnext/Shared/WinRTWebSocketResource.h index f75ae809614..accd2d0a334 100644 --- a/vnext/Shared/WinRTWebSocketResource.h +++ b/vnext/Shared/WinRTWebSocketResource.h @@ -51,9 +51,6 @@ class WinRTWebSocketResource : public IWebSocketResource, public std::enable_sha winrt::fire_and_forget PerformWrite(std::string &&message, bool isBinary) noexcept; winrt::fire_and_forget PerformClose() noexcept; - void OnMessageReceived( - winrt::Windows::Networking::Sockets::IWebSocket const &sender, - winrt::Windows::Networking::Sockets::IMessageWebSocketMessageReceivedEventArgs const &args); void Synchronize() noexcept; public: From 3e55ee95fe694b55b536037e929f850c6ada82a0 Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Fri, 17 Dec 2021 16:23:17 -0600 Subject: [PATCH 3/4] Remove change files --- ...ative-windows-857f8e44-c7be-4025-827d-902572d0c205.json | 7 ------- ...ative-windows-e21dbe3d-18ab-42a0-a4a3-f96fb8c0997f.json | 7 ------- 2 files changed, 14 deletions(-) delete mode 100644 change/react-native-windows-857f8e44-c7be-4025-827d-902572d0c205.json delete mode 100644 change/react-native-windows-e21dbe3d-18ab-42a0-a4a3-f96fb8c0997f.json diff --git a/change/react-native-windows-857f8e44-c7be-4025-827d-902572d0c205.json b/change/react-native-windows-857f8e44-c7be-4025-827d-902572d0c205.json deleted file mode 100644 index caa65d7d036..00000000000 --- a/change/react-native-windows-857f8e44-c7be-4025-827d-902572d0c205.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "type": "prerelease", - "comment": "Use shared pointer in MessageReceived", - "packageName": "react-native-windows", - "email": "julio.rocha@microsoft.com", - "dependentChangeType": "patch" -} diff --git a/change/react-native-windows-e21dbe3d-18ab-42a0-a4a3-f96fb8c0997f.json b/change/react-native-windows-e21dbe3d-18ab-42a0-a4a3-f96fb8c0997f.json deleted file mode 100644 index 93547280d96..00000000000 --- a/change/react-native-windows-e21dbe3d-18ab-42a0-a4a3-f96fb8c0997f.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "type": "prerelease", - "comment": "Avoid capturing raw `this` pointer in WS and AsyncStorage'", - "packageName": "react-native-windows", - "email": "julio.rocha@microsoft.com", - "dependentChangeType": "patch" -} From aaf31442fe816af5222b3973c3e0197f01b94501 Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Fri, 17 Dec 2021 16:33:44 -0600 Subject: [PATCH 4/4] Change files --- ...ative-windows-56c880d7-2062-4dd3-828a-a054307770a4.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/react-native-windows-56c880d7-2062-4dd3-828a-a054307770a4.json diff --git a/change/react-native-windows-56c880d7-2062-4dd3-828a-a054307770a4.json b/change/react-native-windows-56c880d7-2062-4dd3-828a-a054307770a4.json new file mode 100644 index 00000000000..249f807af3f --- /dev/null +++ b/change/react-native-windows-56c880d7-2062-4dd3-828a-a054307770a4.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "[0.67] Avoid capturing raw this pointer in WebSocket module", + "packageName": "react-native-windows", + "email": "julio.rocha@microsoft.com", + "dependentChangeType": "patch" +}