Craft CMS 2 - S3AssetSourceType: Non-AWS Compatibility

Craft2-S3AssetSourceType-Non-AWS-Compatibility.patch

From 2be9bbc6b7996112c56c788a33907871bdc20bb1 Mon Sep 17 00:00:00 2001
From: Andy Thomas <git@andythom.as>
Date: Thu, 8 Oct 2020 08:32:33 +0100
Subject: [PATCH] S3AssetSourceType: Non-AWS Compatibility

- Add overridable default configuration for various S3 Asset Source attributes to allow compatibility with other S3-Compatible services (eg. DigitalOcean Spaces).
- Update/amend S3AssetSourceType to utilise new configuration.
- Amended S3 library class to support parsing of XML responses with a `text/xml` header.
---
 src/assetsourcetypes/S3AssetSourceType.php | 72 +++++++++++++++-------
 src/enums/ConfigFile.php                   | 15 ++---
 src/etc/config/defaults/s3assetsource.php  | 47 ++++++++++++++
 src/lib/S3.php                             | 15 ++++-
 4 files changed, 117 insertions(+), 32 deletions(-)
 create mode 100644 src/etc/config/defaults/s3assetsource.php

diff --git a/src/assetsourcetypes/S3AssetSourceType.php b/src/assetsourcetypes/S3AssetSourceType.php
index 457effc106..f95350b037 100644
--- a/src/assetsourcetypes/S3AssetSourceType.php
+++ b/src/assetsourcetypes/S3AssetSourceType.php
@@ -19,16 +19,6 @@ class S3AssetSourceType extends BaseAssetSourceType
 	// Properties
 	// =========================================================================
 
-	/**
-	 * A list of predefined endpoints.
-	 *
-	 * @var array
-	 */
-	private static $_predefinedEndpoints = array(
-		'US' => 's3.amazonaws.com',
-		'EU' => 's3-eu-west-1.amazonaws.com'
-	);
-
 	/**
 	 * @var \S3
 	 */
@@ -48,7 +38,22 @@ class S3AssetSourceType extends BaseAssetSourceType
 	 */
 	public static function getBucketList($keyId, $secret)
 	{
-		$s3 = new \S3($keyId, $secret);
+		$host = craft()->config->get('host', 's3assetsource');
+		$region = craft()->config->get('region', 's3assetsource');
+
+		$endpoint = str_replace(
+			array('{host}', '{region}'),
+			array($host, $region),
+			craft()->config->get('endpoint', 's3assetsource')
+		);
+
+		$urlPrefix = str_replace(
+			array('{host}', '{region}'),
+			array($host, $region),
+			craft()->config->get('urlPrefix', 's3assetsource')
+		);
+
+		$s3 = new \S3($keyId, $secret, false, $endpoint);
 		$s3->setExceptions(true);
 
 		try
@@ -67,13 +72,16 @@ class S3AssetSourceType extends BaseAssetSourceType
 		{
 			try
 			{
-				$location = $s3->getBucketLocation($bucket);
+				$location = $region ?: $s3->getBucketLocation($bucket);
 				$bucketList[] = array(
 					'bucket' => $bucket,
 					'location' => $location,
-					'urlPrefix' => 'http://'.static::getEndpointByLocation($location).'/'.$bucket.'/'
+					'urlPrefix' => str_replace(
+                        array('{bucket}', '{endpointByLocation}', '{location}'),
+                        array($bucket, static::getEndpointByLocation($location), $location),
+                    	$urlPrefix
+                    ),
 				);
-
 			}
 			catch (\Exception $exception)
 			{
@@ -93,12 +101,17 @@ class S3AssetSourceType extends BaseAssetSourceType
 	 */
 	public static function getEndpointByLocation($location)
 	{
-		if (isset(static::$_predefinedEndpoints[$location]))
-		{
-			return static::$_predefinedEndpoints[$location];
-		}
-
-		return 's3-'.$location.'.amazonaws.com';
+		$predefinedEndpoints = craft()->config->get('predefinedEndpoints', 's3assetsource') ?: array();
+		$host = craft()->config->get('host', 's3assetsource');
+		$endpointByLocation = (isset($predefinedEndpoints[$location]))
+			? $predefinedEndpoints[$location]
+			: craft()->config->get('endpointByLocation', 's3assetsource');
+
+		return str_replace(
+			array('{host}', '{location}'),
+			array($host, $location),
+			$endpointByLocation
+		);
 	}
 
 	/**
@@ -108,7 +121,7 @@ class S3AssetSourceType extends BaseAssetSourceType
 	 */
 	public function getName()
 	{
-		return 'Amazon S3';
+		return craft()->config->get('assetSourceName', 's3assetsource');
 	}
 
 	/**
@@ -433,7 +446,7 @@ class S3AssetSourceType extends BaseAssetSourceType
 	{
 		$baseFileName = IOHelper::getFileName($fileName, false);
 		$prefix = $this->_getPathPrefix().$folder->path;
-		
+
 		$this->_prepareForRequests();
 		$fileList = $this->_s3->getBucket($this->getSettings()->bucket, $prefix.$baseFileName);
 
@@ -717,6 +730,10 @@ class S3AssetSourceType extends BaseAssetSourceType
 			$diff = $expires->format('U') - $now->format('U');
 			$headers['Cache-Control'] = 'max-age='.$diff.', must-revalidate';
 		}
+		else if (empty($object) && craft()->config->get('putObjectForceContentLength', 's3assetsource'))
+		{
+			$headers['Content-Length'] = 0;
+		}
 
 		return $this->_s3->putObject($object, $bucket, $uriPath, $permissions, array(), $headers);
 	}
@@ -790,7 +807,16 @@ class S3AssetSourceType extends BaseAssetSourceType
 		}
 
 		\S3::setAuth($settings->keyId, $settings->secret);
-		$this->_s3->setEndpoint(static::getEndpointByLocation($settings->location));
+
+		$host = craft()->config->get('host', 's3assetsource');
+		$region = craft()->config->get('region', 's3assetsource');
+		$endpoint = str_replace(
+			array('{host}', '{region}'),
+			array($host, $region),
+			craft()->config->get('endpoint', 's3assetsource')
+		);
+
+		$this->_s3->setEndpoint($endpoint ?: static::getEndpointByLocation($settings->location));
 	}
 
 	/**
diff --git a/src/enums/ConfigFile.php b/src/enums/ConfigFile.php
index 05a83750c5..17f6e7b425 100644
--- a/src/enums/ConfigFile.php
+++ b/src/enums/ConfigFile.php
@@ -18,11 +18,12 @@ abstract class ConfigFile extends BaseEnum
 	// Constants
 	// =========================================================================
 
-	const FileCache    = 'filecache';
-	const General      = 'general';
-	const Db           = 'db';
-	const DbCache      = 'dbcache';
-	const Memcache     = 'memcache';
-	const RedisCache   = 'rediscache';
-	const ApcCache     = 'apc';
+	const FileCache     = 'filecache';
+	const General       = 'general';
+	const Db            = 'db';
+	const DbCache       = 'dbcache';
+	const Memcache      = 'memcache';
+	const RedisCache    = 'rediscache';
+	const ApcCache      = 'apc';
+	const S3AssetSource = 's3assetsource';
 }
diff --git a/src/etc/config/defaults/s3assetsource.php b/src/etc/config/defaults/s3assetsource.php
new file mode 100644
index 0000000000..d2a4698d5a
--- /dev/null
+++ b/src/etc/config/defaults/s3assetsource.php
@@ -0,0 +1,47 @@
+<?php
+
+/**
+ * DO NOT EDIT THIS FILE.
+ *
+ * This file is subject to be overwritten by a Craft update at any time.
+ *
+ * If you want to change any of these settings, copy it into craft/config/s3assetsource.php, and make your change there.
+ */
+
+return array(
+	/**
+	 * The S3 Asset Source name.
+	 */
+	'assetSourceName' => 'Amazon S3',
+	/**
+	 * The S3 Asset Source host.
+	 */
+    'host' => 'amazonaws.com',
+    /**
+	 * The S3 Asset Source region - leave empty to use bucket location detection.
+	 */
+    'region' => '',
+    /**
+	 * The S3 Asset Source endpoint - {host/region} will be replaced by their configured values.
+	 */
+    'endpoint' => 's3.{host}',
+    /**
+	 * The S3 Asset Source endpoint by location - default format if not predefined. {host/location} will be replaced by their configured values.
+	 */
+    'endpointByLocation' => 's3-{location}.{host}',
+	/**
+	 * The S3 Asset Source predefined endpoints by location. {host/location} will be replaced by the configured value.
+	 */
+    'predefinedEndpoints' => array(
+		'US' => 's3.{host}',
+		'EU' => 's3-eu-west-1.{host}',
+	),
+    /**
+	 * The S3 Asset Source URL prefix - {bucket/host/endpointbyLocation/location} will be replaced by their configured values.
+	 */
+    'urlPrefix' => 'http://{endpointByLocation}/{bucket}/',
+    /**
+	 * Force a Content-Length header of zero on empty putObject requests to solve 411 Length Required issues.
+	 */
+    'putObjectForceContentLength' => 0,
+);
diff --git a/src/lib/S3.php b/src/lib/S3.php
index c12a1c67a8..86b71da3b9 100644
--- a/src/lib/S3.php
+++ b/src/lib/S3.php
@@ -2214,9 +2214,20 @@ final class S3Request
 
 		@curl_close($curl);
 
+		if (isset($this->response->headers['type'])) {
+			$is_xml = in_array(
+				strstr($this->response->headers['type'], ';', true),
+				array(
+					'application/xml',
+					'text/xml'
+				)
+			);
+		} else {
+			$is_xml = 0;
+		}
+
 		// Parse body into XML
-		if ($this->response->error === false && isset($this->response->headers['type']) &&
-		$this->response->headers['type'] == 'application/xml' && isset($this->response->body))
+		if ($this->response->error === false && $is_xml && isset($this->response->body))
 		{
 			$this->response->body = simplexml_load_string($this->response->body);
 
-- 
2.27.0

Thanks so much for this patch Andy! Helping me out in a big way getting some old Craft 2 sites migrated over. Cheers!

Thanks for the feedback @jooosh - glad you've found it useful!

Also, I've just updated the patch as I recently found another issue with bucket listings not working with DigitalOcean Spaces. The responses weren't getting parsed properly due to using a text/xml header whilst the library expects this to be application/xml. The update fixes this by checking for a response header type & seeing if it's one of these two values.