From 746e245aa923b0a0366949f11616cd4af7693f48 Mon Sep 17 00:00:00 2001 From: Andrew Selder Date: Tue, 19 Jul 2016 15:42:09 -0700 Subject: [PATCH 1/2] Don't blow up when passing frozen string to send_file disposition. Since `attachment` does an in-place mutation of `response['Content-Disposition']`, we need to guarantee that this object is not frozen. If a frozen string is passed in, `to_s` is a no-op and returned the same frozen string. So we need to make a new unfrozen string to allow this to work. Also this could have had interesting side effects if the user had passed in a string while holding a reference to it, they would have found their string to be changed by the call to send_file ``` x = 'inline' send_file(some_path, disposition: x) x == 'inline' #=> false ``` --- lib/sinatra/base.rb | 2 +- test/helpers_test.rb | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/sinatra/base.rb b/lib/sinatra/base.rb index c302031df2..3ab0878a09 100644 --- a/lib/sinatra/base.rb +++ b/lib/sinatra/base.rb @@ -358,7 +358,7 @@ def content_type(type = nil, params = {}) # Set the Content-Disposition to "attachment" with the specified filename, # instructing the user agents to prompt to save. def attachment(filename = nil, disposition = :attachment) - response['Content-Disposition'] = disposition.to_s + response['Content-Disposition'] = String.new(disposition.to_s) if filename params = '; filename="%s"' % File.basename(filename) response['Content-Disposition'] << params diff --git a/test/helpers_test.rb b/test/helpers_test.rb index 3575e945cd..86d6d0a438 100644 --- a/test/helpers_test.rb +++ b/test/helpers_test.rb @@ -879,6 +879,12 @@ def send_file_app(opts={}) assert_equal 'inline; filename="file.txt"', response['Content-Disposition'] end + it "does not raise an error when :disposition set to a frozen string" do + send_file_app :disposition => 'inline'.freeze + get '/file.txt' + assert_equal 'inline; filename="file.txt"', response['Content-Disposition'] + end + it "sets the Content-Disposition header when :filename provided" do send_file_app :filename => 'foo.txt' get '/file.txt' From 0fd085ca41c32b7c579ea49989e4a46a78c9492a Mon Sep 17 00:00:00 2001 From: Andrew Selder Date: Tue, 19 Jul 2016 20:32:05 -0700 Subject: [PATCH 2/2] Use `dup` instead of String.new --- lib/sinatra/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/sinatra/base.rb b/lib/sinatra/base.rb index 3ab0878a09..168c34149f 100644 --- a/lib/sinatra/base.rb +++ b/lib/sinatra/base.rb @@ -358,7 +358,7 @@ def content_type(type = nil, params = {}) # Set the Content-Disposition to "attachment" with the specified filename, # instructing the user agents to prompt to save. def attachment(filename = nil, disposition = :attachment) - response['Content-Disposition'] = String.new(disposition.to_s) + response['Content-Disposition'] = disposition.to_s.dup if filename params = '; filename="%s"' % File.basename(filename) response['Content-Disposition'] << params