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

[bugzilla-daemon at webkit.org]

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

--- Comment #7 from Takashi Komori <Takashi.Komori at sony.com> ---
Thank you for your review!

(In reply to Fujii Hironori from comment #3)
> Comment on attachment 366052 [details]
> 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.

Fixed.

> > 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.

Reverted to former argument list.

> > 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.

Fixed.

> > 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&).

Changed.

> > 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.

Fixed.

> > Tools/MiniBrowser/win/WebKitBrowserWindow.cpp:334
> > +std::wstring WebKitBrowserWindow::createPEMString(WKProtectionSpaceRef protectionSpace)
> 
> This shouldn't be a WebKitBrowserWindow's method.

Moved to Common.cpp

> > 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)

Fixed.

> > 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

Fixed.

-- 
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/20190328/91a9f38c/attachment-0001.html>