[Curl] Allocate CurlSSLVerifier only when it is required.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Jan 2018 22:43:11 +0000 (22:43 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Jan 2018 22:43:11 +0000 (22:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=182061

CurlSSLVerifier was a member function of CurlRequest. This patch do
lazy initialization of it only when actually it is required.
Also configuration method is not required by moving those stuff to
constructor of SSLVerifier which makes much safer because there's
no change to change its behavior from outside.

Patch by Basuke Suzuki <Basuke.Suzuki@sony.com> on 2018-01-24
Reviewed by Alex Christensen.

* platform/network/curl/CurlRequest.cpp:
(WebCore::CurlRequest::willSetupSslCtx):
(WebCore::CurlRequest::didCompleteTransfer):
(WebCore::CurlRequest::finalizeTransfer):
* platform/network/curl/CurlRequest.h:
* platform/network/curl/CurlSSLVerifier.cpp:
(WebCore::CurlSSLVerifier::CurlSSLVerifier):
(WebCore::CurlSSLVerifier::setSslCtx): Deleted.
* platform/network/curl/CurlSSLVerifier.h:
(WebCore::CurlSSLVerifier::setCurlHandle): Deleted.
(WebCore::CurlSSLVerifier::setHostName): Deleted.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@227575 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/network/curl/CurlRequest.cpp
Source/WebCore/platform/network/curl/CurlRequest.h
Source/WebCore/platform/network/curl/CurlSSLVerifier.cpp
Source/WebCore/platform/network/curl/CurlSSLVerifier.h

index 68806eb..3d148af 100644 (file)
@@ -1,3 +1,28 @@
+2018-01-24  Basuke Suzuki  <Basuke.Suzuki@sony.com>
+
+        [Curl] Allocate CurlSSLVerifier only when it is required.
+        https://bugs.webkit.org/show_bug.cgi?id=182061
+
+        CurlSSLVerifier was a member function of CurlRequest. This patch do
+        lazy initialization of it only when actually it is required.
+        Also configuration method is not required by moving those stuff to
+        constructor of SSLVerifier which makes much safer because there's
+        no change to change its behavior from outside.
+
+        Reviewed by Alex Christensen.
+
+        * platform/network/curl/CurlRequest.cpp:
+        (WebCore::CurlRequest::willSetupSslCtx):
+        (WebCore::CurlRequest::didCompleteTransfer):
+        (WebCore::CurlRequest::finalizeTransfer):
+        * platform/network/curl/CurlRequest.h:
+        * platform/network/curl/CurlSSLVerifier.cpp:
+        (WebCore::CurlSSLVerifier::CurlSSLVerifier):
+        (WebCore::CurlSSLVerifier::setSslCtx): Deleted.
+        * platform/network/curl/CurlSSLVerifier.h:
+        (WebCore::CurlSSLVerifier::setCurlHandle): Deleted.
+        (WebCore::CurlSSLVerifier::setHostName): Deleted.
+
 2018-01-24  Antti Koivisto  <antti@apple.com>
 
         Assertion failure in RenderMultiColumnSet::requiresBalancing() on fast/multicol/spanner-crash-when-adding-summary.html
index a11e118..d536332 100644 (file)
@@ -232,9 +232,7 @@ CURL* CurlRequest::setupTransfer()
 
 CURLcode CurlRequest::willSetupSslCtx(void* sslCtx)
 {
-    m_sslVerifier.setCurlHandle(m_curlHandle.get());
-    m_sslVerifier.setHostName(m_request.url().host());
-    m_sslVerifier.setSslCtx(sslCtx);
+    m_sslVerifier = std::make_unique<CurlSSLVerifier>(m_curlHandle.get(), m_request.url().host(), sslCtx);
 
     return CURLE_OK;
 }
@@ -427,8 +425,8 @@ void CurlRequest::didCompleteTransfer(CURLcode result)
     } else {
         auto type = (result == CURLE_OPERATION_TIMEDOUT && m_request.timeoutInterval() > 0.0) ? ResourceError::Type::Timeout : ResourceError::Type::General;
         auto resourceError = ResourceError::httpError(result, m_request.url(), type);
-        if (m_sslVerifier.sslErrors())
-            resourceError.setSslErrors(m_sslVerifier.sslErrors());
+        if (m_sslVerifier && m_sslVerifier->sslErrors())
+            resourceError.setSslErrors(m_sslVerifier->sslErrors());
 
         finalizeTransfer();
         callClient([error = resourceError.isolatedCopy()](CurlRequestClient& client) {
@@ -447,6 +445,7 @@ void CurlRequest::finalizeTransfer()
 {
     closeDownloadFile();
     m_formDataStream.clean();
+    m_sslVerifier = nullptr;
     m_multipartHandle = nullptr;
     m_curlHandle = nullptr;
 }
index b003f72..82fdc85 100644 (file)
@@ -157,7 +157,7 @@ private:
 
     std::unique_ptr<CurlHandle> m_curlHandle;
     CurlFormDataStream m_formDataStream;
-    CurlSSLVerifier m_sslVerifier;
+    std::unique_ptr<CurlSSLVerifier> m_sslVerifier;
     std::unique_ptr<CurlMultipartHandle> m_multipartHandle;
 
     CurlResponse m_response;
index 3e682b2..34d6c97 100644 (file)
 
 namespace WebCore {
 
-void CurlSSLVerifier::setSslCtx(void* sslCtx)
+CurlSSLVerifier::CurlSSLVerifier(CurlHandle* curlHandle, const String& hostName, void* sslCtx)
+    : m_curlHandle(curlHandle)
+    , m_hostName(hostName)
 {
-    if (!sslCtx)
-        return;
-
-    SSL_CTX_set_app_data(static_cast<SSL_CTX*>(sslCtx), this);
-    SSL_CTX_set_verify(static_cast<SSL_CTX*>(sslCtx), SSL_VERIFY_PEER, certVerifyCallback);
+    if (sslCtx) {
+        SSL_CTX_set_app_data(static_cast<SSL_CTX*>(sslCtx), this);
+        SSL_CTX_set_verify(static_cast<SSL_CTX*>(sslCtx), SSL_VERIFY_PEER, certVerifyCallback);
+    }
 }
 
 int CurlSSLVerifier::certVerifyCallback(int ok, X509_STORE_CTX* storeCtx)
index 4b9530d..6ec536c 100644 (file)
@@ -50,11 +50,7 @@ public:
         SSL_CERTIFICATE_GENERIC_ERROR = (1 << 6) // Some other error occurred validating the certificate
     };
 
-    CurlSSLVerifier() = default;
-
-    void setCurlHandle(CurlHandle* curlHandle) { m_curlHandle = curlHandle; }
-    void setHostName(const String& hostName) { m_hostName = hostName; }
-    void setSslCtx(void*);
+    CurlSSLVerifier(CurlHandle*, const String& hostName, void* sslCtx);
 
     int sslErrors() { return m_sslErrors; }