-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Javascript Binding - Finalize JavascriptBindingApiAllowOrigins: Per-browser isolation and origin normalization #5218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
ae50a6f
fb03970
be83917
4043025
74f6726
4eeaef6
2eac865
effcf91
f971f4c
b54d60f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ | |
| #include "Wrapper\Browser.h" | ||
| #include "..\CefSharp.Core.Runtime\Internals\Messaging\Messages.h" | ||
| #include "..\CefSharp.Core.Runtime\Internals\Serialization\Primitives.h" | ||
| #include <include/cef_parser.h> | ||
|
|
||
| using namespace System; | ||
| using namespace System::Diagnostics; | ||
|
|
@@ -87,7 +88,7 @@ namespace CefSharp | |
| //Using LegacyBinding with multiple ChromiumWebBrowser instances that share the same | ||
| //render process and using LegacyBinding will cause problems for the limited caching implementation | ||
| //that exists at the moment, for now we'll remove an object if already exists, same behaviour | ||
| //as the new binding method. | ||
| //as the new binding method. | ||
| //TODO: This should be removed when https://github.com/cefsharp/CefSharp/issues/2306 | ||
| //Is complete as objects will be stored at the browser level | ||
| if (_javascriptObjects->ContainsKey(obj->JavascriptName)) | ||
|
|
@@ -98,16 +99,33 @@ namespace CefSharp | |
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| _jsBindingApiEnabled = extraInfo->GetBool("JavascriptBindingApiEnabled"); | ||
| if (extraInfo->HasKey("JavascriptBindingApiEnabled")) | ||
| { | ||
| wrapper->JavascriptBindingApiEnabled = extraInfo->GetBool("JavascriptBindingApiEnabled"); | ||
| } | ||
|
|
||
| if (extraInfo->HasKey("JavascriptBindingApiHasAllowOrigins")) | ||
| { | ||
| wrapper->JavascriptBindingApiHasAllowOrigins = extraInfo->GetBool("JavascriptBindingApiHasAllowOrigins"); | ||
|
|
||
| if (extraInfo->HasKey("JsBindingPropertyName") || extraInfo->HasKey("JsBindingPropertyNameCamelCase")) | ||
| if (wrapper->JavascriptBindingApiHasAllowOrigins) | ||
| { | ||
| //TODO: Create constant for these and legacy binding strings above | ||
| _jsBindingPropertyName = extraInfo->GetString("JsBindingPropertyName"); | ||
| _jsBindingPropertyNameCamelCase = extraInfo->GetString("JsBindingPropertyNameCamelCase"); | ||
| auto allowOrigins = extraInfo->GetList("JavascriptBindingApiAllowOrigins"); | ||
| if (allowOrigins.get() && allowOrigins->IsValid()) | ||
| { | ||
| wrapper->JavascriptBindingApiAllowOrigins = allowOrigins->Copy(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (extraInfo->HasKey("JsBindingPropertyName") || extraInfo->HasKey("JsBindingPropertyNameCamelCase")) | ||
| { | ||
| //TODO: Create constant for these and legacy binding strings above | ||
| _jsBindingPropertyName = extraInfo->GetString("JsBindingPropertyName"); | ||
| _jsBindingPropertyNameCamelCase = extraInfo->GetString("JsBindingPropertyNameCamelCase"); | ||
| } | ||
| } | ||
|
|
||
| void CefAppUnmanagedWrapper::OnBrowserDestroyed(CefRefPtr<CefBrowser> browser) | ||
|
|
@@ -147,11 +165,12 @@ namespace CefSharp | |
| } | ||
| } | ||
|
|
||
| if (_jsBindingApiEnabled) | ||
| auto browserWrapper = FindBrowserWrapper(browser->GetIdentifier()); | ||
|
|
||
| if (browserWrapper != nullptr && browserWrapper->JavascriptBindingApiEnabled && IsJavascriptBindingApiAllowed(frame)) | ||
| { | ||
| //TODO: Look at adding some sort of javascript mapping layer to reduce the code duplication | ||
| auto global = context->GetGlobal(); | ||
| auto browserWrapper = FindBrowserWrapper(browser->GetIdentifier()); | ||
| auto processId = System::Diagnostics::Process::GetCurrentProcess()->Id; | ||
|
|
||
| //TODO: JSB: Split functions into their own classes | ||
|
|
@@ -328,6 +347,58 @@ namespace CefSharp | |
| return rootObject; | ||
| } | ||
|
|
||
| bool CefAppUnmanagedWrapper::IsJavascriptBindingApiAllowed(CefRefPtr<CefFrame> frame) | ||
| { | ||
| auto browserWrapper = FindBrowserWrapper(frame->GetBrowser()->GetIdentifier()); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would have expected the browser for frame to be same as the browser that was passed into It's been a while since I worked on this code, so my memory is a little vague on the specifics. |
||
|
|
||
| if (browserWrapper == nullptr || !browserWrapper->JavascriptBindingApiHasAllowOrigins) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| auto allowOrigins = browserWrapper->JavascriptBindingApiAllowOrigins; | ||
| if (!allowOrigins.get()) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| auto frameUrl = frame->GetURL(); | ||
|
|
||
| CefURLParts frameUrlParts; | ||
|
|
||
| if (CefParseURL(frameUrl, frameUrlParts)) | ||
| { | ||
| auto originStr = frameUrlParts.origin.str; | ||
| auto originLen = frameUrlParts.origin.length; | ||
|
|
||
| if (originLen > 0 && originStr[originLen - 1] == L'/') | ||
| { | ||
| originLen--; | ||
| } | ||
|
|
||
| auto frameUrlOrigin = CefString(originStr, originLen); | ||
|
|
||
| auto size = static_cast<int>(allowOrigins->GetSize()); | ||
|
|
||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| for (int i = 0; i < size; i++) | ||
| { | ||
| auto origin = allowOrigins->GetString(i); | ||
| auto frameOriginPtr = reinterpret_cast<const wchar_t*>(frameUrlOrigin.c_str()); | ||
| auto allowedOriginPtr = reinterpret_cast<const wchar_t*>(origin.c_str()); | ||
|
Comment on lines
+386
to
+387
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use (I haven't checked to see if it's possible) |
||
|
|
||
| if (frameOriginPtr != nullptr && allowedOriginPtr != nullptr) | ||
| { | ||
| if (_wcsicmp(frameOriginPtr, allowedOriginPtr) == 0) | ||
| { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| CefBrowserWrapper^ CefAppUnmanagedWrapper::FindBrowserWrapper(int browserId) | ||
| { | ||
| CefBrowserWrapper^ wrapper = nullptr; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.