From 3a0975503e916eb163e81898cc827f352d3cbbad Mon Sep 17 00:00:00 2001 From: Jean-Philippe Doyle Date: Fri, 3 Jul 2015 15:15:31 -0400 Subject: [PATCH] Ensure S3 presigned default expires time is not changing It occurred to us that signatures end up invalid randomly because the default signature time is computed twice and we can end up with a policy that was signed for a policy with an expiration time 1 second earlier. To be specific, the policy is computed twice inside the `fields` method which uses the `formation_expiration` twice too, which in turn computes `Time.now` at two different times. @see https://github.com/marcel/aws-s3/pull/54 (similar issue) --- lib/aws/s3/presigned_post.rb | 13 ++++++------- spec/aws/s3/presigned_post_spec.rb | 11 +++++++++++ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/lib/aws/s3/presigned_post.rb b/lib/aws/s3/presigned_post.rb index 569639b21e9..d45c6027e1f 100644 --- a/lib/aws/s3/presigned_post.rb +++ b/lib/aws/s3/presigned_post.rb @@ -207,7 +207,7 @@ def initialize(bucket, opts = {}) @content_length = range_value(opts[:content_length]) @conditions = opts[:conditions] || {} @ignored_fields = [opts[:ignore]].flatten.compact - @expires = opts[:expires] + @expires = opts[:expires] || Time.now.utc + 60*60 super @@ -397,17 +397,16 @@ def with_condition(field, condition) # @api private private def format_expiration - time = expires || Time.now.utc + 60*60 time = - case time + case expires when Time - time + expires when DateTime - Time.parse(time.to_s) + Time.parse(expires.to_s) when Integer - (Time.now + time) + (Time.now + expires) when String - Time.parse(time) + Time.parse(expires) end time.utc.iso8601 end diff --git a/spec/aws/s3/presigned_post_spec.rb b/spec/aws/s3/presigned_post_spec.rb index 6278d23021d..c4be026af0c 100644 --- a/spec/aws/s3/presigned_post_spec.rb +++ b/spec/aws/s3/presigned_post_spec.rb @@ -352,6 +352,17 @@ def policy_conditions(post) policy["expiration"].should == "2011-05-25T01:51:04Z" end + it "should reuse the default expire set during initialize" do + now = Time.parse("2011-05-24T17:54:04-07:00Z") + Time.stub(:now).and_return(now) + policy["expiration"].should == "2011-05-25T01:54:04Z" + + later = Time.parse("2011-05-24T17:54:05-07:00Z") + Time.stub(:now).and_return(later) + later_policy = JSON.load(Base64.decode64(post.policy)) + later_policy["expiration"].should == "2011-05-25T01:54:04Z" + end + context 'when :expires is provided' do it 'should support Time' do