Summary: | [GTK] Add API to get/set the security policy of a given URI scheme to WebKit2 GTK+ | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||||
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | gustavo, gyuyoung.kim, mario, mrobinson, rakuco, webkit.review.bot | ||||||||
Priority: | P2 | Keywords: | Gtk | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Linux | ||||||||||
Attachments: |
|
Description
Carlos Garcia Campos
2012-09-12 04:49:38 PDT
Created attachment 163591 [details]
Patch
Same API than WebKit1.
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API Comment on attachment 163591 [details] Patch Attachment 163591 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13825600 Created attachment 163597 [details]
Try to fix the mac build
Comment on attachment 163597 [details] Try to fix the mac build View in context: https://bugs.webkit.org/attachment.cgi?id=163597&action=review > Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:648 > + // We keep the WebCore::SchemeRegistry of the UI process in sync with the > + // web process one, so that we can return the WebKitSecurityPolicy for > + // a given URI scheme synchronously without blocking. > + if (policy & WEBKIT_SECURITY_POLICY_LOCAL) { > + WebCore::SchemeRegistry::registerURLSchemeAsLocal(urlScheme); > + webContext->registerURLSchemeAsLocal(urlScheme); > + } > + if (policy & WEBKIT_SECURITY_POLICY_NO_ACCESS_TO_OTHER_SCHEME) { > + WebCore::SchemeRegistry::registerURLSchemeAsNoAccess(urlScheme); > + webContext->registerURLSchemeAsNoAccess(urlScheme); > + } > + if (policy & WEBKIT_SECURITY_POLICY_DISPLAY_ISOLATED) { > + WebCore::SchemeRegistry::registerURLSchemeAsDisplayIsolated(urlScheme); > + webContext->registerURLSchemeAsDisplayIsolated(urlScheme); > + } > + if (policy & WEBKIT_SECURITY_POLICY_SECURE) { > + WebCore::SchemeRegistry::registerURLSchemeAsSecure(urlScheme); > + webContext->registerURLSchemeAsSecure(urlScheme); > + } > + if (policy & WEBKIT_SECURITY_POLICY_CORS_ENABLED) { > + WebCore::SchemeRegistry::registerURLSchemeAsCORSEnabled(urlScheme); > + webContext->registerURLSchemeAsCORSEnabled(urlScheme); > + } > + if (policy & WEBKIT_SECURITY_POLICY_EMPTY_DOCUMENT) { > + WebCore::SchemeRegistry::registerURLSchemeAsEmptyDocument(urlScheme); > + webContext->registerURLSchemeAsEmptyDocument(urlScheme); > + } > +} > + One thing I notice here is that if you say, activate WEBKIT_SECURITY_POLICY_LOCAL, and then call this method again with the same scheme without WEBKIT_SECURITY_POLICY_LOCAL removeURLSchemeRegisteredAsLocal is never called. It seems that you can never undo any of the actions you take. On the other hand there doesn't seem to be a way in WebCore to undo registration for all other types of schemes. Perhaps that means this API doesn't map to the one in WebCore. Perhaps you need to add a method in WebCore to remove all registrations for a scheme and call that first here. This is not designed to remove uri schemes (In reply to comment #6) > This is not designed to remove uri schemes If that's not a usecase you want to support it makes sense to match the WebCore API, I think. Only expose the register* methods. That way it's unambiguous. I can see this being very confusing if you try to change the security policy of a scheme (calling webkit_web_context_set_security_policy_for_uri_scheme more than once for the same scheme). (In reply to comment #7) > (In reply to comment #6) > > This is not designed to remove uri schemes > > If that's not a usecase you want to support it makes sense to match the WebCore API, I think. Only expose the register* methods. That way it's unambiguous. I can see this being very confusing if you try to change the security policy of a scheme (calling webkit_web_context_set_security_policy_for_uri_scheme more than once for the same scheme). You are right, it can be confusing, maybe we could add a WebKitSecurityPolicy object with methods to register uri schemes: webkit_secutiry_policy_register_uri_scheme_as_local() webkit_secutiry_policy_register_uri_scheme_as_secure() .... And also methods to query: webkit_secutiry_policy_is_uri_scheme_local() webkit_secutiry_policy_is_uri_scheme_secure() .... And method to get the security policy object (or boxed type) from the context WebKitSecutiryPolicy* webkit_web_context_get_secutory_policy(). I used the flags to reduce the api, but I agree it can be confusing, we should do the same in wk1. (In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #6) > > > This is not designed to remove uri schemes > > > > If that's not a usecase you want to support it makes sense to match the WebCore API, I think. Only expose the register* methods. That way it's unambiguous. I can see this being very confusing if you try to change the security policy of a scheme (calling webkit_web_context_set_security_policy_for_uri_scheme more than once for the same scheme). > > You are right, it can be confusing, maybe we could add a WebKitSecurityPolicy object with methods to register uri schemes: > > webkit_secutiry_policy_register_uri_scheme_as_local() > webkit_secutiry_policy_register_uri_scheme_as_secure() > .... > > And also methods to query: > > webkit_secutiry_policy_is_uri_scheme_local() > webkit_secutiry_policy_is_uri_scheme_secure() > .... > > And method to get the security policy object (or boxed type) from the context > > WebKitSecutiryPolicy* webkit_web_context_get_secutory_policy(). > > I used the flags to reduce the api, but I agree it can be confusing, we should do the same in wk1. You're suggestion makes sense to me. Sorry that I didn't notice this when you did the version for WebKti1! Created attachment 164548 [details]
New patch using a different API
Comment on attachment 164548 [details] New patch using a different API View in context: https://bugs.webkit.org/attachment.cgi?id=164548&action=review Looks good. Couple small suggestions before landing... > Source/WebKit2/UIProcess/API/gtk/WebKitSecurityManager.cpp:59 > +static void webkit_security_manager_class_init(WebKitSecurityManagerClass* findClass) Small nit here: findClass could just be klass or securityManagerClass. I guess this is just a copy-paste thing? > Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebContext.cpp:318 > + void check(const char* scheme, unsigned policy) Maybe a name like verifyThatSchemeMatchesPolicy? (In reply to comment #11) > (From update of attachment 164548 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=164548&action=review > > Looks good. Couple small suggestions before landing... > > > Source/WebKit2/UIProcess/API/gtk/WebKitSecurityManager.cpp:59 > > +static void webkit_security_manager_class_init(WebKitSecurityManagerClass* findClass) > > Small nit here: findClass could just be klass or securityManagerClass. I guess this is just a copy-paste thing? hehe, yes, I copied this from the CookieManager that was copied from the FindCrontroller. The CookieManager still has findClass :-P > > Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebContext.cpp:318 > > + void check(const char* scheme, unsigned policy) > > Maybe a name like verifyThatSchemeMatchesPolicy? Sure. Committed r128989: <http://trac.webkit.org/changeset/128989> |