Changeset 229138 in webkit

Timestamp:

Mar 1, 2018, 12:39:00 PM (7 years ago)

Author:

commit-queue@webkit.org

Message:

[Curl] Split DNS cache expiration and connection timeout setting.
​https://bugs.webkit.org/show_bug.cgi?id=182979

It was mis-implemented and was named ambiguous name 'enableTimeout'.
Implement each feature correctly.

Patch by Basuke Suzuki <Basuke Suzuki> on 2018-03-01
Reviewed by Per Arne Vollan.

• platform/network/curl/CurlContext.cpp:

(WebCore::EnvironmentVariableReader::read):
(WebCore::EnvironmentVariableReader::defined):
(WebCore::EnvironmentVariableReader::readAs):
(WebCore::EnvironmentVariableReader::sscanTemplate):
(WebCore::EnvironmentVariableReader::sscanTemplate<unsigned>):
(WebCore::CurlContext::CurlContext):
(WebCore::safeTimeValue):
(WebCore::CurlHandle::setDnsCacheTimeout):
(WebCore::CurlHandle::setConnectTimeout):
(WebCore::CurlHandle::setTimeout):
(WebCore::CurlHandle::enableTimeout): Deleted.

• platform/network/curl/CurlContext.h:

(WebCore::CurlContext::dnsCacheTimeout const):
(WebCore::CurlContext::connectTimeout const):

• platform/network/curl/CurlRequest.cpp:

(WebCore::CurlRequest::setupTransfer):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/network/curl/CurlContext.cpp (modified) (3 diffs)
• platform/network/curl/CurlContext.h (modified) (4 diffs)
• platform/network/curl/CurlRequest.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-03-01  Basuke Suzuki  <Basuke.Suzuki@sony.com>
+
+        [Curl] Split DNS cache expiration and connection timeout setting.
+        https://bugs.webkit.org/show_bug.cgi?id=182979
+
+        It was mis-implemented and was named ambiguous name 'enableTimeout'.
+        Implement each feature correctly.
+
+        Reviewed by Per Arne Vollan.
+
+        * platform/network/curl/CurlContext.cpp:
+        (WebCore::EnvironmentVariableReader::read):
+        (WebCore::EnvironmentVariableReader::defined):
+        (WebCore::EnvironmentVariableReader::readAs):
+        (WebCore::EnvironmentVariableReader::sscanTemplate):
+        (WebCore::EnvironmentVariableReader::sscanTemplate<unsigned>):
+        (WebCore::CurlContext::CurlContext):
+        (WebCore::safeTimeValue):
+        (WebCore::CurlHandle::setDnsCacheTimeout):
+        (WebCore::CurlHandle::setConnectTimeout):
+        (WebCore::CurlHandle::setTimeout):
+        (WebCore::CurlHandle::enableTimeout): Deleted.
+        * platform/network/curl/CurlContext.h:
+        (WebCore::CurlContext::dnsCacheTimeout const):
+        (WebCore::CurlContext::connectTimeout const):
+        * platform/network/curl/CurlRequest.cpp:
+        (WebCore::CurlRequest::setupTransfer):
+
 2018-03-01  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/platform/network/curl/CurlContext.cpp

@@ -43 +43 @@
 namespace WebCore {
 
+class EnvironmentVariableReader {
+public:
+    const char* read(const char* name) { return ::getenv(name); }
+    bool defined(const char* name) { return read(name) != nullptr; }
+
+    template<typename T> std::optional<T> readAs(const char* name)
+    {
+        if (const char* valueStr = read(name)) {
+            T value;
+            if (sscanf(valueStr, sscanTemplate<T>(), &value) == 1)
+                return value;
+        }
+
+        return std::nullopt;
+    }
+
+private:
+    template<typename T> const char* sscanTemplate()
+    {
+        ASSERT_NOT_REACHED();
+        return nullptr;
+    }
+
+    // define specialized member function for specific type.
+    template<> constexpr const char* sscanTemplate<unsigned>() { return "%u"; }
+};
+
 // CurlContext -------------------------------------------------------------------
 
@@ -55 +82 @@
     initShareHandle();
 
+    EnvironmentVariableReader envVar;
+
+    if (auto value = envVar.readAs<unsigned>("WEBKIT_CURL_DNS_CACHE_TIMEOUT"))
+        m_dnsCacheTimeout = Seconds(*value);
+
+    if (auto value = envVar.readAs<unsigned>("WEBKIT_CURL_CONNECT_TIMEOUT"))
+        m_connectTimeout = Seconds(*value);
+
 #ifndef NDEBUG
-    m_verbose = getenv("DEBUG_CURL");
-
-    char* logFile = getenv("CURL_LOG_FILE");
+    m_verbose = envVar.defined("DEBUG_CURL");
+
+    auto logFile = envVar.read("CURL_LOG_FILE");
     if (logFile)
         m_logFile = fopen(logFile, "a");
@@ -448 +483 @@
 }
 
-void CurlHandle::enableTimeout()
-{
-    static const long dnsCacheTimeout = 5 * 60; // [sec.]
-
-    curl_easy_setopt(m_handle, CURLOPT_DNS_CACHE_TIMEOUT, dnsCacheTimeout);
-}
-
-void CurlHandle::setTimeout(long timeoutMilliseconds)
-{
-    curl_easy_setopt(m_handle, CURLOPT_TIMEOUT_MS, timeoutMilliseconds);
+static CURLoption safeTimeValue(double time)
+{
+    auto value = static_cast<unsigned>(time >= 0.0 ? time : 0);
+    return static_cast<CURLoption>(value);
+}
+
+void CurlHandle::setDnsCacheTimeout(Seconds timeout)
+{
+    curl_easy_setopt(m_handle, CURLOPT_DNS_CACHE_TIMEOUT, safeTimeValue(timeout.seconds()));
+}
+
+void CurlHandle::setConnectTimeout(Seconds timeout)
+{
+    curl_easy_setopt(m_handle, CURLOPT_CONNECTTIMEOUT, safeTimeValue(timeout.seconds()));
+}
+
+void CurlHandle::setTimeout(Seconds timeout)
+{
+    curl_easy_setopt(m_handle, CURLOPT_TIMEOUT_MS, safeTimeValue(timeout.milliseconds()));
 }
 

trunk/Source/WebCore/platform/network/curl/CurlContext.h

@@ -33 +33 @@
 #include <wtf/NeverDestroyed.h>
 #include <wtf/Noncopyable.h>
+#include <wtf/Seconds.h>
 #include <wtf/Threading.h>
 
@@ -121 +122 @@
     bool isHttp2Enabled() const;
 
+    // Timeout
+    Seconds dnsCacheTimeout() const { return m_dnsCacheTimeout; }
+    Seconds connectTimeout() const { return m_connectTimeout; }
+
 #ifndef NDEBUG
     FILE* getLogFile() const { return m_logFile; }
@@ -127 +132 @@
 
 private:
+    CurlContext();
+    void initShareHandle();
+
     ProxyInfo m_proxy;
     CurlShareHandle m_shareHandle;
     CurlSSLHandle m_sslHandle;
 
-    CurlContext();
-    void initShareHandle();
+    Seconds m_dnsCacheTimeout { Seconds::fromMinutes(5) };
+    Seconds m_connectTimeout { 30.0 };
 
 #ifndef NDEBUG
@@ -251 +259 @@
     void enableProxyIfExists();
 
-    void enableTimeout();
-    void setTimeout(long timeoutMilliseconds);
+    void setDnsCacheTimeout(Seconds);
+    void setConnectTimeout(Seconds);
+    void setTimeout(Seconds);
 
     // Callback function

trunk/Source/WebCore/platform/network/curl/CurlRequest.cpp

@@ -195 +195 @@
     m_curlHandle->enableAllowedProtocols();
     m_curlHandle->enableAcceptEncoding();
-    m_curlHandle->enableTimeout();
-
-    long timeoutMilliseconds = (m_request.timeoutInterval() > 0.0) ? static_cast<long>(m_request.timeoutInterval() * 1000.0) : 0;
-    m_curlHandle->setTimeout(timeoutMilliseconds);
+
+    m_curlHandle->setTimeout(Seconds(m_request.timeoutInterval()));
+    m_curlHandle->setDnsCacheTimeout(CurlContext::singleton().dnsCacheTimeout());
+    m_curlHandle->setConnectTimeout(CurlContext::singleton().connectTimeout());
 
     m_curlHandle->enableProxyIfExists();