This project is archived and is in readonly mode.

#4969 new
fkocherga

Fixing config.action_dispatch.x_sendfile_header default

Reported by fkocherga | June 25th, 2010 @ 03:00 PM

The problem happens if one decide to comment out in application.rb:

#config.action_dispatch.x_sendfile_header = 'X-Sendfile'

relying on the web server providing correct header values, for example for Nginx:

proxy_set_header   X-Sendfile-Type     X-Accel-Redirect;

and on Rack::Sendfile middleware handling these headers correctly.
Server's 'X-Sendfile-Type' value gets simply ignored by Rack::Sendfile if latter is initialized with empty string:

#rack/sendfile.rb
def variation(env)
  @variation ||
    env['sendfile.type'] ||
    env['HTTP_X_SENDFILE_TYPE']
end

The Rack::Sendfile also has to be fixed, however before it does the Rails app is able to work correctly just by providing nil as default.

Comments and changes to this ticket

  • fkocherga

    fkocherga June 25th, 2010 @ 03:13 PM

    Cannot attach file, some lighthouseapp problems, the patch is very simple:


    actionpack/lib/action_dispatch/railtie.rb | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)

    diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb
    index ed93211..5aa217b 100644
    --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -4,7 +4,7 @@ require "rails" module ActionDispatch class Railtie < Rails::Railtie

     config.action_dispatch = ActiveSupport::OrderedOptions.new
    
    • config.action_dispatch.x_sendfile_header = ""
    • config.action_dispatch.x_sendfile_header = nil config.action_dispatch.ip_spoofing_check = true config.action_dispatch.show_exceptions = true

    @@ -13,4 +13,3 @@ module ActionDispatch

       ActionDispatch::Callbacks.to_prepare { app.routes_reloader.execute_if_updated }
     end
    

    end -- 1.7.0.4

  • fkocherga

    fkocherga June 25th, 2010 @ 03:15 PM

    Formatted properly:

    ---
     actionpack/lib/action_dispatch/railtie.rb |    4 ++--
     1 files changed, 2 insertions(+), 2 deletions(-)
    
    diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb
    index ed93211..5aa217b 100644
    --- a/actionpack/lib/action_dispatch/railtie.rb
    +++ b/actionpack/lib/action_dispatch/railtie.rb
    @@ -4,7 +4,7 @@ require "rails"
     module ActionDispatch
       class Railtie < Rails::Railtie
         config.action_dispatch = ActiveSupport::OrderedOptions.new
    -    config.action_dispatch.x_sendfile_header = ""
    +    config.action_dispatch.x_sendfile_header = nil
         config.action_dispatch.ip_spoofing_check = true
         config.action_dispatch.show_exceptions = true
    
    @@ -13,4 +13,3 @@ module ActionDispatch
           ActionDispatch::Callbacks.to_prepare { app.routes_reloader.execute_if_updated }
         end
       end
    -- 
    1.7.0.4
    
  • Bruno Michel

    Bruno Michel June 26th, 2010 @ 05:07 PM

    • Tag changed from tagged:x-sendfile to patch, tagged:x-sendfile
  • Andrea Campi

    Andrea Campi October 16th, 2010 @ 11:24 PM

    -1, the current behavior is intended and documented (barely, in the generated application.rb):

      # If you have no front-end server that supports something like X-Sendfile,
      # just comment this out and Rails will serve the files
    

    It seems to me you can configure autodetection based on X-Sendfile-Type by setting config.action_dispatch.x_sendfile_header=nil in your environment?
    If that's the case, it should probably be documented.
    If you can confirm that's the case, I can take care of updating the documentation.

  • Bertg

    Bertg October 18th, 2010 @ 04:13 PM

    I think the default behaviour is confusing. and the patch should be applied, and have the documentation reflect this behaviour.

    I can't think of any reason why Rails should, by default, disable the auto detection of these settings.

  • Andrea Campi

    Andrea Campi October 19th, 2010 @ 10:22 PM

    @Bertg: feel free to provide a patch that changes both the code and the documentation. Note that Rails is not disabling auto-detection; the developer is, if she comments out the line as directed by the documentation.

  • Andrea Campi

    Andrea Campi October 19th, 2010 @ 10:22 PM

    • Tag changed from patch, tagged:x-sendfile to actiondispatch, patch, x-sendfile
  • bingbing

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Attachments

Pages