From bed5825f775bdf2a1af6eec9dc1f4071dbde5ead Mon Sep 17 00:00:00 2001 From: Alexey Vakhov Date: Tue, 18 Oct 2011 22:57:55 +0400 Subject: Remove superfluous assignment in cookies --- actionpack/lib/action_dispatch/middleware/cookies.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 8c4615c0c1..a4ffd40a66 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -174,7 +174,7 @@ module ActionDispatch options = { :value => value } end - value = @cookies[key.to_s] = value + @cookies[key.to_s] = value handle_options(options) -- cgit v1.2.3 From afde6fdd5ef3e6b0693a7e330777e85ef4cffddb Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Wed, 19 Oct 2011 12:59:33 -0500 Subject: Added X-Request-Id tracking and TaggedLogging to easily log that and other production concerns --- actionpack/lib/action_dispatch/http/request.rb | 10 ++++++ .../lib/action_dispatch/middleware/request_id.rb | 38 ++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 actionpack/lib/action_dispatch/middleware/request_id.rb (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 37d0a3e0b8..7a5237dcf3 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -177,6 +177,16 @@ module ActionDispatch @remote_ip ||= (@env["action_dispatch.remote_ip"] || ip).to_s end + # Returns the unique request id, which is based off either the X-Request-Id header that can + # be generated by a firewall, load balancer, or web server or by the RequestId middleware + # (which sets the action_dispatch.request_id environment variable). + # + # This unique ID is useful for tracing a request from end-to-end as part of logging or debugging. + # This relies on the rack variable set by the ActionDispatch::RequestId middleware. + def uuid + @uuid ||= env["action_dispatch.request_id"] + end + # Returns the lowercase name of the HTTP server software. def server_software (@env['SERVER_SOFTWARE'] && /^([a-zA-Z]+)/ =~ @env['SERVER_SOFTWARE']) ? $1.downcase : nil diff --git a/actionpack/lib/action_dispatch/middleware/request_id.rb b/actionpack/lib/action_dispatch/middleware/request_id.rb new file mode 100644 index 0000000000..968ad6c28d --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/request_id.rb @@ -0,0 +1,38 @@ +require 'digest/md5' + +module ActionDispatch + # Makes a unique request id available to the action_dispatch.request_id env variable (which is then accessible through + # ActionDispatch::Request#uuid) and sends the same id to the client via the X-Request-Id header. + # + # The unique request id is either based off the X-Request-Id header in the request, which would typically be generated + # by a firewall, load balancer, or the web server, or, if this header is not available, a random uuid. If the + # header is accepted from the outside world, we sanitize it to a max of 255 chars and alphanumeric and dashes only. + # + # The unique request id can be used to trace a request end-to-end and would typically end up being part of log files + # from multiple pieces of the stack. + class RequestId + def initialize(app) + @app = app + end + + def call(env) + env["action_dispatch.request_id"] = external_request_id(env) || internal_request_id + + status, headers, body = @app.call(env) + + headers["X-Request-Id"] = env["action_dispatch.request_id"] + [ status, headers, body ] + end + + private + def external_request_id(env) + if env["HTTP_X_REQUEST_ID"].present? + env["HTTP_X_REQUEST_ID"].gsub(/[^\w\d\-]/, "").first(255) + end + end + + def internal_request_id + SecureRandom.uuid + end + end +end -- cgit v1.2.3 From ddbb2cae3146fc125375a0aae61bbaca9328b797 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 19 Oct 2011 22:15:25 +0300 Subject: Require securerandom as it is the proper dependency. --- actionpack/lib/action_dispatch/middleware/request_id.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/request_id.rb b/actionpack/lib/action_dispatch/middleware/request_id.rb index 968ad6c28d..c515798d48 100644 --- a/actionpack/lib/action_dispatch/middleware/request_id.rb +++ b/actionpack/lib/action_dispatch/middleware/request_id.rb @@ -1,4 +1,4 @@ -require 'digest/md5' +require 'securerandom' module ActionDispatch # Makes a unique request id available to the action_dispatch.request_id env variable (which is then accessible through -- cgit v1.2.3 From 1b50207ed3a2f545763b8c0b3afcd35d9d36d4e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 19 Oct 2011 22:17:54 +0300 Subject: Require missing string access dependency. --- actionpack/lib/action_dispatch/middleware/request_id.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/request_id.rb b/actionpack/lib/action_dispatch/middleware/request_id.rb index c515798d48..cdddc55aae 100644 --- a/actionpack/lib/action_dispatch/middleware/request_id.rb +++ b/actionpack/lib/action_dispatch/middleware/request_id.rb @@ -1,4 +1,5 @@ require 'securerandom' +require 'active_support/core_ext/string/access' module ActionDispatch # Makes a unique request id available to the action_dispatch.request_id env variable (which is then accessible through -- cgit v1.2.3 From ada78066fdbccffb1da092a2470211fa252b3c99 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Wed, 19 Oct 2011 14:45:42 -0500 Subject: Blah, SecureRandom#uuid is not supported in 1.8.7 -- cant wait for Rails 4.0 to drop compatibility with 1.8.x --- actionpack/lib/action_dispatch/middleware/request_id.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/request_id.rb b/actionpack/lib/action_dispatch/middleware/request_id.rb index cdddc55aae..4728e9f71e 100644 --- a/actionpack/lib/action_dispatch/middleware/request_id.rb +++ b/actionpack/lib/action_dispatch/middleware/request_id.rb @@ -33,7 +33,7 @@ module ActionDispatch end def internal_request_id - SecureRandom.uuid + SecureRandom.hex(16) end end end -- cgit v1.2.3 From f1fecd9b4e38c289b678bc2aadb406265963c528 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 19 Oct 2011 22:09:36 +0200 Subject: Make tests run on 1.8.x, add integration setup. --- actionpack/lib/action_dispatch/middleware/request_id.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/request_id.rb b/actionpack/lib/action_dispatch/middleware/request_id.rb index 4728e9f71e..f4d721f9bf 100644 --- a/actionpack/lib/action_dispatch/middleware/request_id.rb +++ b/actionpack/lib/action_dispatch/middleware/request_id.rb @@ -18,20 +18,19 @@ module ActionDispatch def call(env) env["action_dispatch.request_id"] = external_request_id(env) || internal_request_id - status, headers, body = @app.call(env) headers["X-Request-Id"] = env["action_dispatch.request_id"] [ status, headers, body ] end - + private def external_request_id(env) if env["HTTP_X_REQUEST_ID"].present? env["HTTP_X_REQUEST_ID"].gsub(/[^\w\d\-]/, "").first(255) end end - + def internal_request_id SecureRandom.hex(16) end -- cgit v1.2.3 From 4ef74536940ea4c8c7f8c2cb0252bfe5f0db6fdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 19 Oct 2011 22:10:43 +0200 Subject: Load object/blank and make use of presence. --- actionpack/lib/action_dispatch/middleware/request_id.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/request_id.rb b/actionpack/lib/action_dispatch/middleware/request_id.rb index f4d721f9bf..d7bb9c58df 100644 --- a/actionpack/lib/action_dispatch/middleware/request_id.rb +++ b/actionpack/lib/action_dispatch/middleware/request_id.rb @@ -1,5 +1,6 @@ require 'securerandom' require 'active_support/core_ext/string/access' +require 'active_support/core_ext/object/blank' module ActionDispatch # Makes a unique request id available to the action_dispatch.request_id env variable (which is then accessible through @@ -26,8 +27,8 @@ module ActionDispatch private def external_request_id(env) - if env["HTTP_X_REQUEST_ID"].present? - env["HTTP_X_REQUEST_ID"].gsub(/[^\w\d\-]/, "").first(255) + if request_id = env["HTTP_X_REQUEST_ID"].presence + request_id.gsub(/[^\w\d\-]/, "").first(255) end end -- cgit v1.2.3 From 951a325c99ef2845f29ef95c85230ac2e835a31c Mon Sep 17 00:00:00 2001 From: Marc Bowes Date: Thu, 20 Oct 2011 10:00:42 +0300 Subject: Remove the unneeded `\d` when sanitizing `X-Request-Id`. --- actionpack/lib/action_dispatch/middleware/request_id.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/request_id.rb b/actionpack/lib/action_dispatch/middleware/request_id.rb index d7bb9c58df..bee446c8a5 100644 --- a/actionpack/lib/action_dispatch/middleware/request_id.rb +++ b/actionpack/lib/action_dispatch/middleware/request_id.rb @@ -28,7 +28,7 @@ module ActionDispatch private def external_request_id(env) if request_id = env["HTTP_X_REQUEST_ID"].presence - request_id.gsub(/[^\w\d\-]/, "").first(255) + request_id.gsub(/[^\w\-]/, "").first(255) end end -- cgit v1.2.3 From 2b04c2f66e3cf5abbbf118eaa1e692b9e1380e4e Mon Sep 17 00:00:00 2001 From: Brian Durand Date: Fri, 21 Oct 2011 13:13:29 -0500 Subject: Add ActionDispatch::Session::CacheStore as a generic way of storing sessions in a cache. --- .../middleware/session/cache_store.rb | 50 ++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 actionpack/lib/action_dispatch/middleware/session/cache_store.rb (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/session/cache_store.rb b/actionpack/lib/action_dispatch/middleware/session/cache_store.rb new file mode 100644 index 0000000000..d3b6fd12fa --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/session/cache_store.rb @@ -0,0 +1,50 @@ +require 'action_dispatch/middleware/session/abstract_store' +require 'rack/session/memcache' + +module ActionDispatch + module Session + # Session store that uses an ActiveSupport::Cache::Store to store the sessions. This store is most useful + # if you don't store critical data in your sessions and you don't need them to live for extended periods + # of time. + class CacheStore < AbstractStore + # Create a new store. The cache to use can be passed in the :cache option. If it is + # not specified, Rails.cache will be used. + def initialize(app, options = {}) + @cache = options[:cache] || Rails.cache + options[:expire_after] ||= @cache.options[:expires_in] + super + end + + # Get a session from the cache. + def get_session(env, sid) + sid ||= generate_sid + session = @cache.read(cache_key(sid)) + session ||= {} + [sid, session] + end + + # Set a session in the cache. + def set_session(env, sid, session, options) + key = cache_key(sid) + if session + @cache.write(key, session, :expires_in => options[:expire_after]) + else + @cache.delete(key) + end + sid + end + + # Remove a session from the cache. + def destroy_session(env, sid, options) + @cache.delete(cache_key(sid)) + generate_sid + end + + private + # Turn the session id into a cache key. + def cache_key(sid) + "_session_id:#{sid}" + end + end + end +end -- cgit v1.2.3 From e368583ba716f90120e7e6ccfc5ee76de015521c Mon Sep 17 00:00:00 2001 From: mjy Date: Tue, 25 Oct 2011 15:08:28 -0400 Subject: Adds missing closing regex slashes. --- actionpack/lib/action_dispatch/routing/mapper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index ef31d1e004..09a8c10043 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -696,7 +696,7 @@ module ActionDispatch # Allows you to constrain the nested routes based on a set of rules. # For instance, in order to change the routes to allow for a dot character in the +id+ parameter: # - # constraints(:id => /\d+\.\d+) do + # constraints(:id => /\d+\.\d+/) do # resources :posts # end # @@ -706,7 +706,7 @@ module ActionDispatch # You may use this to also restrict other parameters: # # resources :posts do - # constraints(:post_id => /\d+\.\d+) do + # constraints(:post_id => /\d+\.\d+/) do # resources :comments # end # end -- cgit v1.2.3 From 5f4550889dcab7def4122d37a3379d57627f68e2 Mon Sep 17 00:00:00 2001 From: Alexey Vakhov Date: Tue, 1 Nov 2011 08:55:03 +0400 Subject: Fix typo in constraints method documentation --- actionpack/lib/action_dispatch/routing/mapper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 09a8c10043..e8bfe9bbd0 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -735,7 +735,7 @@ module ActionDispatch # if the user should be given access to that route, or +false+ if the user should not. # # class Iphone - # def self.matches(request) + # def self.matches?(request) # request.env["HTTP_USER_AGENT"] =~ /iPhone/ # end # end -- cgit v1.2.3 From 702aecb126712de9f996da74357cafe14f449d24 Mon Sep 17 00:00:00 2001 From: Aviv Ben-Yosef Date: Tue, 1 Nov 2011 08:59:20 +0200 Subject: Fix typo in Dispatcher#controller documentation --- actionpack/lib/action_dispatch/routing/route_set.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index e7bc431783..2bcde16110 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -37,7 +37,7 @@ module ActionDispatch # If this is a default_controller (i.e. a controller specified by the user) # we should raise an error in case it's not found, because it usually means - # an user error. However, if the controller was retrieved through a dynamic + # a user error. However, if the controller was retrieved through a dynamic # segment, as in :controller(/:action), we should simply return nil and # delegate the control back to Rack cascade. Besides, if this is not a default # controller, it means we should respect the @scope[:module] parameter. -- cgit v1.2.3 From 746331711585cfcb158dafcaf3ea5d60d9825ed2 Mon Sep 17 00:00:00 2001 From: Alexey Vakhov Date: Thu, 3 Nov 2011 08:43:28 +0400 Subject: Fix small typos in routing docs --- actionpack/lib/action_dispatch/routing/mapper.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index e8bfe9bbd0..970236a05a 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -285,7 +285,7 @@ module ActionDispatch # A pattern can also point to a +Rack+ endpoint i.e. anything that # responds to +call+: # - # match 'photos/:id' => lambda {|hash| [200, {}, "Coming soon" } + # match 'photos/:id' => lambda {|hash| [200, {}, "Coming soon"] } # match 'photos/:id' => PhotoRackApp # # Yes, controller actions are just rack endpoints # match 'photos/:id' => PhotosController.action(:show) @@ -1023,6 +1023,7 @@ module ActionDispatch # creates seven different routes in your application, all mapping to # the +Photos+ controller: # + # GET /photos # GET /photos/new # POST /photos # GET /photos/:id @@ -1038,6 +1039,7 @@ module ActionDispatch # # This generates the following comments routes: # + # GET /photos/:photo_id/comments # GET /photos/:photo_id/comments/new # POST /photos/:photo_id/comments # GET /photos/:photo_id/comments/:id -- cgit v1.2.3 From a50f659e081785479b068b311862703584a589ca Mon Sep 17 00:00:00 2001 From: Olivier Lacan Date: Thu, 3 Nov 2011 10:01:32 -0400 Subject: CSS fix to prevent error output from being breaking out of body element. Using the white-space: pre-wrap adds extra line breaks to prevent the text from breaking out of the element's box. In this case single line output can be extremely long, breaking out the element. See for reference: http://www.quirksmode.org/css/whitespace.html Before: http://link.olivierlacan.com/BVU4 After: http://link.olivierlacan.com/BUfM --- actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb | 1 + 1 file changed, 1 insertion(+) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb index 6e71fd7ddc..1a308707d1 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb @@ -16,6 +16,7 @@ background-color: #eee; padding: 10px; font-size: 11px; + white-space: pre-wrap; } a { color: #000; } -- cgit v1.2.3 From bd559b00680b6e4c272d4df36f239768750f1a6b Mon Sep 17 00:00:00 2001 From: Bradford Folkens Date: Tue, 8 Nov 2011 07:52:35 -0600 Subject: Fix trouble using :subdomain in development environment when using numeric addresses. See-also pull request #3561 from 3-1-stable Otherwise the following occurs: TypeError: can't convert nil into String /Users/bfolkens/dev/bfolkens-rails-core/actionpack/lib/action_dispatch/http/url.rb:75:in host_or_subdomain_and_domain' /Users/bfolkens/dev/bfolkens-rails-core/actionpack/lib/action_dispatch/http/url.rb:37:in url_for' /Users/bfolkens/dev/bfolkens-rails-core/actionpack/lib/action_dispatch/routing/url_for.rb:147:in test_subdomain_may_be_accepted_with_numeric_host' /Users/bfolkens/dev/bfolkens-rails-core/activesupport/lib/active_support/testing/setup_and_teardown.rb:67:in run' /Users/bfolkens/dev/bfolkens-rails-core/activesupport/lib/active_support/callbacks.rb:426:in send' /Users/bfolkens/dev/bfolkens-rails-core/activesupport/lib/active_support/callbacks.rb:81:in run' --- actionpack/lib/action_dispatch/http/url.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index c8ddd07bfa..129a8b1031 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -64,7 +64,7 @@ module ActionDispatch end def host_or_subdomain_and_domain(options) - return options[:host] if options[:subdomain].nil? && options[:domain].nil? + return options[:host] if !named_host?(options[:host]) || (options[:subdomain].nil? && options[:domain].nil?) tld_length = options[:tld_length] || @@tld_length -- cgit v1.2.3 From 9432163c60fc4387d1dfb11ca7c92a08ce72f1c2 Mon Sep 17 00:00:00 2001 From: Andre Arko Date: Fri, 11 Nov 2011 21:22:49 -1000 Subject: refactor RemoteIp middleware - return the last forwarded IP before REMOTE_ADDR to handle proxies - remove completely superfluous RemoteIpGetter class - remove duplication of trusted proxies regexp - remove unused constant from Request - move comments from Request to where they are actually relevant - edit comments for clarity of purpose The original code (confusingly) tried to return REMOTE_ADDR both at the beginning and the end of the chain of options. Since REMOTE_ADDR is _always_ set, this is kind of silly. This change leaves REMOTE_ADDR as the last option, so that proxied requests will be assigned the correct remote IP address. --- actionpack/lib/action_dispatch/http/request.rb | 19 +---- .../lib/action_dispatch/middleware/remote_ip.rb | 81 ++++++++++++---------- 2 files changed, 45 insertions(+), 55 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 7a5237dcf3..69ca050d0c 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -155,24 +155,7 @@ module ActionDispatch @ip ||= super end - # Which IP addresses are "trusted proxies" that can be stripped from - # the right-hand-side of X-Forwarded-For. - # - # http://en.wikipedia.org/wiki/Private_network#Private_IPv4_address_spaces. - TRUSTED_PROXIES = %r{ - ^127\.0\.0\.1$ | # localhost - ^(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 - )\. - }x - - # Determines originating IP address. REMOTE_ADDR is the standard - # but will fail if the user is behind a proxy. HTTP_CLIENT_IP and/or - # HTTP_X_FORWARDED_FOR are set by proxies so check for these if - # REMOTE_ADDR is a proxy. HTTP_X_FORWARDED_FOR may be a comma- - # delimited list in the case of multiple chained proxies; the last - # address which is not trusted is the originating IP. + # Originating IP address, usually set by the RemoteIp middleware. def remote_ip @remote_ip ||= (@env["action_dispatch.remote_ip"] || ip).to_s end diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index c7d710b98e..79f9ddcd04 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -2,50 +2,57 @@ module ActionDispatch class RemoteIp class IpSpoofAttackError < StandardError ; end - class RemoteIpGetter - def initialize(env, check_ip_spoofing, trusted_proxies) - @env = env - @check_ip_spoofing = check_ip_spoofing - @trusted_proxies = trusted_proxies + # 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 + TRUSTED_PROXIES = %r{ + ^127\.0\.0\.1$ | # localhost + ^(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 + )\. + }x + + def initialize(app, check_ip_spoofing = true, custom_proxies = nil) + @app = app + @check_ip_spoofing = check_ip_spoofing + if custom_proxies + custom_regexp = Regexp.new(custom_proxies, "i") + @trusted_proxies = Regexp.union(TRUSTED_PROXIES, custom_regexp) + else + @trusted_proxies = TRUSTED_PROXIES end + end - def remote_addrs - @remote_addrs ||= begin - list = @env['REMOTE_ADDR'] ? @env['REMOTE_ADDR'].split(/[,\s]+/) : [] - list.reject { |addr| addr =~ @trusted_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 last address which is not a known proxy + # will be the originating IP. + def call(env) + client_ip = env['HTTP_CLIENT_IP'] + forwarded_ips = ips_from(env, 'HTTP_X_FORWARDED_FOR') + remote_addrs = ips_from(env, 'REMOTE_ADDR') + + if client_ip && @check_ip_spoofing && !forwarded_ips.include?(client_ip) + # 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}" \ + "HTTP_X_FORWARDED_FOR=#{env['HTTP_X_FORWARDED_FOR'].inspect}" end - def to_s - return remote_addrs.first if remote_addrs.any? - - forwarded_ips = @env['HTTP_X_FORWARDED_FOR'] ? @env['HTTP_X_FORWARDED_FOR'].strip.split(/[,\s]+/) : [] - - if client_ip = @env['HTTP_CLIENT_IP'] - if @check_ip_spoofing && !forwarded_ips.include?(client_ip) - # 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}" \ - "HTTP_X_FORWARDED_FOR=#{@env['HTTP_X_FORWARDED_FOR'].inspect}" - end - return client_ip - end - - return forwarded_ips.reject { |ip| ip =~ @trusted_proxies }.last || @env["REMOTE_ADDR"] - end + remote_ip = client_ip || forwarded_ips.last || remote_addrs.last + env["action_dispatch.remote_ip"] = remote_ip + @app.call(env) end - def initialize(app, check_ip_spoofing = true, trusted_proxies = nil) - @app = app - @check_ip_spoofing = check_ip_spoofing - regex = '(^127\.0\.0\.1$|^(10|172\.(1[6-9]|2[0-9]|30|31)|192\.168)\.)' - regex << "|(#{trusted_proxies})" if trusted_proxies - @trusted_proxies = Regexp.new(regex, "i") - end + protected - def call(env) - env["action_dispatch.remote_ip"] = RemoteIpGetter.new(env, @check_ip_spoofing, @trusted_proxies) - @app.call(env) + def ips_from(env, header) + ips = env[header] ? env[header].strip.split(/[,\s]+/) : [] + ips.reject{|ip| ip =~ @trusted_proxies } end + end end \ No newline at end of file -- cgit v1.2.3 From 317f4e22365e2d9b8200aefbda943798e8f85a82 Mon Sep 17 00:00:00 2001 From: Andre Arko Date: Sat, 12 Nov 2011 00:45:31 -1000 Subject: defer calculating the remote IP until requested --- .../lib/action_dispatch/middleware/remote_ip.rb | 60 +++++++++++++--------- 1 file changed, 36 insertions(+), 24 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 79f9ddcd04..3b813b03bb 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -13,6 +13,8 @@ module ActionDispatch )\. }x + attr_reader :check_ip_spoofing, :trusted_proxies + def initialize(app, check_ip_spoofing = true, custom_proxies = nil) @app = app @check_ip_spoofing = check_ip_spoofing @@ -24,35 +26,45 @@ module ActionDispatch end 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 last address which is not a known proxy - # will be the originating IP. def call(env) - client_ip = env['HTTP_CLIENT_IP'] - forwarded_ips = ips_from(env, 'HTTP_X_FORWARDED_FOR') - remote_addrs = ips_from(env, 'REMOTE_ADDR') - - if client_ip && @check_ip_spoofing && !forwarded_ips.include?(client_ip) - # 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}" \ - "HTTP_X_FORWARDED_FOR=#{env['HTTP_X_FORWARDED_FOR'].inspect}" - end - - remote_ip = client_ip || forwarded_ips.last || remote_addrs.last - env["action_dispatch.remote_ip"] = remote_ip + env["action_dispatch.remote_ip"] = GetIp.new(env, self) @app.call(env) end - protected + class GetIp + def initialize(env, middleware) + @env, @middleware = env, middleware + 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 last address which is not a known proxy + # will be the originating IP. + def to_s + client_ip = @env['HTTP_CLIENT_IP'] + forwarded_ips = ips_from('HTTP_X_FORWARDED_FOR') + remote_addrs = ips_from('REMOTE_ADDR') - def ips_from(env, header) - ips = env[header] ? env[header].strip.split(/[,\s]+/) : [] - ips.reject{|ip| ip =~ @trusted_proxies } + check_ip = client_ip && @middleware.check_ip_spoofing + if check_ip && !forwarded_ips.include?(client_ip) + # 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}" \ + "HTTP_X_FORWARDED_FOR=#{env['HTTP_X_FORWARDED_FOR'].inspect}" + end + + client_ip || forwarded_ips.last || remote_addrs.last + end + + protected + + def ips_from(header) + ips = @env[header] ? @env[header].strip.split(/[,\s]+/) : [] + ips.reject{|ip| ip =~ @middleware.trusted_proxies } + end end end -end \ No newline at end of file +end -- cgit v1.2.3 From b74aedff3c4df83376ac969163e646bcf20bc7d4 Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Sat, 12 Nov 2011 23:15:54 +0900 Subject: Unneeded require memoizable --- actionpack/lib/action_dispatch/http/headers.rb | 2 -- 1 file changed, 2 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index 505d5560b1..040b51e040 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -1,5 +1,3 @@ -require 'active_support/memoizable' - module ActionDispatch module Http class Headers < ::Hash -- cgit v1.2.3 From 2189bff732490aa842c88f1691993520fa1eb9ab Mon Sep 17 00:00:00 2001 From: Andre Arko Date: Sun, 13 Nov 2011 10:20:29 -1000 Subject: correctly raise IpSpoofAttackError message --- actionpack/lib/action_dispatch/middleware/remote_ip.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 3b813b03bb..3208256d56 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -51,8 +51,8 @@ module ActionDispatch if check_ip && !forwarded_ips.include?(client_ip) # 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}" \ - "HTTP_X_FORWARDED_FOR=#{env['HTTP_X_FORWARDED_FOR'].inspect}" + "HTTP_CLIENT_IP=#{@env['HTTP_CLIENT_IP'].inspect}" \ + "HTTP_X_FORWARDED_FOR=#{@env['HTTP_X_FORWARDED_FOR'].inspect}" end client_ip || forwarded_ips.last || remote_addrs.last -- cgit v1.2.3 From 2d063c6269a546c8bab4b36c027246f582bfaa13 Mon Sep 17 00:00:00 2001 From: Andre Arko Date: Sun, 13 Nov 2011 10:20:55 -1000 Subject: turns out the tests expect remote_addrs.first --- actionpack/lib/action_dispatch/middleware/remote_ip.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 3208256d56..5f81b639ae 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -55,7 +55,7 @@ module ActionDispatch "HTTP_X_FORWARDED_FOR=#{@env['HTTP_X_FORWARDED_FOR'].inspect}" end - client_ip || forwarded_ips.last || remote_addrs.last + client_ip || forwarded_ips.last || remote_addrs.first end protected -- cgit v1.2.3 From 9c4532bf74604ae1c52a44d1aa8c1022a312ff88 Mon Sep 17 00:00:00 2001 From: Andre Arko Date: Sun, 13 Nov 2011 10:22:12 -1000 Subject: remove ignored flag, fixes warnings --- actionpack/lib/action_dispatch/middleware/remote_ip.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 5f81b639ae..58e25aed5a 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -19,7 +19,7 @@ module ActionDispatch @app = app @check_ip_spoofing = check_ip_spoofing if custom_proxies - custom_regexp = Regexp.new(custom_proxies, "i") + custom_regexp = Regexp.new(custom_proxies) @trusted_proxies = Regexp.union(TRUSTED_PROXIES, custom_regexp) else @trusted_proxies = TRUSTED_PROXIES -- cgit v1.2.3 From 00a0a4ddebe0160f851d28e29d5fb7e8e7a2a5dc Mon Sep 17 00:00:00 2001 From: Andre Arko Date: Mon, 14 Nov 2011 11:20:20 -1000 Subject: cleaner names --- actionpack/lib/action_dispatch/middleware/remote_ip.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 58e25aed5a..446fcce823 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -13,16 +13,16 @@ module ActionDispatch )\. }x - attr_reader :check_ip_spoofing, :trusted_proxies + attr_reader :check_ip, :proxies def initialize(app, check_ip_spoofing = true, custom_proxies = nil) @app = app - @check_ip_spoofing = check_ip_spoofing + @check_ip = check_ip_spoofing if custom_proxies custom_regexp = Regexp.new(custom_proxies) - @trusted_proxies = Regexp.union(TRUSTED_PROXIES, custom_regexp) + @proxies = Regexp.union(TRUSTED_PROXIES, custom_regexp) else - @trusted_proxies = TRUSTED_PROXIES + @proxies = TRUSTED_PROXIES end end @@ -47,7 +47,7 @@ module ActionDispatch forwarded_ips = ips_from('HTTP_X_FORWARDED_FOR') remote_addrs = ips_from('REMOTE_ADDR') - check_ip = client_ip && @middleware.check_ip_spoofing + check_ip = client_ip && @middleware.check_ip if check_ip && !forwarded_ips.include?(client_ip) # We don't know which came from the proxy, and which from the user raise IpSpoofAttackError, "IP spoofing attack?!" \ @@ -62,7 +62,7 @@ module ActionDispatch def ips_from(header) ips = @env[header] ? @env[header].strip.split(/[,\s]+/) : [] - ips.reject{|ip| ip =~ @middleware.trusted_proxies } + ips.reject{|ip| ip =~ @middleware.proxies } end end -- cgit v1.2.3 From cda1a5d5fe002ca92aca01586e8a60439605f960 Mon Sep 17 00:00:00 2001 From: Andre Arko Date: Mon, 14 Nov 2011 11:20:57 -1000 Subject: memoize the relatively expensive remote IP code --- actionpack/lib/action_dispatch/middleware/remote_ip.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 446fcce823..ee0d19a50d 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -42,7 +42,7 @@ module ActionDispatch # HTTP_X_FORWARDED_FOR may be a comma-delimited list in the case of # multiple chained proxies. The last address which is not a known proxy # will be the originating IP. - def to_s + def calculate_ip client_ip = @env['HTTP_CLIENT_IP'] forwarded_ips = ips_from('HTTP_X_FORWARDED_FOR') remote_addrs = ips_from('REMOTE_ADDR') @@ -58,6 +58,12 @@ module ActionDispatch client_ip || forwarded_ips.last || remote_addrs.first end + def to_s + return @ip if @calculated_ip + @calculated_ip = true + @ip = calculate_ip + end + protected def ips_from(header) -- cgit v1.2.3 From 4f2bf6491cbc482d25a9357c2eb7fc8047d4f12e Mon Sep 17 00:00:00 2001 From: Andre Arko Date: Mon, 14 Nov 2011 11:22:44 -1000 Subject: Return the calculated remote_ip or ip This was an especially nasty bug introduced in 317f4e2, by the way that an instance of GetIp is not nil, but GetIp#to_s could sometimes return nil. Gross, huh? --- actionpack/lib/action_dispatch/http/request.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 69ca050d0c..b10f6b48c7 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -157,7 +157,8 @@ module ActionDispatch # Originating IP address, usually set by the RemoteIp middleware. def remote_ip - @remote_ip ||= (@env["action_dispatch.remote_ip"] || ip).to_s + # Coerce the remote_ip object into a string, because to_s could return nil + @remote_ip ||= @env["action_dispatch.remote_ip"].to_s || ip end # Returns the unique request id, which is based off either the X-Request-Id header that can -- cgit v1.2.3 From d743954792ccf5975a11ee88cdd690e8f1915728 Mon Sep 17 00:00:00 2001 From: Andre Arko Date: Mon, 14 Nov 2011 16:43:21 -1000 Subject: GetIp#to_s should never return nil. That's icky. --- actionpack/lib/action_dispatch/http/request.rb | 5 ++--- actionpack/lib/action_dispatch/middleware/remote_ip.rb | 9 ++++++--- 2 files changed, 8 insertions(+), 6 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index b10f6b48c7..0e8bd0bd6d 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -155,10 +155,9 @@ module ActionDispatch @ip ||= super end - # Originating IP address, usually set by the RemoteIp middleware. + # Originating IP address from the RemoteIp middleware. def remote_ip - # Coerce the remote_ip object into a string, because to_s could return nil - @remote_ip ||= @env["action_dispatch.remote_ip"].to_s || ip + @remote_ip ||= @env["action_dispatch.remote_ip"] end # Returns the unique request id, which is based off either the X-Request-Id header that can diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index ee0d19a50d..77aa4e743e 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -55,7 +55,10 @@ module ActionDispatch "HTTP_X_FORWARDED_FOR=#{@env['HTTP_X_FORWARDED_FOR'].inspect}" end - client_ip || forwarded_ips.last || remote_addrs.first + not_proxy = client_ip || forwarded_ips.last || remote_addrs.first + + # Return first REMOTE_ADDR if there are no other options + not_proxy || ips_from('REMOTE_ADDR', :all).first end def to_s @@ -66,9 +69,9 @@ module ActionDispatch protected - def ips_from(header) + def ips_from(header, allow_proxies = false) ips = @env[header] ? @env[header].strip.split(/[,\s]+/) : [] - ips.reject{|ip| ip =~ @middleware.proxies } + allow_proxies ? ips : ips.reject{|ip| ip =~ @middleware.proxies } end end -- cgit v1.2.3 From 8d1a2b3ecde5a8745b3eaab4763a71d80ca3441f Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Tue, 15 Nov 2011 22:47:18 +0000 Subject: Revert "Merge pull request #3640 from indirect/remote_ip" This reverts commit 6491aadc525b8703708e0fd0fbf05bd436a47801, reversing changes made to 83bf0b626cf2134260903e57d74f67de57384073. See https://github.com/rails/rails/pull/3640#issuecomment-2752761 for explanation. --- actionpack/lib/action_dispatch/http/request.rb | 5 +++-- actionpack/lib/action_dispatch/middleware/remote_ip.rb | 9 +++------ 2 files changed, 6 insertions(+), 8 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 0e8bd0bd6d..b10f6b48c7 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -155,9 +155,10 @@ module ActionDispatch @ip ||= super end - # Originating IP address from the RemoteIp middleware. + # Originating IP address, usually set by the RemoteIp middleware. def remote_ip - @remote_ip ||= @env["action_dispatch.remote_ip"] + # Coerce the remote_ip object into a string, because to_s could return nil + @remote_ip ||= @env["action_dispatch.remote_ip"].to_s || ip end # Returns the unique request id, which is based off either the X-Request-Id header that can diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 77aa4e743e..ee0d19a50d 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -55,10 +55,7 @@ module ActionDispatch "HTTP_X_FORWARDED_FOR=#{@env['HTTP_X_FORWARDED_FOR'].inspect}" end - not_proxy = client_ip || forwarded_ips.last || remote_addrs.first - - # Return first REMOTE_ADDR if there are no other options - not_proxy || ips_from('REMOTE_ADDR', :all).first + client_ip || forwarded_ips.last || remote_addrs.first end def to_s @@ -69,9 +66,9 @@ module ActionDispatch protected - def ips_from(header, allow_proxies = false) + def ips_from(header) ips = @env[header] ? @env[header].strip.split(/[,\s]+/) : [] - allow_proxies ? ips : ips.reject{|ip| ip =~ @middleware.proxies } + ips.reject{|ip| ip =~ @middleware.proxies } end end -- cgit v1.2.3 From f05ccf805a6d2a3ed73ef9928577e8b0ebbb3c49 Mon Sep 17 00:00:00 2001 From: Andre Arko Date: Wed, 16 Nov 2011 12:53:43 -1000 Subject: Revert "Revert "Merge pull request #3640 from indirect/remote_ip"" This reverts commit 8d1a2b3ecde5a8745b3eaab4763a71d80ca3441f, because I have fixed the issues this commit caused in the next commit. --- actionpack/lib/action_dispatch/http/request.rb | 5 ++--- actionpack/lib/action_dispatch/middleware/remote_ip.rb | 9 ++++++--- 2 files changed, 8 insertions(+), 6 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index b10f6b48c7..0e8bd0bd6d 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -155,10 +155,9 @@ module ActionDispatch @ip ||= super end - # Originating IP address, usually set by the RemoteIp middleware. + # Originating IP address from the RemoteIp middleware. def remote_ip - # Coerce the remote_ip object into a string, because to_s could return nil - @remote_ip ||= @env["action_dispatch.remote_ip"].to_s || ip + @remote_ip ||= @env["action_dispatch.remote_ip"] end # Returns the unique request id, which is based off either the X-Request-Id header that can diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index ee0d19a50d..77aa4e743e 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -55,7 +55,10 @@ module ActionDispatch "HTTP_X_FORWARDED_FOR=#{@env['HTTP_X_FORWARDED_FOR'].inspect}" end - client_ip || forwarded_ips.last || remote_addrs.first + not_proxy = client_ip || forwarded_ips.last || remote_addrs.first + + # Return first REMOTE_ADDR if there are no other options + not_proxy || ips_from('REMOTE_ADDR', :all).first end def to_s @@ -66,9 +69,9 @@ module ActionDispatch protected - def ips_from(header) + def ips_from(header, allow_proxies = false) ips = @env[header] ? @env[header].strip.split(/[,\s]+/) : [] - ips.reject{|ip| ip =~ @middleware.proxies } + allow_proxies ? ips : ips.reject{|ip| ip =~ @middleware.proxies } end end -- cgit v1.2.3 From 5621abd5698536f1676306930f6aef105d7ae6dc Mon Sep 17 00:00:00 2001 From: Andre Arko Date: Wed, 16 Nov 2011 12:49:15 -1000 Subject: :facepalm: Request#remote_ip has to work without the middleware --- actionpack/lib/action_dispatch/http/request.rb | 4 ++-- actionpack/lib/action_dispatch/middleware/remote_ip.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 0e8bd0bd6d..4e6feb708e 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -155,9 +155,9 @@ module ActionDispatch @ip ||= super end - # Originating IP address from the RemoteIp middleware. + # Originating IP address, usually set by the RemoteIp middleware. def remote_ip - @remote_ip ||= @env["action_dispatch.remote_ip"] + @remote_ip ||= @env["action_dispatch.remote_ip"] || ip end # Returns the unique request id, which is based off either the X-Request-Id header that can diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 77aa4e743e..3a88f2ca43 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -58,7 +58,7 @@ module ActionDispatch not_proxy = client_ip || forwarded_ips.last || remote_addrs.first # Return first REMOTE_ADDR if there are no other options - not_proxy || ips_from('REMOTE_ADDR', :all).first + not_proxy || ips_from('REMOTE_ADDR', :allow_proxies).first end def to_s -- cgit v1.2.3 From a9044d011790063e17159209f7bb1cbea255d4dd Mon Sep 17 00:00:00 2001 From: Andre Arko Date: Wed, 16 Nov 2011 12:58:49 -1000 Subject: the object itself isn't the IP, #to_s is the IP --- actionpack/lib/action_dispatch/http/request.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 4e6feb708e..69ca050d0c 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -157,7 +157,7 @@ module ActionDispatch # Originating IP address, usually set by the RemoteIp middleware. def remote_ip - @remote_ip ||= @env["action_dispatch.remote_ip"] || ip + @remote_ip ||= (@env["action_dispatch.remote_ip"] || ip).to_s end # Returns the unique request id, which is based off either the X-Request-Id header that can -- cgit v1.2.3 From e3671422556ac61f39539264713ba9c04814b80f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 16 Nov 2011 16:55:11 -0800 Subject: Initialize our instance variables. --- actionpack/lib/action_dispatch/middleware/remote_ip.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 3a88f2ca43..8dbe3af6f1 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -33,7 +33,9 @@ module ActionDispatch class GetIp def initialize(env, middleware) - @env, @middleware = env, middleware + @env = env + @middleware = middleware + @calculate_ip = false end # Determines originating IP address. REMOTE_ADDR is the standard -- cgit v1.2.3 From c3ae1d2aec400d6aaea78bd94eb7845b71f1ec15 Mon Sep 17 00:00:00 2001 From: Arun Agrawal Date: Thu, 17 Nov 2011 12:50:19 +0530 Subject: It should be @calculated_ip not @calculate_ip We are using @calculated_ip. @calculate_ip is no where used --- actionpack/lib/action_dispatch/middleware/remote_ip.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 8dbe3af6f1..66ece60860 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -35,7 +35,7 @@ module ActionDispatch def initialize(env, middleware) @env = env @middleware = middleware - @calculate_ip = false + @calculated_ip = false end # Determines originating IP address. REMOTE_ADDR is the standard -- cgit v1.2.3 From 76780c34f5e0f0e821e408482172454751514241 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 21 Oct 2011 10:25:47 -0700 Subject: some refactoring of the match method --- actionpack/lib/action_dispatch/routing/mapper.rb | 42 ++++++++++-------------- 1 file changed, 17 insertions(+), 25 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 970236a05a..ff96b43a9d 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1245,32 +1245,38 @@ module ActionDispatch parent_resource.instance_of?(Resource) && @scope[:shallow] end - def match(*args) - options = args.extract_options!.dup + def match(path, *rest) + if rest.empty? && Hash === path + options = path + path, to = options.find { |name, value| name.is_a?(String) } + options.merge!(:to => to).delete(path) + paths = [path] + else + options = rest.pop || {} + paths = [path] + rest + end + options[:anchor] = true unless options.key?(:anchor) - if args.length > 1 - args.each { |path| match(path, options.dup) } + if paths.length > 1 + paths.each { |path| match(path, options.dup) } return self end on = options.delete(:on) if VALID_ON_OPTIONS.include?(on) - args.push(options) - return send(on){ match(*args) } + return send(on){ match(path, options) } elsif on raise ArgumentError, "Unknown scope #{on.inspect} given to :on" end if @scope[:scope_level] == :resources - args.push(options) - return nested { match(*args) } + return nested { match(path, options) } elsif @scope[:scope_level] == :resource - args.push(options) - return member { match(*args) } + return member { match(path, options) } end - action = args.first + action = path path = path_for_action(action, options.delete(:path)) if action.to_s =~ /^[\w\/]+$/ @@ -1466,19 +1472,6 @@ module ActionDispatch end end - module Shorthand #:nodoc: - def match(path, *rest) - if rest.empty? && Hash === path - options = path - path, to = options.find { |name, value| name.is_a?(String) } - options.merge!(:to => to).delete(path) - super(path, options) - else - super - end - end - end - def initialize(set) #:nodoc: @set = set @scope = { :path_names => @set.resources_path_names } @@ -1489,7 +1482,6 @@ module ActionDispatch include Redirection include Scoping include Resources - include Shorthand end end end -- cgit v1.2.3 From 494ab25772ba5eca741fe98f6beb48954949993f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 21 Oct 2011 11:15:41 -0700 Subject: breaking match down to smaller methods --- actionpack/lib/action_dispatch/routing/mapper.rb | 41 +++++++++++++----------- 1 file changed, 22 insertions(+), 19 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index ff96b43a9d..4c4b99c03e 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -374,10 +374,6 @@ module ActionDispatch # # Matches any request starting with 'path' # match 'path' => 'c#a', :anchor => false def match(path, options=nil) - mapping = Mapping.new(@set, @scope, path, options || {}) - app, conditions, requirements, defaults, as, anchor = mapping.to_route - @set.add_route(app, conditions, requirements, defaults, as, anchor) - self end # Mount a Rack-based application to be used within the application. @@ -1249,7 +1245,8 @@ module ActionDispatch if rest.empty? && Hash === path options = path path, to = options.find { |name, value| name.is_a?(String) } - options.merge!(:to => to).delete(path) + options[:to] = to + options.delete(path) paths = [path] else options = rest.pop || {} @@ -1258,25 +1255,29 @@ module ActionDispatch options[:anchor] = true unless options.key?(:anchor) - if paths.length > 1 - paths.each { |path| match(path, options.dup) } - return self - end + paths.each { |path| decomposed_match(path, options.dup) } + self + end + def decomposed_match(path, options) # :nodoc: on = options.delete(:on) if VALID_ON_OPTIONS.include?(on) - return send(on){ match(path, options) } + return send(on){ decomposed_match(path, options) } elsif on raise ArgumentError, "Unknown scope #{on.inspect} given to :on" end - if @scope[:scope_level] == :resources - return nested { match(path, options) } - elsif @scope[:scope_level] == :resource - return member { match(path, options) } + case @scope[:scope_level] + when :resources + nested { decomposed_match(path, options) } + when :resource + member { decomposed_match(path, options) } + else + add_route(path, options) end + end - action = path + def add_route(action, options) # :nodoc: path = path_for_action(action, options.delete(:path)) if action.to_s =~ /^[\w\/]+$/ @@ -1285,13 +1286,15 @@ module ActionDispatch action = nil end - if options.key?(:as) && !options[:as] + if !options.fetch(:as) { true } options.delete(:as) else options[:as] = name_for_action(options[:as], action) end - super(path, options) + mapping = Mapping.new(@set, @scope, path, options) + app, conditions, requirements, defaults, as, anchor = mapping.to_route + @set.add_route(app, conditions, requirements, defaults, as, anchor) end def root(options={}) @@ -1355,11 +1358,11 @@ module ActionDispatch end def resource_scope? #:nodoc: - @scope[:scope_level].in?([:resource, :resources]) + [:resource, :resources].include? @scope[:scope_level] end def resource_method_scope? #:nodoc: - @scope[:scope_level].in?([:collection, :member, :new]) + [:collection, :member, :new].include? @scope[:scope_level] end def with_exclusive_scope -- cgit v1.2.3 From 7459ba4f6c7d7b98fc0985255ccfd93186b0950f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 21 Oct 2011 11:42:59 -0700 Subject: pushing hash validation up --- actionpack/lib/action_dispatch/routing/mapper.rb | 27 ++++++++++++------------ 1 file changed, 14 insertions(+), 13 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 4c4b99c03e..d196f16600 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1255,25 +1255,26 @@ module ActionDispatch options[:anchor] = true unless options.key?(:anchor) + if options[:on] && !VALID_ON_OPTIONS.include?(options[:on]) + raise ArgumentError, "Unknown scope #{on.inspect} given to :on" + end + paths.each { |path| decomposed_match(path, options.dup) } self end def decomposed_match(path, options) # :nodoc: - on = options.delete(:on) - if VALID_ON_OPTIONS.include?(on) - return send(on){ decomposed_match(path, options) } - elsif on - raise ArgumentError, "Unknown scope #{on.inspect} given to :on" - end - - case @scope[:scope_level] - when :resources - nested { decomposed_match(path, options) } - when :resource - member { decomposed_match(path, options) } + if on = options.delete(:on) + send(on) { decomposed_match(path, options) } else - add_route(path, options) + case @scope[:scope_level] + when :resources + nested { decomposed_match(path, options) } + when :resource + member { decomposed_match(path, options) } + else + add_route(path, options) + end end end -- cgit v1.2.3 From 648f6113d1f0c6f9cdb2352a24e84c2c204d564b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 21 Oct 2011 15:04:48 -0700 Subject: move constants to methods since nothing else is using them --- actionpack/lib/action_dispatch/routing/mapper.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index d196f16600..f49155349c 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -863,8 +863,6 @@ module ActionDispatch CANONICAL_ACTIONS = %w(index create new show update destroy) class Resource #:nodoc: - DEFAULT_ACTIONS = [:index, :create, :new, :show, :update, :destroy, :edit] - attr_reader :controller, :path, :options def initialize(entities, options = {}) @@ -876,7 +874,7 @@ module ActionDispatch end def default_actions - self.class::DEFAULT_ACTIONS + [:index, :create, :new, :show, :update, :destroy, :edit] end def actions @@ -930,16 +928,17 @@ module ActionDispatch end class SingletonResource < Resource #:nodoc: - DEFAULT_ACTIONS = [:show, :create, :update, :destroy, :new, :edit] - def initialize(entities, options) super - @as = nil @controller = (options[:controller] || plural).to_s @as = options[:as] end + def default_actions + [:show, :create, :update, :destroy, :new, :edit] + end + def plural @plural ||= name.to_s.pluralize end -- cgit v1.2.3 From 33543ac87148cfdd5b1917a0698bccaf55690e28 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 21 Oct 2011 15:14:25 -0700 Subject: stop doing is_a? checks on the resource type --- actionpack/lib/action_dispatch/routing/mapper.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index f49155349c..91220e1cf7 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -986,7 +986,7 @@ module ActionDispatch return self end - resource_scope(SingletonResource.new(resources.pop, options)) do + resource_scope(:resource, SingletonResource.new(resources.pop, options)) do yield if block_given? collection do @@ -1117,7 +1117,7 @@ module ActionDispatch return self end - resource_scope(Resource.new(resources.pop, options)) do + resource_scope(:resources, Resource.new(resources.pop, options)) do yield if block_given? collection do @@ -1387,8 +1387,8 @@ module ActionDispatch @scope[:scope_level_resource] = old_resource end - def resource_scope(resource) #:nodoc: - with_scope_level(resource.is_a?(SingletonResource) ? :resource : :resources, resource) do + def resource_scope(level, resource) #:nodoc: + with_scope_level(level, resource) do scope(parent_resource.resource_scope) do yield end -- cgit v1.2.3 From 636405d2a6187cf1fe90f35b5e2945758afde160 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 21 Oct 2011 15:18:36 -0700 Subject: cleaning up variable names to match method parameter names --- actionpack/lib/action_dispatch/routing/mapper.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 91220e1cf7..4fc0d78267 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1387,8 +1387,8 @@ module ActionDispatch @scope[:scope_level_resource] = old_resource end - def resource_scope(level, resource) #:nodoc: - with_scope_level(level, resource) do + def resource_scope(kind, resource) #:nodoc: + with_scope_level(kind, resource) do scope(parent_resource.resource_scope) do yield end @@ -1396,10 +1396,12 @@ module ActionDispatch end def nested_options #:nodoc: - {}.tap do |options| - options[:as] = parent_resource.member_name - options[:constraints] = { "#{parent_resource.singular}_id".to_sym => id_constraint } if id_constraint? - end + options = { :as => parent_resource.member_name } + options[:constraints] = { + :"#{parent_resource.singular}_id" => id_constraint + } if id_constraint? + + options end def id_constraint? #:nodoc: -- cgit v1.2.3 From ecbae9947830bb3ec42db097140bf1b2154dee31 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 21 Oct 2011 15:40:10 -0700 Subject: no need for type checking --- actionpack/lib/action_dispatch/routing/mapper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 4fc0d78267..f2a2e92011 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1350,7 +1350,7 @@ module ActionDispatch end def scope_action_options? #:nodoc: - @scope[:options].is_a?(Hash) && (@scope[:options][:only] || @scope[:options][:except]) + @scope[:options] && (@scope[:options][:only] || @scope[:options][:except]) end def scope_action_options #:nodoc: -- cgit v1.2.3 From 3178cc9a80262d3bf7754f3507ef60243b46634f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 21 Oct 2011 16:47:49 -0700 Subject: copy options keys to the right place so that undo will work correctly --- actionpack/lib/action_dispatch/routing/mapper.rb | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index f2a2e92011..21624bcfc2 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -603,9 +603,12 @@ module ActionDispatch options[:constraints] ||= {} unless options[:constraints].is_a?(Hash) - block, options[:constraints] = options[:constraints], {} + options[:blocks] = options[:constraints] + options[:constraints] = {} end + options[:options] = options + scope_options.each do |option| if value = options.delete(option) recover[option] = @scope[option] @@ -613,21 +616,12 @@ module ActionDispatch end end - recover[:block] = @scope[:blocks] - @scope[:blocks] = merge_blocks_scope(@scope[:blocks], block) - - recover[:options] = @scope[:options] - @scope[:options] = merge_options_scope(@scope[:options], options) - yield self ensure scope_options.each do |option| @scope[option] = recover[option] if recover.has_key?(option) end - - @scope[:options] = recover[:options] - @scope[:blocks] = recover[:block] end # Scopes routes to a specific controller -- cgit v1.2.3 From 4589b2419b6c2f6d8b1ea0873999a4d0fa21bdb3 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 31 Oct 2011 16:26:11 -0400 Subject: require that all blocks have arity of 2 --- actionpack/lib/action_dispatch/routing/redirection.rb | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb index 804991ad5f..27dbf72519 100644 --- a/actionpack/lib/action_dispatch/routing/redirection.rb +++ b/actionpack/lib/action_dispatch/routing/redirection.rb @@ -43,13 +43,19 @@ module ActionDispatch path = args.shift path_proc = if path.is_a?(String) - proc { |params| (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params) } + proc { |params, request| (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params) } elsif options.any? options_proc(options) elsif path.respond_to?(:call) proc { |params, request| path.call(params, request) } elsif block - block + if block.arity < 2 + msg = "redirect blocks with arity of #{block.arity} are deprecated. Your block must take 2 parameters: the environment, and a request object" + ActiveSupport::Deprecation.warn msg + lambda { |params, _| block.call(params) } + else + block + end else raise ArgumentError, "redirection argument not supported" end @@ -85,8 +91,7 @@ module ActionDispatch lambda do |env| req = Request.new(env) - params = [req.symbolized_path_parameters] - params << req if path_proc.arity > 1 + params = [req.symbolized_path_parameters, req] uri = URI.parse(path_proc.call(*params)) uri.scheme ||= req.scheme @@ -107,4 +112,4 @@ module ActionDispatch end end -end \ No newline at end of file +end -- cgit v1.2.3 From 163b645472b863231558c93abcdc1454db00b287 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 31 Oct 2011 16:28:38 -0400 Subject: arity check has been pushed up, so no need for proc wrapping --- actionpack/lib/action_dispatch/routing/redirection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb index 27dbf72519..d61320980d 100644 --- a/actionpack/lib/action_dispatch/routing/redirection.rb +++ b/actionpack/lib/action_dispatch/routing/redirection.rb @@ -47,7 +47,7 @@ module ActionDispatch elsif options.any? options_proc(options) elsif path.respond_to?(:call) - proc { |params, request| path.call(params, request) } + path elsif block if block.arity < 2 msg = "redirect blocks with arity of #{block.arity} are deprecated. Your block must take 2 parameters: the environment, and a request object" -- cgit v1.2.3 From a8a4264858d5eac4f11ce6545f63e854bbc35759 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 31 Oct 2011 16:36:48 -0400 Subject: make sure to require the right deprecation warning file --- actionpack/lib/action_dispatch/routing/redirection.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb index d61320980d..a234e26151 100644 --- a/actionpack/lib/action_dispatch/routing/redirection.rb +++ b/actionpack/lib/action_dispatch/routing/redirection.rb @@ -1,4 +1,5 @@ require 'action_dispatch/http/request' +require 'active_support/deprecation/reporting' module ActionDispatch module Routing -- cgit v1.2.3 From 0809c675ef5831852b7c1aa8497402b2beff5185 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 31 Oct 2011 17:49:20 -0400 Subject: remove the :path feature to redirects, since it cannot work --- .../lib/action_dispatch/routing/redirection.rb | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb index a234e26151..cd9ba5a4db 100644 --- a/actionpack/lib/action_dispatch/routing/redirection.rb +++ b/actionpack/lib/action_dispatch/routing/redirection.rb @@ -68,23 +68,15 @@ module ActionDispatch def options_proc(options) proc do |params, request| - path = if options[:path].nil? - request.path - elsif params.empty? || !options[:path].match(/%\{\w*\}/) - options.delete(:path) - else - (options.delete(:path) % params) - end - - default_options = { + url_options = { :protocol => request.protocol, - :host => request.host, - :port => request.optional_port, - :path => path, - :params => request.query_parameters - } + :host => request.host, + :port => request.optional_port, + :path => request.path, + :params => request.query_parameters + }.merge options - ActionDispatch::Http::URL.url_for(options.reverse_merge(default_options)) + ActionDispatch::Http::URL.url_for url_options end end -- cgit v1.2.3 From d34efdd260d7a894537267a6186f16abe1b9335c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 31 Oct 2011 18:18:04 -0400 Subject: moving redirection to objects --- .../lib/action_dispatch/routing/redirection.rb | 103 ++++++++++++--------- 1 file changed, 58 insertions(+), 45 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb index cd9ba5a4db..35dabae1f2 100644 --- a/actionpack/lib/action_dispatch/routing/redirection.rb +++ b/actionpack/lib/action_dispatch/routing/redirection.rb @@ -3,6 +3,54 @@ require 'active_support/deprecation/reporting' module ActionDispatch module Routing + class Redirect + attr_reader :status, :block + + def initialize(status, block) + @status = status + @block = block + end + + def call(env) + req = Request.new(env) + + uri = URI.parse(path(req.symbolized_path_parameters, req)) + uri.scheme ||= req.scheme + uri.host ||= req.host + uri.port ||= req.port unless req.standard_port? + + body = %(You are being redirected.) + + headers = { + 'Location' => uri.to_s, + 'Content-Type' => 'text/html', + 'Content-Length' => body.length.to_s + } + + [ status, headers, [body] ] + end + + def path(params, request) + block.call params, request + end + end + + class OptionRedirect < Redirect + alias :options :block + + def path(params, request) + url_options = { + :protocol => request.protocol, + :host => request.host, + :port => request.optional_port, + :path => request.path, + :params => request.query_parameters + }.merge options + + ActionDispatch::Http::URL.url_for url_options + end + end + module Redirection # Redirect any path to another path: @@ -43,66 +91,31 @@ module ActionDispatch path = args.shift - path_proc = if path.is_a?(String) - proc { |params, request| (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params) } + if path.is_a?(String) + block_redirect status, lambda { |params, request| + (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params) + } elsif options.any? - options_proc(options) + OptionRedirect.new(status, options) elsif path.respond_to?(:call) - path + block_redirect status, path elsif block if block.arity < 2 msg = "redirect blocks with arity of #{block.arity} are deprecated. Your block must take 2 parameters: the environment, and a request object" ActiveSupport::Deprecation.warn msg - lambda { |params, _| block.call(params) } + block_redirect status, lambda { |params, _| block.call(params) } else - block + block_redirect status, block end else raise ArgumentError, "redirection argument not supported" end - - redirection_proc(status, path_proc) end private - - def options_proc(options) - proc do |params, request| - url_options = { - :protocol => request.protocol, - :host => request.host, - :port => request.optional_port, - :path => request.path, - :params => request.query_parameters - }.merge options - - ActionDispatch::Http::URL.url_for url_options - end - end - - def redirection_proc(status, path_proc) - lambda do |env| - req = Request.new(env) - - params = [req.symbolized_path_parameters, req] - - uri = URI.parse(path_proc.call(*params)) - uri.scheme ||= req.scheme - uri.host ||= req.host - uri.port ||= req.port unless req.standard_port? - - body = %(You are being redirected.) - - headers = { - 'Location' => uri.to_s, - 'Content-Type' => 'text/html', - 'Content-Length' => body.length.to_s - } - - [ status, headers, [body] ] - end + def block_redirect(status, path_proc) + Redirect.new status, path_proc end - end end end -- cgit v1.2.3 From 99d94f126d05398ec0917d75253ab1548bc54ba3 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 1 Nov 2011 15:53:02 -0200 Subject: Refactoring the redirect method for the router api. --- .../lib/action_dispatch/routing/redirection.rb | 44 ++++++++++------------ 1 file changed, 19 insertions(+), 25 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb index 35dabae1f2..b7df456e91 100644 --- a/actionpack/lib/action_dispatch/routing/redirection.rb +++ b/actionpack/lib/action_dispatch/routing/redirection.rb @@ -3,7 +3,7 @@ require 'active_support/deprecation/reporting' module ActionDispatch module Routing - class Redirect + class Redirect # :nodoc: attr_reader :status, :block def initialize(status, block) @@ -35,7 +35,7 @@ module ActionDispatch end end - class OptionRedirect < Redirect + class OptionRedirect < Redirect # :nodoc: alias :options :block def path(params, request) @@ -89,33 +89,27 @@ module ActionDispatch options = args.last.is_a?(Hash) ? args.pop : {} status = options.delete(:status) || 301 + return OptionRedirect.new(status, options) if options.any? + path = args.shift - if path.is_a?(String) - block_redirect status, lambda { |params, request| - (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params) - } - elsif options.any? - OptionRedirect.new(status, options) - elsif path.respond_to?(:call) - block_redirect status, path - elsif block - if block.arity < 2 - msg = "redirect blocks with arity of #{block.arity} are deprecated. Your block must take 2 parameters: the environment, and a request object" - ActiveSupport::Deprecation.warn msg - block_redirect status, lambda { |params, _| block.call(params) } - else - block_redirect status, block - end - else - raise ArgumentError, "redirection argument not supported" - end - end + block = lambda { |params, request| + (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params) + } if String === path + + block = path if path.respond_to? :call - private - def block_redirect(status, path_proc) - Redirect.new status, path_proc + # :FIXME: remove in Rails 4.0 + if block && block.respond_to?(:arity) && block.arity < 2 + msg = "redirect blocks with arity of #{block.arity} are deprecated. Your block must take 2 parameters: the environment, and a request object" + ActiveSupport::Deprecation.warn msg + block = lambda { |params, _| block.call(params) } end + + raise ArgumentError, "redirection argument not supported" unless block + + Redirect.new status, block + end end end end -- cgit v1.2.3 From 396ef44be48dbcbc75d313980d0ba272a6200099 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 18 Nov 2011 10:36:02 -0800 Subject: Revert "make sure to require the right deprecation warning file" This reverts commit 9d725e3df502a07222f35576108eb2df2bd88259. --- actionpack/lib/action_dispatch/routing/redirection.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb index b7df456e91..330400e139 100644 --- a/actionpack/lib/action_dispatch/routing/redirection.rb +++ b/actionpack/lib/action_dispatch/routing/redirection.rb @@ -1,5 +1,4 @@ require 'action_dispatch/http/request' -require 'active_support/deprecation/reporting' module ActionDispatch module Routing -- cgit v1.2.3 From d806ea20506f824efbe2c8b69928929977cc3a59 Mon Sep 17 00:00:00 2001 From: Arun Agrawal Date: Sat, 19 Nov 2011 11:29:29 +0530 Subject: Warning removed for shadowing variable --- actionpack/lib/action_dispatch/routing/mapper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 21624bcfc2..e3ad3f9ba7 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1252,7 +1252,7 @@ module ActionDispatch raise ArgumentError, "Unknown scope #{on.inspect} given to :on" end - paths.each { |path| decomposed_match(path, options.dup) } + paths.each { |_path| decomposed_match(_path, options.dup) } self end -- cgit v1.2.3 From 3d2bd6938586086fbce4d6c087e4f9cd528e0212 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sat, 19 Nov 2011 20:35:54 -0800 Subject: Revert "copy options keys to the right place so that undo will work correctly" This reverts commit 3178cc9a80262d3bf7754f3507ef60243b46634f. --- actionpack/lib/action_dispatch/routing/mapper.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index e3ad3f9ba7..7947e9d393 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -603,12 +603,9 @@ module ActionDispatch options[:constraints] ||= {} unless options[:constraints].is_a?(Hash) - options[:blocks] = options[:constraints] - options[:constraints] = {} + block, options[:constraints] = options[:constraints], {} end - options[:options] = options - scope_options.each do |option| if value = options.delete(option) recover[option] = @scope[option] @@ -616,12 +613,21 @@ module ActionDispatch end end + recover[:block] = @scope[:blocks] + @scope[:blocks] = merge_blocks_scope(@scope[:blocks], block) + + recover[:options] = @scope[:options] + @scope[:options] = merge_options_scope(@scope[:options], options) + yield self ensure scope_options.each do |option| @scope[option] = recover[option] if recover.has_key?(option) end + + @scope[:options] = recover[:options] + @scope[:blocks] = recover[:block] end # Scopes routes to a specific controller -- cgit v1.2.3 From a9e8cf78fda696738f63e726796f6232c3751603 Mon Sep 17 00:00:00 2001 From: lest Date: Mon, 21 Nov 2011 20:13:54 +0300 Subject: add ActionController::Metal#show_detailed_exceptions? --- .../lib/action_dispatch/middleware/show_exceptions.rb | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index 2fa68c64c5..569063f4db 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -38,9 +38,8 @@ module ActionDispatch "application's log file and/or the web server's log file to find out what " << "went wrong."]] - def initialize(app, consider_all_requests_local = false) + def initialize(app) @app = app - @consider_all_requests_local = consider_all_requests_local end def call(env) @@ -65,11 +64,10 @@ module ActionDispatch log_error(exception) exception = original_exception(exception) - request = Request.new(env) - if @consider_all_requests_local || request.local? - rescue_action_locally(request, exception) + if env['action_dispatch.show_detailed_exceptions'] == true + rescue_action_diagnostics(env, exception) else - rescue_action_in_public(exception) + rescue_action_error_page(exception) end rescue Exception => failsafe_error $stderr.puts "Error during failsafe response: #{failsafe_error}\n #{failsafe_error.backtrace * "\n "}" @@ -78,9 +76,9 @@ module ActionDispatch # Render detailed diagnostics for unhandled exceptions rescued from # a controller action. - def rescue_action_locally(request, exception) + def rescue_action_diagnostics(env, exception) template = ActionView::Base.new([RESCUES_TEMPLATE_PATH], - :request => request, + :request => Request.new(env), :exception => exception, :application_trace => application_trace(exception), :framework_trace => framework_trace(exception), @@ -98,7 +96,7 @@ module ActionDispatch # it will first attempt to render the file at public/500.da.html # then attempt to render public/500.html. If none of them exist, # the body of the response will be left empty. - def rescue_action_in_public(exception) + def rescue_action_error_page(exception) status = status_code(exception) locale_path = "#{public_path}/#{status}.#{I18n.locale}.html" if I18n.locale path = "#{public_path}/#{status}.html" -- cgit v1.2.3 From 3a1d51959bf569a7419fe8ab9416b338334b4800 Mon Sep 17 00:00:00 2001 From: lest Date: Tue, 22 Nov 2011 17:36:58 +0300 Subject: deprecation warning, changelog entry --- actionpack/lib/action_dispatch/middleware/show_exceptions.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index 569063f4db..52dce4cc81 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -2,6 +2,7 @@ require 'active_support/core_ext/exception' require 'action_controller/metal/exceptions' require 'active_support/notifications' require 'action_dispatch/http/request' +require 'active_support/deprecation' module ActionDispatch # This middleware rescues any exception returned by the application and renders @@ -38,7 +39,8 @@ module ActionDispatch "application's log file and/or the web server's log file to find out what " << "went wrong."]] - def initialize(app) + def initialize(app, consider_all_requests_local = nil) + ActiveSupport::Deprecation.warn "Passing consider_all_requests_local option to ActionDispatch::ShowExceptions middleware no longer works" unless consider_all_requests_local.nil? @app = app end -- cgit v1.2.3 From ea70e027b63a1b8bfe4087a4de978ad4eef5575b Mon Sep 17 00:00:00 2001 From: kennyj Date: Wed, 23 Nov 2011 23:49:43 +0900 Subject: Remove unreachable code, and add additional testcases. --- actionpack/lib/action_dispatch/middleware/params_parser.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'actionpack/lib/action_dispatch') diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb index d4208ca96e..84e3dd16dd 100644 --- a/actionpack/lib/action_dispatch/middleware/params_parser.rb +++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb @@ -54,12 +54,7 @@ module ActionDispatch rescue Exception => e # YAML, XML or Ruby code block errors logger.debug "Error occurred while parsing request parameters.\nContents:\n\n#{request.raw_post}" - raise - { "body" => request.raw_post, - "content_type" => request.content_mime_type, - "content_length" => request.content_length, - "exception" => "#{e.message} (#{e.class})", - "backtrace" => e.backtrace } + raise e end def content_type_from_legacy_post_data_format_header(env) -- cgit v1.2.3