diff options
Diffstat (limited to 'actionpack/lib/action_dispatch')
27 files changed, 328 insertions, 218 deletions
diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index 6610315da7..25edd196c3 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -72,7 +72,7 @@ module ActionDispatch end end - # Convert nested Hash to HashWithIndifferentAccess + # Convert nested Hash to ActiveSupport::HashWithIndifferentAccess def normalize_parameters(value) case value when Hash diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index d60c8775af..452809a689 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -1,9 +1,5 @@ -require 'tempfile' require 'stringio' -require 'strscan' -require 'active_support/core_ext/hash/indifferent_access' -require 'active_support/core_ext/string/access' require 'active_support/inflector' require 'action_dispatch/http/headers' require 'action_controller/metal/exceptions' diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb index 79437d6e85..8a97248eb3 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -19,7 +19,7 @@ module ActionDispatch # its interface is available directly. attr_accessor :tempfile - # TODO. + # A string with the headers of the multipart request. attr_accessor :headers def initialize(hash) # :nodoc: @@ -75,7 +75,7 @@ module ActionDispatch end module Upload # :nodoc: - # Convert nested Hash to HashWithIndifferentAccess and replace + # Convert nested Hash to ActiveSupport::HashWithIndifferentAccess and replace # file upload hash with UploadedFile objects def normalize_parameters(value) if Hash === value && value.has_key?(:tempfile) diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index bced7d84c0..43f26d696d 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -32,7 +32,11 @@ module ActionDispatch params.reject! { |_,v| v.to_param.nil? } result = build_host_url(options) - result << (options[:trailing_slash] ? path.sub(/\?|\z/) { "/" + $& } : path) + if options[:trailing_slash] && !path.ends_with?('/') + result << path.sub(/(\?|\z)/) { "/" + $& } + else + result << path + end result << "?#{params.to_query}" unless params.empty? result << "##{Journey::Router::Utils.escape_fragment(options[:anchor].to_param.to_s)}" if options[:anchor] result diff --git a/actionpack/lib/action_dispatch/journey/formatter.rb b/actionpack/lib/action_dispatch/journey/formatter.rb index 4a344f71af..cf755bfbeb 100644 --- a/actionpack/lib/action_dispatch/journey/formatter.rb +++ b/actionpack/lib/action_dispatch/journey/formatter.rb @@ -37,19 +37,16 @@ module ActionDispatch private def extract_parameterized_parts(route, options, recall, parameterize = nil) - constraints = recall.merge(options) - data = constraints.dup + parameterized_parts = recall.merge(options) keys_to_keep = route.parts.reverse.drop_while { |part| !options.key?(part) || (options[part] || recall[part]).nil? } | route.required_parts - (data.keys - keys_to_keep).each do |bad_key| - data.delete(bad_key) + (parameterized_parts.keys - keys_to_keep).each do |bad_key| + parameterized_parts.delete(bad_key) end - parameterized_parts = data.dup - if parameterize parameterized_parts.each do |k, v| parameterized_parts[k] = parameterize.call(k, v) diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index d18efd863a..d3988cf31e 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -30,11 +30,11 @@ module ActionDispatch end def ast - return @decorated_ast if @decorated_ast - - @decorated_ast = path.ast - @decorated_ast.grep(Nodes::Terminal).each { |n| n.memo = self } - @decorated_ast + @decorated_ast ||= begin + decorated_ast = path.ast + decorated_ast.grep(Nodes::Terminal).each { |n| n.memo = self } + decorated_ast + end end def requirements # :nodoc: @@ -45,7 +45,7 @@ module ActionDispatch end def segments - @path.names + path.names end def required_keys diff --git a/actionpack/lib/action_dispatch/journey/routes.rb b/actionpack/lib/action_dispatch/journey/routes.rb index 32829a1f20..a99d6d0d6a 100644 --- a/actionpack/lib/action_dispatch/journey/routes.rb +++ b/actionpack/lib/action_dispatch/journey/routes.rb @@ -16,12 +16,12 @@ module ActionDispatch end def length - @routes.length + routes.length end alias :size :length def last - @routes.last + routes.last end def each(&block) @@ -33,24 +33,23 @@ module ActionDispatch end def partitioned_routes - @partitioned_routes ||= routes.partition { |r| - r.path.anchored && r.ast.grep(Nodes::Symbol).all? { |n| n.default_regexp? } - } + @partitioned_routes ||= routes.partition do |r| + r.path.anchored && r.ast.grep(Nodes::Symbol).all?(&:default_regexp?) + end end def ast - return @ast if @ast - return if partitioned_routes.first.empty? - - asts = partitioned_routes.first.map { |r| r.ast } - @ast = Nodes::Or.new(asts) + @ast ||= begin + asts = partitioned_routes.first.map(&:ast) + Nodes::Or.new(asts) unless asts.empty? + end end def simulator - return @simulator if @simulator - - gtg = GTG::Builder.new(ast).transition_table - @simulator = GTG::Simulator.new(gtg) + @simulator ||= begin + gtg = GTG::Builder.new(ast).transition_table + GTG::Simulator.new(gtg) + end end # Add a route to the routing table. diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index ac8f6af7fe..64230ff1ae 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -2,7 +2,6 @@ require 'action_dispatch/http/request' require 'action_dispatch/middleware/exception_wrapper' require 'action_dispatch/routing/inspector' - module ActionDispatch # This middleware is responsible for logging exceptions and # showing a debugging page in case the request is local. @@ -15,18 +14,17 @@ module ActionDispatch end def call(env) - begin - response = (_, headers, body = @app.call(env)) - - if headers['X-Cascade'] == 'pass' - body.close if body.respond_to?(:close) - raise ActionController::RoutingError, "No route matches [#{env['REQUEST_METHOD']}] #{env['PATH_INFO'].inspect}" - end - rescue Exception => exception - raise exception if env['action_dispatch.show_exceptions'] == false + _, headers, body = response = @app.call(env) + + if headers['X-Cascade'] == 'pass' + body.close if body.respond_to?(:close) + raise ActionController::RoutingError, "No route matches [#{env['REQUEST_METHOD']}] #{env['PATH_INFO'].inspect}" end - exception ? render_exception(env, exception) : response + response + rescue Exception => exception + raise exception if env['action_dispatch.show_exceptions'] == false + render_exception(env, exception) end private @@ -42,12 +40,11 @@ module ActionDispatch :application_trace => wrapper.application_trace, :framework_trace => wrapper.framework_trace, :full_trace => wrapper.full_trace, - :routes => formatted_routes(exception), + :routes_inspector => routes_inspector(exception), :source_extract => wrapper.source_extract, :line_number => wrapper.line_number, :file => wrapper.file ) - file = "rescues/#{wrapper.rescue_template}" body = template.render(:template => file, :layout => 'rescues/layout') render(wrapper.status_code, body) @@ -85,11 +82,9 @@ module ActionDispatch @stderr_logger ||= ActiveSupport::Logger.new($stderr) end - def formatted_routes(exception) - return false unless @routes_app.respond_to?(:routes) - if exception.is_a?(ActionController::RoutingError) || exception.is_a?(ActionView::Template::Error) - inspector = ActionDispatch::Routing::RoutesInspector.new - inspector.collect_routes(@routes_app.routes.routes) + def routes_inspector(exception) + if @routes_app.respond_to?(:routes) && (exception.is_a?(ActionController::RoutingError) || exception.is_a?(ActionView::Template::Error)) + ActionDispatch::Routing::RoutesInspector.new(@routes_app.routes.routes) end end end diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index ee69c8b49d..869d0aa7af 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -1,5 +1,4 @@ require 'action_controller/metal/exceptions' -require 'active_support/core_ext/exception' require 'active_support/core_ext/class/attribute_accessors' module ActionDispatch @@ -97,7 +96,7 @@ module ActionDispatch if File.exists?(full_path) File.open(full_path, "r") do |file| start = [line - 3, 0].max - lines = file.lines.drop(start).take(6) + lines = file.each_line.drop(start).take(6) Hash[*(start+1..(lines.count+start)).zip(lines).flatten] end end diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 5abf8f2802..4e36c9bb49 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -1,23 +1,59 @@ module ActionDispatch + # This middleware calculates the IP address of the remote client that is + # making the request. It does this by checking various headers that could + # contain the address, and then picking the last-set address that is not + # on the list of trusted IPs. This follows the precendent set by e.g. + # {the Tomcat server}[https://issues.apache.org/bugzilla/show_bug.cgi?id=50453], + # with {reasoning explained at length}[http://blog.gingerlime.com/2012/rails-ip-spoofing-vulnerabilities-and-protection] + # by @gingerlime. A more detailed explanation of the algorithm is given + # at GetIp#calculate_ip. + # + # Some Rack servers concatenate repeated headers, like {HTTP RFC 2616}[http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2] + # requires. Some Rack servers simply drop preceeding headers, and only report + # the value that was {given in the last header}[http://andre.arko.net/2011/12/26/repeated-headers-and-ruby-web-servers]. + # If you are behind multiple proxy servers (like Nginx to HAProxy to Unicorn) + # then you should test your Rack server to make sure your data is good. + # + # IF YOU DON'T USE A PROXY, THIS MAKES YOU VULNERABLE TO IP SPOOFING. + # This middleware assumes that there is at least one proxy sitting around + # and setting headers with the client's remote IP address. If you don't use + # a proxy, because you are hosted on e.g. Heroku without SSL, any client can + # claim to have any IP address by setting the X-Forwarded-For header. If you + # care about that, then you need to explicitly drop or ignore those headers + # sometime before this middleware runs. class RemoteIp - class IpSpoofAttackError < StandardError ; end + class IpSpoofAttackError < StandardError; end - # IP addresses that are "trusted proxies" that can be stripped from - # the comma-delimited list in the X-Forwarded-For header. See also: - # http://en.wikipedia.org/wiki/Private_network#Private_IPv4_address_spaces - # http://en.wikipedia.org/wiki/Private_network#Private_IPv6_addresses. + # The default trusted IPs list simply includes IP addresses that are + # guaranteed by the IP specification to be private addresses. Those will + # not be the ultimate client IP in production, and so are discarded. See + # http://en.wikipedia.org/wiki/Private_network for details. TRUSTED_PROXIES = %r{ - ^127\.0\.0\.1$ | # localhost - ^::1$ | - ^(10 | # private IP 10.x.x.x - 172\.(1[6-9]|2[0-9]|3[0-1]) | # private IP in the range 172.16.0.0 .. 172.31.255.255 - 192\.168 | # private IP 192.168.x.x - fc00:: # private IP fc00 - )\. + ^127\.0\.0\.1$ | # localhost IPv4 + ^::1$ | # localhost IPv6 + ^fc00: | # private IPv6 range fc00 + ^10\. | # private IPv4 range 10.x.x.x + ^172\.(1[6-9]|2[0-9]|3[0-1])\.| # private IPv4 range 172.16.0.0 .. 172.31.255.255 + ^192\.168\. # private IPv4 range 192.168.x.x }x attr_reader :check_ip, :proxies + # Create a new +RemoteIp+ middleware instance. + # + # The +check_ip_spoofing+ option is on by default. When on, an exception + # is raised if it looks like the client is trying to lie about its own IP + # address. It makes sense to turn off this check on sites aimed at non-IP + # clients (like WAP devices), or behind proxies that set headers in an + # incorrect or confusing way (like AWS ELB). + # + # The +custom_trusted+ argument can take a regex, which will be used + # instead of +TRUSTED_PROXIES+, or a string, which will be used in addition + # to +TRUSTED_PROXIES+. Any proxy setup will put the value you want in the + # middle (or at the beginning) of the X-Forwarded-For list, with your proxy + # servers after it. If your proxies aren't removed, pass them in via the + # +custom_trusted+ parameter. That way, the middleware will ignore those + # IP addresses, and return the one that you want. def initialize(app, check_ip_spoofing = true, custom_proxies = nil) @app = app @check_ip = check_ip_spoofing @@ -31,15 +67,23 @@ module ActionDispatch end end + # Since the IP address may not be needed, we store the object here + # without calculating the IP to keep from slowing down the majority of + # requests. For those requests that do need to know the IP, the + # GetIp#calculate_ip method will calculate the memoized client IP address. def call(env) env["action_dispatch.remote_ip"] = GetIp.new(env, self) @app.call(env) end + # The GetIp class exists as a way to defer processing of the request data + # into an actual IP address. If the ActionDispatch::Request#remote_ip method + # is called, this class will calculate the value and then memoize it. class GetIp - # IP v4 and v6 (with compression) validation regexp - # https://gist.github.com/1289635 + # This constant contains a regular expression that validates every known + # form of IP v4 and v6 address, with or without abbreviations, adapted + # from {this gist}[https://gist.github.com/1289635]. VALID_IP = %r{ (^(25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[0-9]{1,2})(\.(25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[0-9]{1,2})){3}$) | # ip v4 (^( @@ -63,62 +107,78 @@ module ActionDispatch }x def initialize(env, middleware) - @env = env - @middleware = middleware - @ip = nil + @env = env + @check_ip = middleware.check_ip + @proxies = middleware.proxies end - # Determines originating IP address. REMOTE_ADDR is the standard - # but will be wrong if the user is behind a proxy. Proxies will set - # HTTP_CLIENT_IP and/or HTTP_X_FORWARDED_FOR, so we prioritize those. - # HTTP_X_FORWARDED_FOR may be a comma-delimited list in the case of - # multiple chained proxies. The first address which is in this list - # if it's not a known proxy will be the originating IP. - # Format of HTTP_X_FORWARDED_FOR: - # client_ip, proxy_ip1, proxy_ip2... - # http://en.wikipedia.org/wiki/X-Forwarded-For + # Sort through the various IP address headers, looking for the IP most + # likely to be the address of the actual remote client making this + # request. + # + # REMOTE_ADDR will be correct if the request is made directly against the + # Ruby process, on e.g. Heroku. When the request is proxied by another + # server like HAProxy or Nginx, the IP address that made the original + # request will be put in an X-Forwarded-For header. If there are multiple + # proxies, that header may contain a list of IPs. Other proxy services + # set the Client-Ip header instead, so we check that too. + # + # As discussed in {this post about Rails IP Spoofing}[http://blog.gingerlime.com/2012/rails-ip-spoofing-vulnerabilities-and-protection/], + # while the first IP in the list is likely to be the "originating" IP, + # it could also have been set by the client maliciously. + # + # In order to find the first address that is (probably) accurate, we + # take the list of IPs, remove known and trusted proxies, and then take + # the last address left, which was presumably set by one of those proxies. def calculate_ip - client_ip = @env['HTTP_CLIENT_IP'] - forwarded_ip = ips_from('HTTP_X_FORWARDED_FOR').first - remote_addrs = ips_from('REMOTE_ADDR') - - check_ip = client_ip && @middleware.check_ip - if check_ip && forwarded_ip != client_ip + # Set by the Rack web server, this is a single value. + remote_addr = ips_from('REMOTE_ADDR').last + + # Could be a CSV list and/or repeated headers that were concatenated. + client_ips = ips_from('HTTP_CLIENT_IP').reverse + forwarded_ips = ips_from('HTTP_X_FORWARDED_FOR').reverse + + # +Client-Ip+ and +X-Forwarded-For+ should not, generally, both be set. + # If they are both set, it means that this request passed through two + # proxies with incompatible IP header conventions, and there is no way + # for us to determine which header is the right one after the fact. + # Since we have no idea, we give up and explode. + should_check_ip = @check_ip && client_ips.last + if should_check_ip && !forwarded_ips.include?(client_ips.last) # We don't know which came from the proxy, and which from the user - raise IpSpoofAttackError, "IP spoofing attack?!" \ - "HTTP_CLIENT_IP=#{@env['HTTP_CLIENT_IP'].inspect}" \ + raise IpSpoofAttackError, "IP spoofing attack?! " + + "HTTP_CLIENT_IP=#{@env['HTTP_CLIENT_IP'].inspect} " + "HTTP_X_FORWARDED_FOR=#{@env['HTTP_X_FORWARDED_FOR'].inspect}" end - client_ips = remove_proxies [client_ip, forwarded_ip, remote_addrs].flatten - if client_ips.present? - client_ips.first - else - # If there is no client ip we can return first valid proxy ip from REMOTE_ADDR - remote_addrs.find { |ip| valid_ip? ip } - end + # We assume these things about the IP headers: + # + # - X-Forwarded-For will be a list of IPs, one per proxy, or blank + # - Client-Ip is propagated from the outermost proxy, or is blank + # - REMOTE_ADDR will be the IP that made the request to Rack + ips = [forwarded_ips, client_ips, remote_addr].flatten.compact + + # If every single IP option is in the trusted list, just return REMOTE_ADDR + filter_proxies(ips).first || remote_addr end + # Memoizes the value returned by #calculate_ip and returns it for + # ActionDispatch::Request to use. def to_s @ip ||= calculate_ip end - private + protected def ips_from(header) - @env[header] ? @env[header].strip.split(/[,\s]+/) : [] - end - - def valid_ip?(ip) - ip =~ VALID_IP - end - - def not_a_proxy?(ip) - ip !~ @middleware.proxies + # Split the comma-separated list into an array of strings + ips = @env[header] ? @env[header].strip.split(/[,\s]+/) : [] + # Only return IPs that are valid according to the regex + ips.select{ |ip| ip =~ VALID_IP } end - def remove_proxies(ips) - ips.select { |ip| valid_ip?(ip) && not_a_proxy?(ip) } + def filter_proxies(ips) + ips.reject { |ip| ip =~ @proxies } end end diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb index 7c12590c49..6fb16bdfe9 100644 --- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb @@ -21,6 +21,8 @@ module ActionDispatch module Compatibility def initialize(app, options = {}) options[:key] ||= '_session_id' + # FIXME Rack's secret is not being used + options[:secret] ||= SecureRandom.hex(30) super end diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index ce5f89ee5b..4437b50f1f 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -21,15 +21,12 @@ module ActionDispatch # # Session options: # - # * <tt>:secret</tt>: An application-wide key string or block returning a - # string called per generated digest. The block is called with the - # CGI::Session instance as an argument. It's important that the secret - # is not vulnerable to a dictionary attack. Therefore, you should choose - # a secret consisting of random numbers and letters and more than 30 - # characters. + # * <tt>:secret</tt>: An application-wide key string. It's important that + # the secret is not vulnerable to a dictionary attack. Therefore, you + # should choose a secret consisting of random numbers and letters and + # more than 30 characters. # # secret: '449fe2e7daee471bffae2fd8dc02313d' - # secret: Proc.new { User.current_user.secret_key } # # * <tt>:digest</tt>: The message digest algorithm used to verify session # integrity defaults to 'SHA1' but may be any digest provided by OpenSSL, diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index 2b37a8d026..fcc5bc12c4 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -16,9 +16,9 @@ module ActionDispatch # catches the exceptions and returns a FAILSAFE_RESPONSE. class ShowExceptions FAILSAFE_RESPONSE = [500, { 'Content-Type' => 'text/plain' }, - ["500 Internal Server Error\n" << - "If you are the administrator of this website, then please read this web " << - "application's log file and/or the web server's log file to find out what " << + ["500 Internal Server Error\n" \ + "If you are the administrator of this website, then please read this web " \ + "application's log file and/or the web server's log file to find out what " \ "went wrong."]] def initialize(app, exceptions_app) @@ -27,13 +27,10 @@ module ActionDispatch end def call(env) - begin - response = @app.call(env) - rescue Exception => exception - raise exception if env['action_dispatch.show_exceptions'] == false - end - - response || render_exception(env, exception) + @app.call(env) + rescue Exception => exception + raise exception if env['action_dispatch.show_exceptions'] == false + render_exception(env, exception) end private diff --git a/actionpack/lib/action_dispatch/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb index 9098f4e170..9e03cbf2b7 100644 --- a/actionpack/lib/action_dispatch/middleware/ssl.rb +++ b/actionpack/lib/action_dispatch/middleware/ssl.rb @@ -45,7 +45,7 @@ module ActionDispatch # http://tools.ietf.org/html/draft-hodges-strict-transport-sec-02 def hsts_headers if @hsts - value = "max-age=#{@hsts[:expires]}" + value = "max-age=#{@hsts[:expires].to_i}" value += "; includeSubDomains" if @hsts[:subdomains] { 'Strict-Transport-Security' => value } else diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index e3b15b43b9..1c9512ad01 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -6,7 +6,7 @@ module ActionDispatch def initialize(root, cache_control) @root = root.chomp('/') @compiled_root = /^#{Regexp.escape(root)}/ - @file_server = ::Rack::File.new(@root, cache_control) + @file_server = ::Rack::File.new(@root, 'Cache-Control' => cache_control) end def match?(path) diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb index 55079848bd..ab24118f3e 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb @@ -1,8 +1,8 @@ <% unless @exception.blamed_files.blank? %> <% if (hide = @exception.blamed_files.length > 8) %> - <a href="#" onclick="document.getElementById('blame_trace').style.display='block'; return false;">Show blamed files</a> + <a href="#" onclick="toggleTrace()">Toggle blamed files</a> <% end %> - <pre id="blame_trace" <%='style="display:none"' if hide %>><code><%=h @exception.describe_blame %></code></pre> + <pre id="blame_trace" <%='style="display:none"' if hide %>><code><%= @exception.describe_blame %></code></pre> <% end %> <% @@ -18,17 +18,17 @@ %> <h2 style="margin-top: 30px">Request</h2> -<p><b>Parameters</b>: <pre><%=h request_dump %></pre></p> +<p><b>Parameters</b>:</p> <pre><%= request_dump %></pre> <div class="details"> - <div class="summary"><a href="#" onclick="document.getElementById('session_dump').style.display='block'; return false;">Show session dump</a></div> - <div id="session_dump" style="display:none"><p><pre><%= debug_hash @request.session %></pre></p></div> + <div class="summary"><a href="#" onclick="toggleSessionDump()">Toggle session dump</a></div> + <div id="session_dump" style="display:none"><pre><%= debug_hash @request.session %></pre></div> </div> <div class="details"> - <div class="summary"><a href="#" onclick="document.getElementById('env_dump').style.display='block'; return false;">Show env dump</a></div> - <div id="env_dump" style="display:none"><p><pre><%= debug_hash @request.env.slice(*@request.class::ENV_METHODS) %></pre></p></div> + <div class="summary"><a href="#" onclick="toggleEnvDump()">Toggle env dump</a></div> + <div id="env_dump" style="display:none"><pre><%= debug_hash @request.env.slice(*@request.class::ENV_METHODS) %></pre></div> </div> <h2 style="margin-top: 30px">Response</h2> -<p><b>Headers</b>: <pre><%=h defined?(@response) ? @response.headers.inspect.gsub(',', ",\n") : 'None' %></pre></p> +<p><b>Headers</b>:</p> <pre><%= defined?(@response) ? @response.headers.inspect.gsub(',', ",\n") : 'None' %></pre> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb index 8771b5fd6d..9d947aea40 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb @@ -12,15 +12,15 @@ <div id="traces"> <% names.each do |name| %> <% - show = "document.getElementById('#{name.gsub(/\s/, '-')}').style.display='block';" - hide = (names - [name]).collect {|hide_name| "document.getElementById('#{hide_name.gsub(/\s/, '-')}').style.display='none';"} + show = "show('#{name.gsub(/\s/, '-')}');" + hide = (names - [name]).collect {|hide_name| "hide('#{hide_name.gsub(/\s/, '-')}');"} %> <a href="#" onclick="<%= hide.join %><%= show %>; return false;"><%= name %></a> <%= '|' unless names.last == name %> <% end %> <% traces.each do |name, trace| %> <div id="<%= name.gsub(/\s/, '-') %>" style="display: <%= (name == "Application Trace") ? 'block' : 'none' %>;"> - <pre><code><%=h trace.join "\n" %></code></pre> + <pre><code><%= trace.join "\n" %></code></pre> </div> <% end %> </div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb index 1c6b5010a3..57a2940802 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb @@ -1,14 +1,14 @@ <header> <h1> - <%=h @exception.class.to_s %> + <%= @exception.class.to_s %> <% if @request.parameters['controller'] %> - in <%=h @request.parameters['controller'].camelize %>Controller<% if @request.parameters['action'] %>#<%=h @request.parameters['action'] %><% end %> + in <%= @request.parameters['controller'].camelize %>Controller<% if @request.parameters['action'] %>#<%= @request.parameters['action'] %><% end %> <% end %> </h1> </header> <div id="container"> - <h2><%=h @exception.message %></h2> + <h2><%= @exception.message %></h2> <%= render template: "rescues/_source" %> <%= render template: "rescues/_trace" %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb index bcab5959a0..9878c2747e 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb @@ -4,7 +4,11 @@ <meta charset="utf-8" /> <title>Action Controller: Exception caught</title> <style> - body { background-color: #fff; color: #333; margin: 0px} + body { + background-color: #FAFAFA; + color: #333; + margin: 0px; + } body, p, ol, ul, td { font-family: helvetica, verdana, arial, sans-serif; @@ -18,31 +22,25 @@ } pre.box { - border: #eee solid 1px; + border: 1px solid #EEE; padding: 10px; margin: 0px; width: 958px; } header { - background: whiteSmoke; - filter: progid:DXImageTransform.Microsoft.gradient(GradientType=0,startColorstr='#980905',endColorstr='#c52f24'); - background: -webkit-gradient(linear,0% 0,0% 100%,from(#980905),to(#C52F24)); - background: -moz-linear-gradient(270deg,#980905,#C52F24); - color: #fff; - padding: 0.5em; + color: #F0F0F0; + background: #C52F24; + padding: 0.5em 1.5em; } h2 { color: #C52F24; - padding: 2px; line-height: 25px; } .details { - border: 1px solid #E5E5E5; - -webkit-border-radius: 4px; - -moz-border-radius: 4px; + border: 1px solid #D0D0D0; border-radius: 4px; margin: 1em 0px; display: block; @@ -51,7 +49,7 @@ .summary { padding: 8px 15px; - border-bottom: 1px solid #E5E5E5; + border-bottom: 1px solid #D0D0D0; display: block; } @@ -61,8 +59,9 @@ } #container { - margin: auto; - width: 98%; + box-sizing: border-box; + width: 100%; + padding: 0 1.5em; } .source * { @@ -84,7 +83,7 @@ .source .data { font-size: 80%; overflow: auto; - background-color: #fff; + background-color: #FFF; } .info { @@ -99,8 +98,12 @@ text-align: right; } + .line { + padding-left: 10px; + } + .line:hover { - background-color: #f6f6f6; + background-color: #F6F6F6; } .line.active { @@ -109,8 +112,32 @@ a { color: #980905; } a:visited { color: #666; } - a:hover { color: #C52F24;} + a:hover { color: #C52F24; } + + <%= yield :style %> </style> + + <script> + var toggle = function(id) { + var s = document.getElementById(id).style; + s.display = s.display == 'none' ? 'block' : 'none'; + } + var show = function(id) { + document.getElementById(id).style.display = 'block'; + } + var hide = function(id) { + document.getElementById(id).style.display = 'none'; + } + var toggleTrace = function() { + toggle('blame_trace'); + } + var toggleSessionDump = function() { + toggle('session_dump'); + } + var toggleEnvDump = function() { + toggle('env_dump'); + } + </script> </head> <body> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb index c5917b9acb..ca14215946 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb @@ -3,5 +3,5 @@ </header> <div id="container"> - <h2><%=h @exception.message %></h2> + <h2><%= @exception.message %></h2> </div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb index ca85e6d048..61690d3e50 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb @@ -2,27 +2,29 @@ <h1>Routing Error</h1> </header> <div id="container"> - <h2><%=h @exception.message %></h2> + <h2><%= @exception.message %></h2> <% unless @exception.failures.empty? %> <p> <h2>Failure reasons:</h2> <ol> <% @exception.failures.each do |route, reason| %> - <li><code><%=h route.inspect.gsub('\\', '') %></code> failed because <%=h reason.downcase %></li> + <li><code><%= route.inspect.gsub('\\', '') %></code> failed because <%= reason.downcase %></li> <% end %> </ol> </p> <% end %> + <%= render template: "rescues/_trace" %> - <h2> - Routes - </h2> + <% if @routes_inspector %> + <h2> + Routes + </h2> - <p> - Routes match in priority from top to bottom - </p> + <p> + Routes match in priority from top to bottom + </p> -<%= render layout: "routes/route_wrapper" do %> - <%= render partial: "routes/route", collection: @routes %> -<% end %> + <%= @routes_inspector.format(ActionDispatch::Routing::HtmlTableFormatter.new(self)) %> + <% end %> +</div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb index 01faf5a475..63216ef7c5 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb @@ -1,20 +1,20 @@ <% @source_extract = @exception.source_extract(0, :html) %> <header> <h1> - <%=h @exception.original_exception.class.to_s %> in - <%=h @request.parameters["controller"].capitalize if @request.parameters["controller"]%>#<%=h @request.parameters["action"] %> + <%= @exception.original_exception.class.to_s %> in + <%= @request.parameters["controller"].capitalize if @request.parameters["controller"]%>#<%= @request.parameters["action"] %> </h1> </header> <div id="container"> <p> - Showing <i><%=h @exception.file_name %></i> where line <b>#<%=h @exception.line_number %></b> raised: - <pre><code><%=h @exception.message %></code></pre> - </p> + Showing <i><%= @exception.file_name %></i> where line <b>#<%= @exception.line_number %></b> raised: + </p> + <pre><code><%= @exception.message %></code></pre> <div class="source"> <div class="info"> - <p>Extracted source (around line <strong>#<%=h @exception.line_number %></strong>): + <p>Extracted source (around line <strong>#<%= @exception.line_number %></strong>):</p> </div> <div class="data"> <table cellpadding="0" cellspacing="0" class="lines"> @@ -36,9 +36,8 @@ </div> </div> - <p><%=h @exception.sub_template_message %></p> + <p><%= @exception.sub_template_message %></p> <%= render template: "rescues/_trace" %> <%= render template: "rescues/_request_and_response" %> - </div> </div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb index 65fc34df90..c1fbf67eed 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb @@ -2,5 +2,5 @@ <h1>Unknown action</h1> </header> <div id="container"> - <h2><%=h @exception.message %></h2> - </div> + <h2><%= @exception.message %></h2> +</div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/routes/_route_wrapper.html.erb b/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb index dc17cb77ef..9026c4eeb2 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/routes/_route_wrapper.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb @@ -1,7 +1,7 @@ -<style type='text/css'> - #route_table td {padding: 0 30px;} - #route_table {margin: 0 auto 0;} -</style> +<% content_for :style do %> + #route_table td { padding: 0 30px; } + #route_table { margin: 0 auto 0; } +<% end %> <table id='route_table' class='route_table'> <thead> diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index 73b2b4ac1d..ea3e8357d4 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -61,25 +61,39 @@ module ActionDispatch ## # This class is just used for displaying route information when someone - # executes `rake routes`. People should not use this class. + # executes `rake routes` or looks at the RoutingError page. + # People should not use this class. class RoutesInspector # :nodoc: - def initialize - @engines = Hash.new + def initialize(routes) + @engines = {} + @routes = routes end - def format(all_routes, filter = nil) - if filter - all_routes = all_routes.select{ |route| route.defaults[:controller] == filter } + def format(formatter, filter = nil) + routes_to_display = filter_routes(filter) + + routes = collect_routes(routes_to_display) + formatter.section :application, 'Application routes', routes + + @engines.each do |name, engine_routes| + formatter.section :engine, "Routes for #{name}", engine_routes end - routes = collect_routes(all_routes) + formatter.result + end + + private - formatted_routes(routes) + - formatted_routes_for_engines + def filter_routes(filter) + if filter + @routes.select { |route| route.defaults[:controller] == filter } + else + @routes + end end def collect_routes(routes) - routes = routes.collect do |route| + routes.collect do |route| RouteWrapper.new(route) end.reject do |route| route.internal? @@ -100,21 +114,49 @@ module ActionDispatch @engines[name] = collect_routes(routes.routes) end end + end - def formatted_routes_for_engines - @engines.map do |name, routes| - ["\nRoutes for #{name}:"] + formatted_routes(routes) - end.flatten + class ConsoleFormatter + def initialize + @buffer = [] end - def formatted_routes(routes) - name_width = routes.map{ |r| r[:name].length }.max - verb_width = routes.map{ |r| r[:verb].length }.max - path_width = routes.map{ |r| r[:path].length }.max + def result + @buffer.join("\n") + end - routes.map do |r| - "#{r[:name].rjust(name_width)} #{r[:verb].ljust(verb_width)} #{r[:path].ljust(path_width)} #{r[:reqs]}" + def section(type, title, routes) + @buffer << "\n#{title}:" unless type == :application + @buffer << draw_section(routes) + end + + private + def draw_section(routes) + name_width = routes.map { |r| r[:name].length }.max + verb_width = routes.map { |r| r[:verb].length }.max + path_width = routes.map { |r| r[:path].length }.max + + routes.map do |r| + "#{r[:name].rjust(name_width)} #{r[:verb].ljust(verb_width)} #{r[:path].ljust(path_width)} #{r[:reqs]}" + end end + end + + class HtmlTableFormatter + def initialize(view) + @view = view + @buffer = [] + end + + def section(type, title, routes) + @buffer << %(<tr><th colspan="4">#{title}</th></tr>) + @buffer << @view.render(partial: "routes/route", collection: routes) + end + + def result + @view.raw @view.render(layout: "routes/table") { + @view.raw @buffer.join("\n") + } end end end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 0dc6403ec0..a21383e091 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -26,15 +26,10 @@ module ActionDispatch def matches?(env) req = @request.new(env) - @constraints.each { |constraint| - if constraint.respond_to?(:matches?) && !constraint.matches?(req) - return false - elsif constraint.respond_to?(:call) && !constraint.call(*constraint_args(constraint, req)) - return false - end - } - - return true + @constraints.none? do |constraint| + (constraint.respond_to?(:matches?) && !constraint.matches?(req)) || + (constraint.respond_to?(:call) && !constraint.call(*constraint_args(constraint, req))) + end ensure req.reset_parameters end diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index 79dff7d121..9210bffd1d 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -1,5 +1,6 @@ require 'uri' require 'active_support/core_ext/hash/indifferent_access' +require 'active_support/core_ext/string/access' require 'action_controller/metal/exceptions' module ActionDispatch @@ -208,11 +209,9 @@ module ActionDispatch end def fail_on(exception_class) - begin - yield - rescue exception_class => e - raise MiniTest::Assertion, e.message - end + yield + rescue exception_class => e + raise MiniTest::Assertion, e.message end end end |