Skip to content
This repository has been archived by the owner on Aug 23, 2022. It is now read-only.

Commit

Permalink
Makes send_file work again by deferring to Rack::Sendfile.
Browse files Browse the repository at this point in the history
  * Add the Rack::Sendfile middleware
  * Make the header to use configurable via config.action_dispatch.x_sendfile_header (default to "X-Sendfile"). 
  * Add Railties tests to confirm that these work
  * Remove the :stream, :buffer_size, and :x_senfile default options to send_file
  * Change the log subscriber to always say "Sent file"
  * Add deprecation warnings for options that are now no-ops

Note that servers can configure this by setting X-Sendfile-Type. Hosting companies and those creating packages of servers specially designed for Rails applications are encouraged to specify this header so that this can work transparently.
  • Loading branch information
Carlhuda committed Feb 24, 2010
1 parent a73f682 commit 5e2bd08
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 72 deletions.
57 changes: 19 additions & 38 deletions actionpack/lib/action_controller/metal/streaming.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,13 @@ module Streaming
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
Expand All @@ -31,24 +26,12 @@ module Streaming
# 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
Expand Down Expand Up @@ -79,23 +62,18 @@ module Streaming
# 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
Expand Down Expand Up @@ -130,32 +108,35 @@ def send_file(path, options = {}) #:doc:
# 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'
)
Expand Down
10 changes: 1 addition & 9 deletions actionpack/lib/action_controller/railties/log_subscriber.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,7 @@ def process_action(event)
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
Expand Down
2 changes: 2 additions & 0 deletions actionpack/lib/action_dispatch/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions actionpack/test/controller/log_subscriber_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,11 @@ def test_send_file
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

Expand Down
23 changes: 0 additions & 23 deletions actionpack/test/controller/send_file_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,18 +74,6 @@ def test_file_url_based_filename
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') }
Expand All @@ -106,7 +94,6 @@ def test_headers_after_send_shouldnt_include_charset
# 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'
Expand All @@ -121,7 +108,6 @@ def test_send_file_headers_bang
@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']
Expand All @@ -134,7 +120,6 @@ def test_send_file_headers_bang

def test_send_file_headers_with_mime_lookup_with_symbol
options = {
:length => 1,
:type => :png
}

Expand All @@ -147,7 +132,6 @@ def test_send_file_headers_with_mime_lookup_with_symbol

def test_send_file_headers_with_bad_symbol
options = {
:length => 1,
:type => :this_type_is_not_registered
}

Expand All @@ -174,11 +158,4 @@ def test_send_file_headers_with_bad_symbol
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
1 change: 1 addition & 0 deletions railties/lib/rails/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ def middleware
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 })
Expand Down
55 changes: 55 additions & 0 deletions railties/test/application/configuration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,5 +171,60 @@ def teardown
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

0 comments on commit 5e2bd08

Please sign in to comment.