RESOLVED FIXED 96497
[GTK] Add API to get/set the security policy of a given URI scheme to WebKit2 GTK+
https://bugs.webkit.org/show_bug.cgi?id=96497
Summary [GTK] Add API to get/set the security policy of a given URI scheme to WebKit2...
Carlos Garcia Campos
Reported 2012-09-12 04:49:38 PDT
This is useful to set the security policy for custom URI schemes, for example, to be treated as local or secure. See bug #95549 for the WebKit1 API.
Attachments
Patch (29.08 KB, patch)
2012-09-12 04:57 PDT, Carlos Garcia Campos
buildbot: commit-queue-
Try to fix the mac build (30.74 KB, patch)
2012-09-12 05:15 PDT, Carlos Garcia Campos
mrobinson: review-
New patch using a different API (46.02 KB, patch)
2012-09-18 06:41 PDT, Carlos Garcia Campos
mrobinson: review+
Carlos Garcia Campos
Comment 1 2012-09-12 04:57:44 PDT
Created attachment 163591 [details] Patch Same API than WebKit1.
WebKit Review Bot
Comment 2 2012-09-12 05:00:00 PDT
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
Build Bot
Comment 3 2012-09-12 05:04:35 PDT
Carlos Garcia Campos
Comment 4 2012-09-12 05:15:16 PDT
Created attachment 163597 [details] Try to fix the mac build
Martin Robinson
Comment 5 2012-09-14 08:42:33 PDT
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.
Carlos Garcia Campos
Comment 6 2012-09-14 09:02:09 PDT
This is not designed to remove uri schemes
Martin Robinson
Comment 7 2012-09-14 09:59:21 PDT
(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).
Carlos Garcia Campos
Comment 8 2012-09-14 10:17:34 PDT
(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.
Martin Robinson
Comment 9 2012-09-14 10:52:15 PDT
(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!
Carlos Garcia Campos
Comment 10 2012-09-18 06:41:55 PDT
Created attachment 164548 [details] New patch using a different API
Martin Robinson
Comment 11 2012-09-18 10:40:19 PDT
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?
Carlos Garcia Campos
Comment 12 2012-09-18 10:47:24 PDT
(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.
Carlos Garcia Campos
Comment 13 2012-09-19 04:32:13 PDT
Note You need to log in before you can comment on or make changes to this bug.