WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
134830
[GTK] Remove WebKitCertificateInfo from WebKit2GTK+ API
https://bugs.webkit.org/show_bug.cgi?id=134830
Summary
[GTK] Remove WebKitCertificateInfo from WebKit2GTK+ API
Carlos Garcia Campos
Reported
2014-07-11 03:22:13 PDT
Add webkit_certificate_info_new() to allow the user to create a WebKitCertificateInfo for a given GTlsCertificate and GTlsCertificateFlags. This would be needed to be able to add exceptions with webkit_web_context_allow_tls_certificate_for_host() for a certificate saved permanently, for example.
Attachments
Patch
(3.79 KB, patch)
2014-07-11 03:26 PDT
,
Carlos Garcia Campos
no flags
Details
Formatted Diff
Diff
Patch
(24.27 KB, patch)
2014-07-29 05:50 PDT
,
Carlos Garcia Campos
no flags
Details
Formatted Diff
Diff
Rebased to apply on current git master
(24.17 KB, patch)
2014-07-29 05:56 PDT
,
Carlos Garcia Campos
gustavo
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1
2014-07-11 03:26:36 PDT
Created
attachment 234755
[details]
Patch
WebKit Commit Bot
Comment 2
2014-07-11 03:27:46 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
Gustavo Noronha (kov)
Comment 3
2014-07-16 11:25:31 PDT
Comment on
attachment 234755
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=234755&action=review
> Source/WebKit2/UIProcess/API/gtk/WebKitCertificateInfo.cpp:54 > + * @tls_errors: #GTlsCertificateFlags verification status
I don't understand this parameter very well, I don't remember certificate errors reporting very well, but this is a good indication that we should probably improve this. Does this list the types of errors that should be accepted by the exception?
Gustavo Noronha (kov)
Comment 4
2014-07-16 11:26:59 PDT
… improve this documentation I mean
Michael Catanzaro
Comment 5
2014-07-16 11:41:01 PDT
In the context of WebKitCertificateInfo, it's only used to indicate what errors actually occurred. It does not have any effect on the behavior of webkit_web_context_allow_tls_certificate_for_host().
Carlos Garcia Campos
Comment 6
2014-07-21 04:50:38 PDT
(In reply to
comment #3
)
> (From update of
attachment 234755
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=234755&action=review
> > > Source/WebKit2/UIProcess/API/gtk/WebKitCertificateInfo.cpp:54 > > + * @tls_errors: #GTlsCertificateFlags verification status > > I don't understand this parameter very well, I don't remember certificate errors reporting very well, but this is a good indication that we should probably improve this. Does this list the types of errors that should be accepted by the exception?
That pair is what you get from soup_message_get_https_status() or webkit_web_view_get_tls_info(). WebKitCertificateInfo was added later. I agree it's a bit confusing, WebKitCertificateInfo contains the certificate itself and possible TLS errors of the certificate or 0 for valid certificates. In WebCore CertificateInfo contains the pair, because DidCommitLoad message contains a CertificateInfo parameter, and in that case we need both. WebKitCertificateInfo was exposed when we added the API to handle TLS errors and add exceptions for certificates. The idea was that you get a WebKitCertificateInfo in WebKitWebView::failed-with-tls-errors that you can use to get the TLS info and use also later to add an exception. This made the API a bit inconsistent with webkit_web_view_get_tls_info(). So, maybe we could rethink the whole thing now that we are breaking the API. The WebKitCertificateInfo is not really needed for webkit_web_context_allow_tls_certificate_for_host() since only the certificate is used in that case. WebKitWebView::failed-with-tls-errors might receive the pair as parameters instead of WebKitCertificateInfo. That would probably simplify the API since we can get rid of the WebKitCertificateInfo entirely.
Michael Catanzaro
Comment 7
2014-07-21 11:47:28 PDT
I was going to say that the webkit_certificate_info_new() would not really be needed if webkit_web_view_get_tls_info() were to return a WebKitCertificateInfo instead, but if you're planning to remove WebKitCertificateInfo from the API that'd probably be better.
Carlos Garcia Campos
Comment 8
2014-07-29 05:43:59 PDT
This was added to make the API more convenient but it has ended up making it more complicated, so in the end I think it's better to remove it.
Carlos Garcia Campos
Comment 9
2014-07-29 05:50:52 PDT
Created
attachment 235679
[details]
Patch
Carlos Garcia Campos
Comment 10
2014-07-29 05:56:13 PDT
Created
attachment 235680
[details]
Rebased to apply on current git master
Carlos Garcia Campos
Comment 11
2014-07-29 05:57:02 PDT
I think this is the last API change blocking the libs version bump
Gustavo Noronha (kov)
Comment 12
2014-07-29 08:24:13 PDT
Comment on
attachment 235680
[details]
Rebased to apply on current git master This looks good to me. I don't think the likelihood of needing to extend the API is big enough to warrant a webkit object just for it, so let's do it =)
Carlos Garcia Campos
Comment 13
2014-07-30 00:07:11 PDT
Committed
r171792
: <
http://trac.webkit.org/changeset/171792
>
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