RESOLVED FIXED 162962
[SOUP] Ensure NetworkProcess is ready before whitelisting TLS certificates
https://bugs.webkit.org/show_bug.cgi?id=162962
Summary [SOUP] Ensure NetworkProcess is ready before whitelisting TLS certificates
Emanuele Aina
Reported 2016-10-05 02:09:23 PDT
If the API user tries to whitelist TLS certificates before any web view has been created, the action will be ignored because the NetworkProcess hasn't been fired up yet. For example, the snippet below using the GTK+ API does not work, unless the whitelisting is moved after the web view creation: webkit_web_context_allow_tls_certificate_for_host(webkit_web_context_get_default(), crt, host); webView = webkit_web_view_new();
Attachments
Patch (2.27 KB, patch)
2016-10-05 02:11 PDT, Emanuele Aina
achristensen: review+
mcatanzaro: commit-queue-
Emanuele Aina
Comment 1 2016-10-05 02:11:28 PDT
Alex Christensen
Comment 2 2016-10-05 08:42:51 PDT
I'm not opposed to this change, but long-term we're hoping to get rid of it. See discussion in https://bugs.webkit.org/show_bug.cgi?id=162910 Is it still wanted?
Emanuele Aina
Comment 3 2016-10-05 15:59:45 PDT
I just stumbled on this unexpected behaviour where the whitelisting silently failed, I'll leave any other consideration to Michael and Carlos. :)
Alex Christensen
Comment 4 2016-10-05 16:01:05 PDT
Comment on attachment 290698 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290698&action=review r=me, but hopefully we'll remove this whole feature soon. You can decide if you want to land it. > w/Source/WebKit2/UIProcess/WebProcessPool.cpp:1149 > + ASSERT(m_networkProcess); not necessary
Michael Catanzaro
Comment 5 2016-10-05 16:38:39 PDT
We can't remove the feature, it's part of our API and we intend to support it indefinitely. What we can do is reimplement the feature at the GTK API layer using whatever UI process callbacks get added, then it can be removed from the cross-platform WebKit2 code.
Michael Catanzaro
Comment 6 2016-10-05 16:46:36 PDT
Comment on attachment 290698 [details] Patch Indeed, the ASSERT is not helpful here. This looks easy to test, so if you don't want to risk it breaking in the next refactoring, it would be nice if you could try to add a test for it in TestSSL.cpp. The only trick is that you can't use the nice SSLTest class since that doesn't give you the ability to use the WebKitWebContext before the WebKitWebView is created; you'd have to roll your own that inherits from Test rather than LoadTrackingTest/WebViewTest.
Michael Catanzaro
Comment 7 2016-11-19 17:07:15 PST
Emanuele, I think you should commit this (without the ASSERT) even if you don't have time to write a test.
Michael Catanzaro
Comment 8 2016-12-23 09:11:26 PST
Did this get forgotten?
Michael Catanzaro
Comment 9 2016-12-27 09:57:50 PST
Let's land this.
Michael Catanzaro
Comment 10 2016-12-27 09:58:08 PST
Emanuele Aina
Comment 11 2017-02-02 03:30:25 PST
I tried to come up with a test, but then I failed to find the time to finish it. Thank you for taking care of the patch!
Note You need to log in before you can comment on or make changes to this bug.