From 6d242ebe57b2c8ffb66e90fd7ca58cc91d0d28f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julio=20C=C3=A9sar=20Rocha?= Date: Fri, 21 Apr 2023 16:29:27 -0700 Subject: [PATCH 1/2] Use case-insensitive comparison for CORS preflight responses (#11511) * Remove usage of RestoreUseStaticGraphEvaluation * Add test ValidatePreflightResponseHeadersCaseMismatchSucceeds * Use case-insensitive comparer for AllowedHeaders * Change files * Revert "Remove usage of RestoreUseStaticGraphEvaluation" This reverts commit 735b168892dc2d1643f49711df16afc20a07f689. * Rename ConstWcharComparer to CaseInsensitiveComparer * Update vnext/Desktop.UnitTests/OriginPolicyHttpFilterTest.cpp Co-authored-by: Danny van Velzen * Remove unused code * Make ExposedHeaders case-insensitive --------- Co-authored-by: Danny van Velzen --- ...-7cf2e2b2-8bb7-40e6-8859-f584c4033f3a.json | 7 +++ .../OriginPolicyHttpFilterTest.cpp | 49 +++++++++++++++++++ .../Networking/OriginPolicyHttpFilter.cpp | 30 +++++++----- .../Networking/OriginPolicyHttpFilter.h | 23 ++++----- 4 files changed, 85 insertions(+), 24 deletions(-) create mode 100644 change/react-native-windows-7cf2e2b2-8bb7-40e6-8859-f584c4033f3a.json diff --git a/change/react-native-windows-7cf2e2b2-8bb7-40e6-8859-f584c4033f3a.json b/change/react-native-windows-7cf2e2b2-8bb7-40e6-8859-f584c4033f3a.json new file mode 100644 index 00000000000..74aaa2f1646 --- /dev/null +++ b/change/react-native-windows-7cf2e2b2-8bb7-40e6-8859-f584c4033f3a.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "Use case-insensitive comparer for AllowedHeaders", + "packageName": "react-native-windows", + "email": "julio.rocha@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/vnext/Desktop.UnitTests/OriginPolicyHttpFilterTest.cpp b/vnext/Desktop.UnitTests/OriginPolicyHttpFilterTest.cpp index 7bf22675485..dc08b8c6ef2 100644 --- a/vnext/Desktop.UnitTests/OriginPolicyHttpFilterTest.cpp +++ b/vnext/Desktop.UnitTests/OriginPolicyHttpFilterTest.cpp @@ -7,6 +7,9 @@ #include #include "WinRTNetworkingMocks.h" +// Boost Library +#include + // Windows API #include #include @@ -17,6 +20,7 @@ using namespace winrt::Windows::Web::Http; using Microsoft::React::Networking::OriginPolicyHttpFilter; using Microsoft::React::Networking::RequestArgs; using Microsoft::React::Networking::ResponseOperation; +using std::wstring; using winrt::Windows::Foundation::Uri; namespace Microsoft::React::Test { @@ -252,6 +256,51 @@ TEST_CLASS (OriginPolicyHttpFilterTest) { } } + TEST_METHOD(ValidatePreflightResponseHeadersCaseMismatchSucceeds) { + auto mockFilter = winrt::make(); + mockFilter.as()->Mocks.SendRequestAsync = + [](HttpRequestMessage const &request) -> ResponseOperation { + HttpResponseMessage response{}; + + response.StatusCode(HttpStatusCode::Ok); + response.Headers().Insert(L"Access-Control-Allow-Origin", L"*"); + + // Return allowed headers as requested by client, in lower case. + // This tests case-insensitive preflight validation. + auto allowHeaders = boost::to_lower_copy(wstring{request.Headers().Lookup(L"Access-Control-Request-Headers")}); + response.Headers().Insert(L"Access-Control-Allow-Headers", std::move(allowHeaders)); + + co_return response; + }; + + auto reqArgs = winrt::make(); + auto request = HttpRequestMessage(HttpMethod::Get(), Uri{L"http://somehost"}); + request.Properties().Insert(L"RequestArgs", reqArgs); + request.Headers().TryAppendWithoutValidation(L"ChangeMyCase", L"Value"); + // Should implicitly set Conent-Length and Content-Type + request.Content(HttpStringContent{L"PreflightContent"}); + + auto filter = winrt::make(mockFilter); + auto opFilter = filter.as(); + + OriginPolicyHttpFilter::SetStaticOrigin("http://somehost"); + try { + auto sendOp = opFilter->SendPreflightAsync(request); + sendOp.get(); + + auto response = sendOp.GetResults(); + opFilter->ValidatePreflightResponse(request, response); + + OriginPolicyHttpFilter::SetStaticOrigin({}); + Assert::IsTrue(boost::iequals( + response.Headers().Lookup(L"Access-Control-Allow-Headers").c_str(), + L"ChangeMyCase, Content-Length, Content-Type")); + } catch (const winrt::hresult_error &e) { + OriginPolicyHttpFilter::SetStaticOrigin({}); + Assert::Fail(e.message().c_str()); + } + } + TEST_METHOD(ValidatePreflightResponseMainAndContentHeadersSucceeds) { auto mockFilter = winrt::make(); mockFilter.as()->Mocks.SendRequestAsync = diff --git a/vnext/Shared/Networking/OriginPolicyHttpFilter.cpp b/vnext/Shared/Networking/OriginPolicyHttpFilter.cpp index 4cd97230005..5611ad5b9c4 100644 --- a/vnext/Shared/Networking/OriginPolicyHttpFilter.cpp +++ b/vnext/Shared/Networking/OriginPolicyHttpFilter.cpp @@ -37,22 +37,26 @@ namespace Microsoft::React::Networking { #pragma region OriginPolicyHttpFilter -#pragma region ConstWcharComparer +#pragma region CaseInsensitiveComparer -bool OriginPolicyHttpFilter::ConstWcharComparer::operator()(const wchar_t *a, const wchar_t *b) const { +bool OriginPolicyHttpFilter::CaseInsensitiveComparer::operator()(const wchar_t *a, const wchar_t *b) const { return _wcsicmp(a, b) < 0; } -#pragma endregion ConstWcharComparer +bool OriginPolicyHttpFilter::CaseInsensitiveComparer::operator()(const wstring &a, const wstring &b) const { + return _wcsicmp(a.c_str(), b.c_str()) < 0; +} + +#pragma endregion CaseInsensitiveComparer // https://fetch.spec.whatwg.org/#forbidden-method -/*static*/ set OriginPolicyHttpFilter::s_forbiddenMethods = - {L"CONNECT", L"TRACE", L"TRACK"}; +/*static*/ set + OriginPolicyHttpFilter::s_forbiddenMethods = {L"CONNECT", L"TRACE", L"TRACK"}; -/*static*/ set +/*static*/ set OriginPolicyHttpFilter::s_simpleCorsMethods = {L"GET", L"HEAD", L"POST"}; -/*static*/ set +/*static*/ set OriginPolicyHttpFilter::s_simpleCorsRequestHeaderNames = { L"Accept", L"Accept-Language", @@ -64,11 +68,11 @@ bool OriginPolicyHttpFilter::ConstWcharComparer::operator()(const wchar_t *a, co L"Viewport-Width", L"Width"}; -/*static*/ set +/*static*/ set OriginPolicyHttpFilter::s_simpleCorsResponseHeaderNames = {L"Cache-Control", L"Content-Language", L"Content-Type", L"Expires", L"Last-Modified", L"Pragma"}; -/*static*/ set +/*static*/ set OriginPolicyHttpFilter::s_simpleCorsContentTypeValues = { L"application/x-www-form-urlencoded", L"multipart/form-data", @@ -76,7 +80,7 @@ bool OriginPolicyHttpFilter::ConstWcharComparer::operator()(const wchar_t *a, co // https://fetch.spec.whatwg.org/#forbidden-header-name // Chromium still bans "User-Agent" due to https://crbug.com/571722 -/*static*/ set +/*static*/ set OriginPolicyHttpFilter::s_corsForbiddenRequestHeaderNames = { L"Accept-Charset", L"Accept-Encoding", @@ -99,13 +103,13 @@ bool OriginPolicyHttpFilter::ConstWcharComparer::operator()(const wchar_t *a, co L"Upgrade", L"Via"}; -/*static*/ set +/*static*/ set OriginPolicyHttpFilter::s_cookieSettingResponseHeaders = { L"Set-Cookie", L"Set-Cookie2", // Deprecated by the spec, but probably still used }; -/*static*/ set +/*static*/ set OriginPolicyHttpFilter::s_corsForbiddenRequestHeaderNamePrefixes = {L"Proxy-", L"Sec-"}; /*static*/ Uri OriginPolicyHttpFilter::s_origin{nullptr}; @@ -293,7 +297,7 @@ bool OriginPolicyHttpFilter::ConstWcharComparer::operator()(const wchar_t *a, co } /*static*/ OriginPolicyHttpFilter::AccessControlValues OriginPolicyHttpFilter::ExtractAccessControlValues( - winrt::Windows::Foundation::Collections::IMap const &headers) { + IMap const &headers) { using std::wregex; using std::wsregex_token_iterator; diff --git a/vnext/Shared/Networking/OriginPolicyHttpFilter.h b/vnext/Shared/Networking/OriginPolicyHttpFilter.h index 49426d62eab..99526f9c6ed 100644 --- a/vnext/Shared/Networking/OriginPolicyHttpFilter.h +++ b/vnext/Shared/Networking/OriginPolicyHttpFilter.h @@ -22,19 +22,20 @@ class OriginPolicyHttpFilter : public winrt:: implements { public: - struct ConstWcharComparer { + struct CaseInsensitiveComparer { bool operator()(const wchar_t *, const wchar_t *) const; + bool operator()(const std::wstring &, const std::wstring &) const; }; private: - static std::set s_forbiddenMethods; - static std::set s_simpleCorsMethods; - static std::set s_simpleCorsRequestHeaderNames; - static std::set s_simpleCorsResponseHeaderNames; - static std::set s_simpleCorsContentTypeValues; - static std::set s_corsForbiddenRequestHeaderNames; - static std::set s_corsForbiddenRequestHeaderNamePrefixes; - static std::set s_cookieSettingResponseHeaders; + static std::set s_forbiddenMethods; + static std::set s_simpleCorsMethods; + static std::set s_simpleCorsRequestHeaderNames; + static std::set s_simpleCorsResponseHeaderNames; + static std::set s_simpleCorsContentTypeValues; + static std::set s_corsForbiddenRequestHeaderNames; + static std::set s_corsForbiddenRequestHeaderNamePrefixes; + static std::set s_cookieSettingResponseHeaders; // NOTE: Assumes static origin through owning client/resource/module/(React) instance's lifetime. static winrt::Windows::Foundation::Uri s_origin; @@ -42,9 +43,9 @@ class OriginPolicyHttpFilter struct AccessControlValues { winrt::hstring AllowedOrigin; winrt::hstring AllowedCredentials; - std::set AllowedHeaders; + std::set AllowedHeaders; std::set AllowedMethods; - std::set ExposedHeaders; + std::set ExposedHeaders; size_t MaxAge; }; From 3377e77f07ef68ed3d073feb8ee1a065dbdb136a Mon Sep 17 00:00:00 2001 From: "Julio C. Rocha" Date: Sat, 22 Apr 2023 00:38:31 -0400 Subject: [PATCH 2/2] Change files --- ...ative-windows-7cf2e2b2-8bb7-40e6-8859-f584c4033f3a.json | 7 ------- ...ative-windows-a522f514-d922-4ef1-b08e-b4ed9da35b8e.json | 7 +++++++ 2 files changed, 7 insertions(+), 7 deletions(-) delete mode 100644 change/react-native-windows-7cf2e2b2-8bb7-40e6-8859-f584c4033f3a.json create mode 100644 change/react-native-windows-a522f514-d922-4ef1-b08e-b4ed9da35b8e.json diff --git a/change/react-native-windows-7cf2e2b2-8bb7-40e6-8859-f584c4033f3a.json b/change/react-native-windows-7cf2e2b2-8bb7-40e6-8859-f584c4033f3a.json deleted file mode 100644 index 74aaa2f1646..00000000000 --- a/change/react-native-windows-7cf2e2b2-8bb7-40e6-8859-f584c4033f3a.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "type": "prerelease", - "comment": "Use case-insensitive comparer for AllowedHeaders", - "packageName": "react-native-windows", - "email": "julio.rocha@microsoft.com", - "dependentChangeType": "patch" -} diff --git a/change/react-native-windows-a522f514-d922-4ef1-b08e-b4ed9da35b8e.json b/change/react-native-windows-a522f514-d922-4ef1-b08e-b4ed9da35b8e.json new file mode 100644 index 00000000000..9a9b7813829 --- /dev/null +++ b/change/react-native-windows-a522f514-d922-4ef1-b08e-b4ed9da35b8e.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Use case-insensitive comparison for CORS preflight responses (#11511)", + "packageName": "react-native-windows", + "email": "dev@rocha.red", + "dependentChangeType": "patch" +}