Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Adds secure :hash interpolation and fixes time zone brittleness in :t…
…imestamp interpolation
  • Loading branch information
jmileham committed Feb 10, 2011
1 parent fc792c8 commit 13c86cf
Show file tree
Hide file tree
Showing 7 changed files with 159 additions and 33 deletions.
2 changes: 1 addition & 1 deletion gemfiles/rails3.gemfile
Expand Up @@ -2,7 +2,7 @@

source "http://rubygems.org"
gem "ruby-debug"
gem "rails", ">=3.0.3"
gem "rails", "~>3.0.0"
gem "rake"
gem "sqlite3-ruby", "~>1.3.0"
gem "shoulda"
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rails3.gemfile.lock
Expand Up @@ -90,7 +90,7 @@ DEPENDENCIES
appraisal
aws-s3
mocha
rails (>= 3.0.3)
rails (~> 3.0.0)
rake
ruby-debug
shoulda
Expand Down
78 changes: 50 additions & 28 deletions lib/paperclip/attachment.rb
Expand Up @@ -8,16 +8,19 @@ class Attachment

def self.default_options
@default_options ||= {
:url => "/system/:attachment/:id/:style/:filename",
:path => ":rails_root/public:url",
:styles => {},
:processors => [:thumbnail],
:convert_options => {},
:default_url => "/:attachment/:style/missing.png",
:default_style => :original,
:storage => :filesystem,
:use_timestamp => true,
:whiny => Paperclip.options[:whiny] || Paperclip.options[:whiny_thumbnails]
:url => "/system/:attachment/:id/:style/:filename",
:path => ":rails_root/public:url",
:styles => {},
:processors => [:thumbnail],
:convert_options => {},
:default_url => "/:attachment/:style/missing.png",
:default_style => :original,
:storage => :filesystem,
:use_timestamp => true,
:whiny => Paperclip.options[:whiny] || Paperclip.options[:whiny_thumbnails],
:use_default_time_zone => true,
:hash_digest => "SHA1",
:hash_data => ":class/:attachment/:id/:style/:updated_at"
}
end

Expand All @@ -32,24 +35,28 @@ def initialize name, instance, options = {}

options = self.class.default_options.merge(options)

@url = options[:url]
@url = @url.call(self) if @url.is_a?(Proc)
@path = options[:path]
@path = @path.call(self) if @path.is_a?(Proc)
@styles = options[:styles]
@normalized_styles = nil
@default_url = options[:default_url]
@default_style = options[:default_style]
@storage = options[:storage]
@use_timestamp = options[:use_timestamp]
@whiny = options[:whiny_thumbnails] || options[:whiny]
@convert_options = options[:convert_options]
@processors = options[:processors]
@options = options
@queued_for_delete = []
@queued_for_write = {}
@errors = {}
@dirty = false
@url = options[:url]
@url = @url.call(self) if @url.is_a?(Proc)
@path = options[:path]
@path = @path.call(self) if @path.is_a?(Proc)
@styles = options[:styles]
@normalized_styles = nil
@default_url = options[:default_url]
@default_style = options[:default_style]
@storage = options[:storage]
@use_timestamp = options[:use_timestamp]
@whiny = options[:whiny_thumbnails] || options[:whiny]
@use_default_time_zone = options[:use_default_time_zone]
@hash_digest = options[:hash_digest]
@hash_data = options[:hash_data]
@hash_secret = options[:hash_secret]
@convert_options = options[:convert_options]
@processors = options[:processors]
@options = options
@queued_for_delete = []
@queued_for_write = {}
@errors = {}
@dirty = false

initialize_storage
end
Expand Down Expand Up @@ -197,6 +204,21 @@ def updated_at
time && time.to_f.to_i
end

# The time zone to use for timestamp interpolation. Using the default
# time zone ensures that results are consistent across all threads.
def time_zone
@use_default_time_zone ? Time.zone_default : Time.zone
end

# Returns a unique hash suitable for obfuscating the URL of an otherwise
# publicly viewable attachment.
def hash
raise ArgumentError, "Unable to generate hash without :hash_secret" unless @hash_secret
require 'openssl' unless defined?(OpenSSL)
data = interpolate(@hash_data)
OpenSSL::HMAC.hexdigest(OpenSSL::Digest.const_get(@hash_digest).new, @hash_secret, data)
end

def generate_fingerprint(source)
data = source.read
source.rewind if source.respond_to?(:rewind)
Expand Down
18 changes: 17 additions & 1 deletion lib/paperclip/interpolations.rb
Expand Up @@ -48,8 +48,18 @@ def url attachment, style_name
end

# Returns the timestamp as defined by the <attachment>_updated_at field
# in the server default time zone unless :use_global_time_zone is set
# to false. Note that a Rails.config.time_zone change will still
# invalidate any path or URL that uses :timestamp. For a
# time_zone-agnostic timestamp, use #updated_at.
def timestamp attachment, style_name
attachment.instance_read(:updated_at).to_s
attachment.instance_read(:updated_at).in_time_zone(attachment.time_zone).to_s
end

# Returns an integer timestamp that is time zone-neutral, so that paths
# remain valid even if a server's time zone changes.
def updated_at attachment, style_name
attachment.updated_at
end

# Returns the Rails.root constant.
Expand Down Expand Up @@ -94,6 +104,12 @@ def fingerprint attachment, style_name
attachment.fingerprint
end

# Returns a the attachment hash. See Paperclip::Attachment#hash for
# more details.
def hash attachment, style_name
attachment.hash
end

# Returns the id of the instance in a split path form. e.g. returns
# 000/001/234 for an id of 1234.
def id_partition attachment, style_name
Expand Down
72 changes: 72 additions & 0 deletions test/attachment_test.rb
Expand Up @@ -96,6 +96,78 @@ class AttachmentTest < Test::Unit::TestCase
assert_equal "1024.omg/1024-bbq/1024what/000/001/024.wtf", @dummy.avatar.path
end
end

context "An attachment with :timestamp interpolations" do
setup do
@file = StringIO.new("...")
@zone = 'UTC'
Time.stubs(:zone).returns(@zone)
@zone_default = 'Eastern Time (US & Canada)'
Time.stubs(:zone_default).returns(@zone_default)
end

context "using default time zone" do
setup do
rebuild_model :path => ":timestamp", :use_default_time_zone => true
@dummy = Dummy.new
@dummy.avatar = @file
end

should "return a time in the default zone" do
assert_equal @dummy.avatar_updated_at.in_time_zone(@zone_default).to_s, @dummy.avatar.path
end
end

context "using per-thread time zone" do
setup do
rebuild_model :path => ":timestamp", :use_default_time_zone => false
@dummy = Dummy.new
@dummy.avatar = @file
end

should "return a time in the per-thread zone" do
assert_equal @dummy.avatar_updated_at.in_time_zone(@zone).to_s, @dummy.avatar.path
end
end
end

context "An attachment with :hash interpolations" do
setup do
@file = StringIO.new("...")
end

should "raise if no secret is provided" do
@attachment = attachment :path => ":hash"
@attachment.assign @file

assert_raise ArgumentError do
@attachment.path
end
end

context "when secret is set" do
setup do
@attachment = attachment :path => ":hash", :hash_secret => "w00t"
@attachment.stubs(:instance_read).with(:updated_at).returns(Time.at(1234567890))
@attachment.stubs(:instance_read).with(:file_name).returns("bla.txt")
@attachment.instance.id = 1234
@attachment.assign @file
end

should "interpolate the hash data" do
@attachment.expects(:interpolate).with(@attachment.options[:hash_data]).returns("interpolated_stuff")
@attachment.hash
end

should "result in the correct interpolation" do
assert_equal "fake_models/avatars/1234/original/1234567890", @attachment.send(:interpolate,@attachment.options[:hash_data])
end

should "result in a correct hash" do
assert_equal "d22b617d1bf10016aa7d046d16427ae203f39fce", @attachment.path
end
end
end

context "An attachment with a :rails_env interpolation" do
setup do
Expand Down
2 changes: 1 addition & 1 deletion test/helper.rb
Expand Up @@ -90,7 +90,7 @@ def rebuild_class options = {}
class FakeModel
attr_accessor :avatar_file_name,
:avatar_file_size,
:avatar_last_updated,
:avatar_updated_at,
:avatar_content_type,
:avatar_fingerprint,
:id
Expand Down
18 changes: 17 additions & 1 deletion test/interpolations_test.rb
Expand Up @@ -112,9 +112,25 @@ def url(*args)

should "return the timestamp" do
now = Time.now
zone = 'UTC'
attachment = mock
attachment.expects(:instance_read).with(:updated_at).returns(now)
assert_equal now.to_s, Paperclip::Interpolations.timestamp(attachment, :style)
attachment.expects(:time_zone).returns(zone)
assert_equal now.in_time_zone(zone).to_s, Paperclip::Interpolations.timestamp(attachment, :style)
end

should "return updated_at" do
attachment = mock
seconds_since_epoch = 1234567890
attachment.expects(:updated_at).returns(seconds_since_epoch)
assert_equal seconds_since_epoch, Paperclip::Interpolations.updated_at(attachment, :style)
end

should "return hash" do
attachment = mock
fake_hash = "a_wicked_secure_hash"
attachment.expects(:hash).returns(fake_hash)
assert_equal fake_hash, Paperclip::Interpolations.hash(attachment, :style)
end

should "call all expected interpolations with the given arguments" do
Expand Down

0 comments on commit 13c86cf

Please sign in to comment.