Make HTTP origin value an instance member #12821
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
acoates-ms
reviewed
Mar 12, 2024
| @@ -653,8 +652,7 @@ void WinRTHttpResource::AddResponseHandler(shared_ptr<IResponseHandler> response | |||
| client = HttpClient{redirFilter}; | |||
| } else { | |||
| auto globalOrigin = GetRuntimeOptionString("Http.GlobalOrigin"); | |||
Contributor
There was a problem hiding this comment.
We should add a public PropertyId, or a setter for ReactPropertyBag/ReactInstanceSettings to allow setting this option on a per instance basis. If someone wanted to have different origins for different RN instances there would still be a synchronization issue here.
We still need the "legacy" GetRuntimeOptionString option for Office's non-abi code paths, but for public use this should all be done using ReactInstanceSettings.
Contributor
Author
There was a problem hiding this comment.
Can that be addressed in a follow-up PR? (I could use that to re-factor other property ID declarations that are done inefficiently).
This fix is required ASAP.
acoates-ms
approved these changes
Mar 12, 2024
This was referenced Mar 13, 2024
JunielKatarn
added a commit
to jurocha-ms/react-native-windows
that referenced
this pull request
Mar 13, 2024
* Make origin an instance member * Change files * Remove s_origin * Remove redundant namespace
JunielKatarn
added a commit
to jurocha-ms/react-native-windows
that referenced
this pull request
Mar 13, 2024
* Make origin an instance member * Change files * Remove s_origin * Remove redundant namespace
JunielKatarn
added a commit
that referenced
this pull request
Mar 13, 2024
* Make HTTP origin value an instance member (#12821) * Make origin an instance member * Change files * Remove s_origin * Remove redundant namespace * Remove change file * Change files * Add missing backports * 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 735b168. * Rename ConstWcharComparer to CaseInsensitiveComparer * Update vnext/Desktop.UnitTests/OriginPolicyHttpFilterTest.cpp Co-authored-by: Danny van Velzen <dannyvv@microsoft.com> * Remove unused code * Make ExposedHeaders case-insensitive --------- Co-authored-by: Danny van Velzen <dannyvv@microsoft.com> * Remove change file * clang format * Remove redundant include --------- Co-authored-by: Danny van Velzen <dannyvv@microsoft.com>
NikoAri
reviewed
Mar 13, 2024
JunielKatarn
added a commit
that referenced
this pull request
Mar 14, 2024
* Make HTTP origin value an instance member (#12821) * Make origin an instance member * Change files * Remove s_origin * Remove redundant namespace * Remove change file * Change files * Empty commit
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Sets HTTP origin URL per filter instance.
Type of Change
Why
A crash can be caused by an inconsistent state when the host app is being shut down and an HTTP request has been scheduled from the JS layer.
This affects the static member
OriginPolicyHttpFilter::s_origin.What
Make the HTTP origin an instance member of
OriginPolicyHttpFilterso the value will be bound to the filter's life time.In most cases, this won't add significant memory overhead because the filter's instance (and its members) is a singleton for the owning resource, module, and React instance.
Testing
Updated tests
ValidatePreflightResponseHeadersCaseMismatchSucceedsandValidatePreflightResponseMainAndContentHeadersSucceeds.Microsoft Reviewers: Open in CodeFlow