Merge pull request #8347 from Sesquipedalian/3.0/webfetchapi_improvments

Fixes some bugs in WebFetchApi

Sources/WebFetch/APIs/CurlFetcher.php

@@ -122,6 +122,13 @@ class CurlFetcher extends WebFetchApi
 	 */
 	public $headers;
 
+	/**
+	 * @var bool
+	 *
+	 * Whether to keep the connection open after the initial request.
+	 */
+	public bool $keep_alive = false;
+
 	/*********************
 	 * Internal properties
 	 *********************/
@@ -133,14 +140,14 @@ class CurlFetcher extends WebFetchApi
 	 */
 	private $default_options = [
 		// Get returned value as a string (don't output it).
-		CURLOPT_RETURNTRANSFER  => 1,
+		CURLOPT_RETURNTRANSFER  => true,
 
 		// We need the headers to do our own redirect.
-		CURLOPT_HEADER          => 1,
+		CURLOPT_HEADER          => true,
 
 		// Don't follow. We will do it ourselves so safe mode and open_basedir
 		// will dig it.
-		CURLOPT_FOLLOWLOCATION  => 0,
+		CURLOPT_FOLLOWLOCATION  => false,
 
 		// Set a normal looking user agent.
 		CURLOPT_USERAGENT       => SMF_USER_AGENT,
@@ -151,20 +158,17 @@ class CurlFetcher extends WebFetchApi
 		// A page should load in this amount of time.
 		CURLOPT_TIMEOUT         => 90,
 
-		// Stop after this many redirects.
-		CURLOPT_MAXREDIRS       => 5,
-
 		// Accept gzip and decode it.
 		CURLOPT_ENCODING        => 'gzip,deflate',
 
 		// Stop curl from verifying the peer's certificate.
-		CURLOPT_SSL_VERIFYPEER  => 0,
+		CURLOPT_SSL_VERIFYPEER  => false,
 
 		// Stop curl from verifying the peer's host.
 		CURLOPT_SSL_VERIFYHOST  => 0,
 
 		// No post data. This will change if some is passed to request().
-		CURLOPT_POST            => 0,
+		CURLOPT_POST            => false,
 	];
 
 	/****************
@@ -184,6 +188,25 @@ public function __construct(array $options = [], int $max_redirect = 3)
 		// Initialize class variables
 		$this->max_redirect = intval($max_redirect);
 		$this->user_options = $options;
+
+		// This class handles redirections itself.
+		if (!empty($this->user_options[CURLOPT_MAXREDIRS])) {
+			$this->max_redirect = $this->user_options[CURLOPT_MAXREDIRS];
+			unset($this->user_options[CURLOPT_MAXREDIRS]);
+		}
+
+		if (!empty($this->user_options[CURLOPT_FOLLOWLOCATION])) {
+			$this->max_redirect = max(3, $this->max_redirect);
+			$this->user_options[CURLOPT_FOLLOWLOCATION] = false;
+		}
+
+		// Do we want to keep the connection open after the initial request?
+		if (
+			version_compare(curl_version()['version'], '7.25.0', '>=')
+			&& !empty($this->user_options[CURLOPT_TCP_KEEPALIVE])
+		) {
+			$this->keep_alive = true;
+		}
 	}
 
 	/**
@@ -235,6 +258,12 @@ public function request(string|Url $url, array|string $post_data = []): object
 		}
 
 		// Set the options and get it.
+		if (version_compare(curl_version()['version'], '7.25.0', '>=')) {
+			$this->user_options[CURLOPT_TCP_KEEPALIVE] = (int) $this->keep_alive;
+		} else {
+			$this->keep_alive = false;
+		}
+
 		$this->setOptions();
 		$this->sendRequest(str_replace(' ', '%20', strval($url)));
 
@@ -393,7 +422,7 @@ private function setOptions(): void
 
 		// POST data options, here we don't allow any override.
 		if (isset($this->post_data)) {
-			$this->options[CURLOPT_POST] = 1;
+			$this->options[CURLOPT_POST] = true;
 			$this->options[CURLOPT_POSTFIELDS] = $this->post_data;
 		}
 	}

Sources/WebFetch/APIs/SocketFetcher.php

@@ -84,16 +84,16 @@ class SocketFetcher extends WebFetchApi
 	 */
 	public $response = [];
 
-	/*********************
-	 * Internal properties
-	 *********************/
-
 	/**
 	 * @var bool
 	 *
 	 * Whether to keep the socket connection open after the initial request.
 	 */
-	private bool $keep_alive;
+	public bool $keep_alive = false;
+
+	/*********************
+	 * Internal properties
+	 *********************/
 
 	/**
 	 * @var bool

Sources/WebFetch/WebFetchApi.php

@@ -61,6 +61,17 @@ abstract class WebFetchApi implements WebFetchApiInterface
 		'https' => ['SocketFetcher', 'CurlFetcher'],
 	];
 
+	/****************************
+	 * Internal static properties
+	 ****************************/
+
+	/**
+	 * @var array
+	 *
+	 * Fetchers that still have an open connection after the initial request.
+	 */
+	private static array $still_alive = [];
+
 	/****************
 	 * Public methods
 	 ****************/
@@ -121,14 +132,35 @@ public static function fetch(Url|string $url, string|array $post_data = [], bool
 			$data = false;
 		}
 
-		foreach (self::$scheme_handlers[$url->scheme] as $class) {
-			$class = __NAMESPACE__ . '\\APIs\\' . $class;
+		if (isset(self::$still_alive[(string) $url])) {
+			$fetcher = self::$still_alive[(string) $url];
+			$fetcher->request($url, $post_data);
+		} else {
+			foreach (self::$scheme_handlers[$url->scheme] as $class) {
+				// Get an instance of the desired class.
+				$class = __NAMESPACE__ . '\\APIs\\' . $class;
 
-			$fetcher = new $class();
-			$fetcher->request($url);
+				$fetcher = new $class();
 
-			if ($fetcher->result('success')) {
-				break;
+				// Do we want to keep this connection alive, and can we do so?
+				if ($keep_alive && property_exists($fetcher, 'keep_alive')) {
+					$fetcher->keep_alive = $keep_alive;
+					self::$still_alive[(string) $url] = $fetcher;
+				}
+
+				// Make the request.
+				$fetcher->request($url, $post_data);
+
+				// If keep_alive was turned off during the request, we don't
+				// need to maintain this instance after we're done the request.
+				if (!($fetcher->keep_alive ?? false)) {
+					unset(self::$still_alive[(string) $url]);
+				}
+
+				// If the request worked, we can stop looping.
+				if ($fetcher->result('success')) {
+					break;
+				}
 			}
 		}