diff options
| -rw-r--r-- | actionpack/lib/action_controller/metal/streaming.rb | 57 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/railties/log_subscriber.rb | 10 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/railtie.rb | 2 | ||||
| -rw-r--r-- | actionpack/test/controller/log_subscriber_test.rb | 4 | ||||
| -rw-r--r-- | actionpack/test/controller/send_file_test.rb | 23 | ||||
| -rw-r--r-- | railties/lib/rails/configuration.rb | 1 | ||||
| -rw-r--r-- | railties/test/application/configuration_test.rb | 55 |
7 files changed, 80 insertions, 72 deletions
diff --git a/actionpack/lib/action_controller/metal/streaming.rb b/actionpack/lib/action_controller/metal/streaming.rb index 0d6fdafe0a..753af3dc58 100644 --- a/actionpack/lib/action_controller/metal/streaming.rb +++ b/actionpack/lib/action_controller/metal/streaming.rb @@ -9,18 +9,13 @@ module ActionController #:nodoc: DEFAULT_SEND_FILE_OPTIONS = { :type => 'application/octet-stream'.freeze, :disposition => 'attachment'.freeze, - :stream => true, - :buffer_size => 4096, - :x_sendfile => false }.freeze - X_SENDFILE_HEADER = 'X-Sendfile'.freeze - protected - # Sends the file, by default streaming it 4096 bytes at a time. This way the - # whole file doesn't need to be read into memory at once. This makes it - # feasible to send even large files. You can optionally turn off streaming - # and send the whole file at once. + # Sends the file. This uses a server-appropriate method (such as X-Sendfile) + # via the Rack::Sendfile middleware. The header to use is set via + # config.action_dispatch.x_sendfile_header, and defaults to "X-Sendfile". + # Your server can also configure this for you by setting the X-Sendfile-Type header. # # Be careful to sanitize the path parameter if it is coming from a web # page. <tt>send_file(params[:path])</tt> allows a malicious user to @@ -31,24 +26,12 @@ module ActionController #:nodoc: # Defaults to <tt>File.basename(path)</tt>. # * <tt>:type</tt> - specifies an HTTP content type. Defaults to 'application/octet-stream'. You can specify # either a string or a symbol for a registered type register with <tt>Mime::Type.register</tt>, for example :json - # * <tt>:length</tt> - used to manually override the length (in bytes) of the content that - # is going to be sent to the client. Defaults to <tt>File.size(path)</tt>. # * <tt>:disposition</tt> - specifies whether the file will be shown inline or downloaded. # Valid values are 'inline' and 'attachment' (default). - # * <tt>:stream</tt> - whether to send the file to the user agent as it is read (+true+) - # or to read the entire file before sending (+false+). Defaults to +true+. - # * <tt>:buffer_size</tt> - specifies size (in bytes) of the buffer used to stream the file. - # Defaults to 4096. # * <tt>:status</tt> - specifies the status code to send with the response. Defaults to '200 OK'. # * <tt>:url_based_filename</tt> - set to +true+ if you want the browser guess the filename from # the URL, which is necessary for i18n filenames on certain browsers # (setting <tt>:filename</tt> overrides this option). - # * <tt>:x_sendfile</tt> - uses X-Sendfile to send the file when set to +true+. This is currently - # only available with Lighttpd/Apache2 and specific modules installed and activated. Since this - # uses the web server to send the file, this may lower memory consumption on your server and - # it will not block your application for further requests. - # See http://blog.lighttpd.net/articles/2006/07/02/x-sendfile and - # http://tn123.ath.cx/mod_xsendfile/ for details. Defaults to +false+. # # The default Content-Type and Content-Disposition headers are # set to download arbitrary binary files in as many browsers as @@ -79,23 +62,18 @@ module ActionController #:nodoc: # http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9 # for the Cache-Control header spec. def send_file(path, options = {}) #:doc: - # self.response_body = File.open(path) - raise MissingFile, "Cannot read file #{path}" unless File.file?(path) and File.readable?(path) - options[:length] ||= File.size(path) options[:filename] ||= File.basename(path) unless options[:url_based_filename] send_file_headers! options - @performed_render = false - if options[:x_sendfile] - head options[:status], X_SENDFILE_HEADER => path - else - self.status = options[:status] || 200 - self.content_type = options[:content_type] if options.key?(:content_type) - self.response_body = File.open(path, "rb") + ActiveSupport::Deprecation.warn(":x_sendfile is no longer needed in send_file", caller) end + + self.status = options[:status] || 200 + self.content_type = options[:content_type] if options.key?(:content_type) + self.response_body = File.open(path, "rb") end # Sends the given binary data to the browser. This method is similar to @@ -130,32 +108,35 @@ module ActionController #:nodoc: # data to the browser, then use <tt>render :text => proc { ... }</tt> # instead. See ActionController::Base#render for more information. def send_data(data, options = {}) #:doc: - send_file_headers! options.merge(:length => data.bytesize) + send_file_headers! options.dup render options.slice(:status, :content_type).merge(:text => data) end private def send_file_headers!(options) options.update(DEFAULT_SEND_FILE_OPTIONS.merge(options)) - [:length, :type, :disposition].each do |arg| + [:type, :disposition].each do |arg| raise ArgumentError, ":#{arg} option required" if options[arg].nil? end - disposition = options[:disposition].dup || 'attachment' + if options.key?(:length) + ActiveSupport::Deprecation.warn("You do not need to provide the file's length", caller) + end - disposition <<= %(; filename="#{options[:filename]}") if options[:filename] + disposition = options[:disposition] + disposition += %(; filename="#{options[:filename]}") if options[:filename] content_type = options[:type] if content_type.is_a?(Symbol) - raise ArgumentError, "Unknown MIME type #{options[:type]}" unless Mime::EXTENSION_LOOKUP.key?(content_type.to_s) - self.content_type = Mime::Type.lookup_by_extension(content_type.to_s) + extension = Mime[content_type] + raise ArgumentError, "Unknown MIME type #{options[:type]}" unless extension + self.content_type = extension else self.content_type = content_type end headers.merge!( - 'Content-Length' => options[:length].to_s, 'Content-Disposition' => disposition, 'Content-Transfer-Encoding' => 'binary' ) diff --git a/actionpack/lib/action_controller/railties/log_subscriber.rb b/actionpack/lib/action_controller/railties/log_subscriber.rb index df9ffa1717..c2299d0b05 100644 --- a/actionpack/lib/action_controller/railties/log_subscriber.rb +++ b/actionpack/lib/action_controller/railties/log_subscriber.rb @@ -22,15 +22,7 @@ module ActionController end def send_file(event) - message = if event.payload[:x_sendfile] - header = ActionController::Streaming::X_SENDFILE_HEADER - "Sent #{header} header %s" - elsif event.payload[:stream] - "Streamed file %s" - else - "Sent file %s" - end - + message = "Sent file %s" message << " (%.1fms)" info(message % [event.payload[:path], event.duration]) end diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index 335daafc01..79e9464337 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -5,6 +5,8 @@ module ActionDispatch class Railtie < Rails::Railtie railtie_name :action_dispatch + config.action_dispatch.x_sendfile_header = "X-Sendfile" + # Prepare dispatcher callbacks and run 'prepare' callbacks initializer "action_dispatch.prepare_dispatcher" do |app| # TODO: This used to say unless defined?(Dispatcher). Find out why and fix. diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb index 0659a4520e..5a3a2dd3ea 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -137,11 +137,11 @@ class ACLogSubscriberTest < ActionController::TestCase end def test_send_xfile - get :xfile_sender + assert_deprecated { get :xfile_sender } wait assert_equal 3, logs.size - assert_match /Sent X\-Sendfile header/, logs[1] + assert_match /Sent file/, logs[1] assert_match /test\/fixtures\/company\.rb/, logs[1] end diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index ce144cc24b..30c9a65b7c 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -74,18 +74,6 @@ class SendFileTest < ActionController::TestCase assert_equal "attachment", response.headers["Content-Disposition"] end - def test_x_sendfile_header - @controller.options = { :x_sendfile => true } - - response = nil - assert_nothing_raised { response = process('file') } - assert_not_nil response - - assert_equal @controller.file_path, response.headers['X-Sendfile'] - assert response.body.blank? - assert !response.etag? - end - def test_data response = nil assert_nothing_raised { response = process('data') } @@ -106,7 +94,6 @@ class SendFileTest < ActionController::TestCase # Test that send_file_headers! is setting the correct HTTP headers. def test_send_file_headers_bang options = { - :length => 1, :type => Mime::PNG, :disposition => 'disposition', :filename => 'filename' @@ -121,7 +108,6 @@ class SendFileTest < ActionController::TestCase @controller.send(:send_file_headers!, options) h = @controller.headers - assert_equal '1', h['Content-Length'] assert_equal 'image/png', @controller.content_type assert_equal 'disposition; filename="filename"', h['Content-Disposition'] assert_equal 'binary', h['Content-Transfer-Encoding'] @@ -134,7 +120,6 @@ class SendFileTest < ActionController::TestCase def test_send_file_headers_with_mime_lookup_with_symbol options = { - :length => 1, :type => :png } @@ -147,7 +132,6 @@ class SendFileTest < ActionController::TestCase def test_send_file_headers_with_bad_symbol options = { - :length => 1, :type => :this_type_is_not_registered } @@ -174,11 +158,4 @@ class SendFileTest < ActionController::TestCase assert_equal 200, @response.status end end - - def test_send_data_content_length_header - @controller.headers = {} - @controller.options = { :type => :text, :filename => 'file_with_utf8_text' } - process('multibyte_text_data') - assert_equal '29', @controller.headers['Content-Length'] - end end diff --git a/railties/lib/rails/configuration.rb b/railties/lib/rails/configuration.rb index 50675d19b8..811c3a9fd9 100644 --- a/railties/lib/rails/configuration.rb +++ b/railties/lib/rails/configuration.rb @@ -12,6 +12,7 @@ module Rails middleware.use('::Rack::Runtime') middleware.use('::Rails::Rack::Logger') middleware.use('::ActionDispatch::ShowExceptions', lambda { Rails.application.config.consider_all_requests_local }) + middleware.use('::Rack::Sendfile', lambda { Rails.application.config.action_dispatch.x_sendfile_header }) middleware.use('::ActionDispatch::Callbacks', lambda { !Rails.application.config.cache_classes }) middleware.use('::ActionDispatch::Cookies') middleware.use(lambda { ActionController::Base.session_store }, lambda { ActionController::Base.session_options }) diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 7ca605ec23..3e03a01ff3 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -171,5 +171,60 @@ module ApplicationTests get "/" assert $prepared end + + test "config.action_dispatch.x_sendfile_header defaults to X-Sendfile" do + require "rails" + require "action_controller/railtie" + + class MyApp < Rails::Application + config.action_controller.session = { :key => "_myapp_session", :secret => "3b7cd727ee24e8444053437c36cc66c4" } + end + + MyApp.initialize! + + class ::OmgController < ActionController::Base + def index + send_file __FILE__ + end + end + + MyApp.routes.draw do + match "/" => "omg#index" + end + + require 'rack/test' + extend Rack::Test::Methods + + get "/" + assert_equal File.expand_path(__FILE__), last_response.headers["X-Sendfile"] + end + + test "config.action_dispatch.x_sendfile_header is sent to Rack::Sendfile" do + require "rails" + require "action_controller/railtie" + + class MyApp < Rails::Application + config.action_controller.session = { :key => "_myapp_session", :secret => "3b7cd727ee24e8444053437c36cc66c4" } + config.action_dispatch.x_sendfile_header = 'X-Lighttpd-Send-File' + end + + MyApp.initialize! + + class ::OmgController < ActionController::Base + def index + send_file __FILE__ + end + end + + MyApp.routes.draw do + match "/" => "omg#index" + end + + require 'rack/test' + extend Rack::Test::Methods + + get "/" + assert_equal File.expand_path(__FILE__), last_response.headers["X-Lighttpd-Send-File"] + end end end |