WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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-
Details
Formatted Diff
Diff
Try to fix the mac build
(30.74 KB, patch)
2012-09-12 05:15 PDT
,
Carlos Garcia Campos
mrobinson
: review-
Details
Formatted Diff
Diff
New patch using a different API
(46.02 KB, patch)
2012-09-18 06:41 PDT
,
Carlos Garcia Campos
mrobinson
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
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
Comment on
attachment 163591
[details]
Patch
Attachment 163591
[details]
did not pass mac-ews (mac): Output:
http://queues.webkit.org/results/13825600
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
Committed
r128989
: <
http://trac.webkit.org/changeset/128989
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug