This patch fixes two problems I discovered through doing some digging:

1. In order to create a proper signed URL, the "expires" header is needed by the AmazonS3 API. Unfortunately, the AmazonS3StreamWrapper class does not pass this argument onto the s3.class.php function authenticate(), which is required:

// If we're generating a URL, return the URL to the calling method.
		if (isset($opt['preauth']) && (integer) $opt['preauth'] > 0)
		{
			$query_params = array(
				'AWSAccessKeyId' => $this->key,
				'Expires' => $headers['Expires'],
				'Signature' => $signature,
			);

2. When a CDN Domain Name is set in AmazonS3 settings, it is only honored if https is set to TRUE and and Cloudfront is also set to true. We're not using Cloudfront, so the two being mutually exclusive proved problematic. The way I've fixed this initially is to do a simply string replacement. This was how other modules have done it, though I believe it may be possible to pass it into the API correctly.

I'd love feedback on this patch, even if there is a better way to address #2. I have yet to add it to production but so far it seems to work well.

CommentFileSizeAuthor
AmazonS3_signedurls_CDN.patch944 bytestorgosPizza
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

torgosPizza’s picture

Status: Active » Needs review

Setting to Needs review to get some more eyes on.

torgosPizza’s picture

We've been using this in production and I haven't seen any issues.

Note that the last part (about CDN domains) will be redundant if #2044307: Support multiple CNAMEs gets rolled into the module.

deviantintegral’s picture

This has been fixed in the 7.x-2.x branch. See:

For anyone needing this on the 7.x-1.x branch, this patch gets a +1 from me, but since the 1.x S3 SDK will likely be EOL, and we don't have test coverage, I'm not making any further commits there.

deviantintegral’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.