Bug 230526 – [Curl] Show TLS connection information in the inspector

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Takashi Komori]

Created attachment 438761 [details]
Added information in the network security tab.

Add TLS version and cipher name in the network security tab.

[Takashi Komori]

Created attachment 438765 [details]
Patch

[Fujii Hironori]

Comment on attachment 438765 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=438765&action=review

> Source/WebCore/platform/network/curl/CurlContext.cpp:875
> +            m_tlsConnectionInfo = makeUnique<TLSConnectionInfo>();

Why is m_tlsConnectionInfo needed? What's the problem if setting to networkLoadMetrics.tlsProtocol directly?

[Fujii Hironori]

Comment on attachment 438765 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=438765&action=review

> Source/WebCore/platform/network/curl/CurlContext.h:322
> +        WTF_MAKE_FAST_ALLOCATED;

I think it'd better to use struct and WTF_MAKE_STRUCT_FAST_ALLOCATED.

[Fujii Hironori]

Comment on attachment 438765 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=438765&action=review

> Source/WebCore/platform/network/curl/OpenSSLHelper.cpp:329
> +String getTLSCipherName(const SSL* ssl)

You shouldn't use 'get' for these functions (Bug 227766). However, OpenSSLHelper.cpp has more 'get'. Sooner or later, they should be renamed.

[Takashi Komori]

Created attachment 438797 [details]
Patch

[Takashi Komori]

(In reply to Fujii Hironori from comment #2)
> Comment on attachment 438765 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=438765&action=review
> 
> > Source/WebCore/platform/network/curl/CurlContext.cpp:875
> > +            m_tlsConnectionInfo = makeUnique<TLSConnectionInfo>();
> 
> Why is m_tlsConnectionInfo needed? What's the problem if setting to
> networkLoadMetrics.tlsProtocol directly?

A CurlRequest gets metrics in CurlRequest::didReceiveHeader and CurlRequest::didCompleteTransfer but in didCompleteTransfer, curl_easy_getinfo can't obtain SSL pointer.
This is because when the connection ends, libcurl clears ssl context in the curl handle internally.
Therefore, we should keep TLS information in m_tlsConnectionInfo when didReceiveHeader is called.

[Takashi Komori]

(In reply to Fujii Hironori from comment #3)
> Comment on attachment 438765 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=438765&action=review
> 
> > Source/WebCore/platform/network/curl/CurlContext.h:322
> > +        WTF_MAKE_FAST_ALLOCATED;
> 
> I think it'd better to use struct and WTF_MAKE_STRUCT_FAST_ALLOCATED.

Fixed.

[Takashi Komori]

(In reply to Fujii Hironori from comment #4)
> Comment on attachment 438765 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=438765&action=review
> 
> > Source/WebCore/platform/network/curl/OpenSSLHelper.cpp:329
> > +String getTLSCipherName(const SSL* ssl)
> 
> You shouldn't use 'get' for these functions (Bug 227766). However,
> OpenSSLHelper.cpp has more 'get'. Sooner or later, they should be renamed.

Removed 'get' from function names.

[EWS]

Tools/Scripts/svn-apply failed to apply attachment 438797 [details] to trunk.
Please resolve the conflicts and upload a new patch.

[Takashi Komori]

Created attachment 438970 [details]
Patch

[Takashi Komori]

Fix the conflict of the patch of bug 226982.

[Takashi Komori]

Created attachment 439005 [details]
Patch for landing

[EWS]

Committed r282904 (242032@main): <https://commits.webkit.org/242032@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 439005 [details].

[Radar WebKit Bug Importer]

<rdar://problem/83429738>