Merge pull request #9659 from JakubOnderka/curl-timeout-5-mins

chg: [sync] Reduce default timeout for remote HTTP request to 300 sec…
MISP · Apr 14, 2024 · 731b969 · 731b969
2 parents df7ff3d + 8a42cf4
commit 731b969
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 12 deletions.
diff --git a/app/Lib/Tools/CurlClient.php b/app/Lib/Tools/CurlClient.php
@@ -6,8 +6,12 @@ class CurlClient extends HttpSocketExtended
     /** @var resource */
     private $ch;
 
-    /** @var int */
-    private $timeout = 10800;
+    /**
+     * Maximum time the transfer is allowed to complete in seconds
+     * 300 seconds is recommended timeout for MISP servers
+     * @var int
+     */
+    private $timeout = 300;
 
     /** @var string|null */
     private $caFile;
@@ -30,6 +34,9 @@ class CurlClient extends HttpSocketExtended
     /** @var array */
     private $proxy = [];
 
+    /** @var array */
+    private $defaultOptions;
+
     /**
      * @param array $params
      * @noinspection PhpMissingParentConstructorInspection
@@ -38,8 +45,6 @@ public function __construct(array $params)
     {
         if (isset($params['timeout'])) {
             $this->timeout = $params['timeout'];
-        } else {
-            $this->timeout = Configure::check('MISP.curl_request_timeout') ? Configure::read('MISP.curl_request_timeout') : 10800;
         }
         if (isset($params['ssl_cafile'])) {
             $this->caFile = $params['ssl_cafile'];
@@ -59,6 +64,7 @@ public function __construct(array $params)
         if (isset($params['ssl_verify_peer'])) {
             $this->verifyPeer = $params['ssl_verify_peer'];
         }
+        $this->defaultOptions = $this->generateDefaultOptions();
     }
 
     /**
@@ -166,6 +172,7 @@ public function configProxy($host, $port = 3128, $method = null, $user = null, $
             return;
         }
         $this->proxy = compact('host', 'port', 'method', 'user', 'pass');
+        $this->defaultOptions = $this->generateDefaultOptions(); // regenerate default options in case proxy setting is changed
     }
 
     /**
@@ -196,7 +203,7 @@ private function internalRequest($method, $url, $query, $request)
             $url .= '?' . http_build_query($query, '', '&', PHP_QUERY_RFC3986);
         }
 
-        $options = $this->generateOptions();
+        $options = $this->defaultOptions; // this will copy default options
         $options[CURLOPT_URL] = $url;
         $options[CURLOPT_CUSTOMREQUEST] = $method;
 
@@ -303,7 +310,7 @@ private function convertCryptoMethod($cryptoMethod)
     /**
      * @return array
      */
-    private function generateOptions()
+    private function generateDefaultOptions()
     {
         $options = [
             CURLOPT_FOLLOWLOCATION => true, // Allows to follow redirect

diff --git a/app/Lib/Tools/SyncTool.php b/app/Lib/Tools/SyncTool.php
@@ -49,7 +49,7 @@ public function setupHttpSocketFeed()
      * @return HttpSocketExtended
      * @throws Exception
      */
-    public function createHttpSocket($params = array())
+    public function createHttpSocket(array $params = [])
     {
         // Use own CA PEM file
         $caPath = Configure::read('MISP.ca_path');
@@ -81,10 +81,11 @@ public function createHttpSocket($params = array())
             }
             $params['ssl_crypto_method'] = $version;
         }
-        if (!isset($params['timeout'])) {
-            $params['timeout'] = Configure::check('MISP.curl_request_timeout') ? Configure::read('MISP.curl_request_timeout') : 10800;
-        }
+
         if (function_exists('curl_init')) {
+            if (!isset($params['timeout']) && Configure::check('MISP.curl_request_timeout')) {
+                $params['timeout'] = (int)Configure::read('MISP.curl_request_timeout');
+            }
             App::uses('CurlClient', 'Tools');
             $HttpSocket = new CurlClient($params);
         } else {

diff --git a/app/Model/Server.php b/app/Model/Server.php
@@ -5139,8 +5139,8 @@ private function generateServerSettings()
                 ),
                 'curl_request_timeout' => [
                     'level' => 1,
-                    'description' => __('Control the timeout of curl requests issued by MISP (during synchronisation, feed fetching, etc.'),
-                    'value' => 10800,
+                    'description' => __('Control the default timeout in seconds of curl HTTP requests issued by MISP (during synchronisation, feed fetching, etc.)'),
+                    'value' => 300,
                     'test' => 'testForNumeric',
                     'type' => 'numeric',
                     'null' => true