[Webkit-unassigned] [Bug 191646] [Curl] Add Server Trust Evaluation Support.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=191646

Fujii Hironori <Hironori.Fujii at sony.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #366052|review?                     |review-
              Flags|                            |

--- Comment #3 from Fujii Hironori <Hironori.Fujii at sony.com> ---
Comment on attachment 366052
  --> https://bugs.webkit.org/attachment.cgi?id=366052
Patch

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

> Source/WebCore/platform/network/curl/AuthenticationChallengeCurl.cpp:80
> +    return ProtectionSpace(url.host().toString(), static_cast<int>(port ? *port : 0), serverType, String(), authenticationScheme, certificateInfo);

"(port ? *port : 0)" can be "port.valueOr(0)". protectionSpaceFromHandle has the same code.

> Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp:423
> +void NetworkDataTaskCurl::restartWithCredential(const AuthenticationChallenge& challenge, const Credential& credential)

It seems that you don't need change the first argument.

> Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp:433
> +    if (!challenge.protectionSpace().certificateInfo().certificateChain().isEmpty())

Is this never be empty in case of ServerTrustEvaluation? 
if (challenge.protectionSpace().authenticationScheme() == ProtectionSpaceAuthenticationSchemeServerTrustEvaluationRequested)
I feel this is somewhat better.

> Tools/MiniBrowser/win/Common.cpp:288
> +std::wstring replaceWString(std::wstring src, const std::wstring& oldValue, const std::wstring& newValue)

You can name this 'replaceString'. If you will need a std::string version, you can overload replaceString(std::string, const std::string&, const std::string&).

> Tools/MiniBrowser/win/Common.cpp:294
> +    while ((pos = src.find(oldValue, pos)) != std::string::npos) {

Nit: std::string::npos should be std::wstring::npos or src.npos.

> Tools/MiniBrowser/win/WebKitBrowserWindow.cpp:323
> +    if (it != m_acceptedServerTrustCert.end() && it->second == pem)

You check only the first matching. m_acceptedServerTrustCert can be std::set<std::pair<std::wstring,std::wstring>>.

> Tools/MiniBrowser/win/WebKitBrowserWindow.cpp:334
> +std::wstring WebKitBrowserWindow::createPEMString(WKProtectionSpaceRef protectionSpace)

This shouldn't be a WebKitBrowserWindow's method.

> Tools/MiniBrowser/win/WebKitBrowserWindow.cpp:345
> +        pems += std::wstring(reinterpret_cast<const char*>(data), reinterpret_cast<const char*>(data + size));

std::wstring(reinterpret_cast<const char*>(data), size)

> Tools/MiniBrowser/win/WebKitBrowserWindow.h:77
> +    std::map<std::wstring, std::wstring> m_acceptedServerTrustCert;

should be plural if this is just a single cert.
m_acceptedServerTrustCerts

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190327/40f04780/attachment.html>