diff options
Diffstat (limited to 'actionpack')
-rw-r--r-- | actionpack/CHANGELOG.md | 10 | ||||
-rw-r--r-- | actionpack/lib/action_controller/metal/hide_actions.rb | 2 | ||||
-rw-r--r-- | actionpack/lib/action_controller/metal/strong_parameters.rb | 5 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/http/cache.rb | 4 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/http/request.rb | 22 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/http/url.rb | 13 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/middleware/remote_ip.rb | 2 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/routing/route_set.rb | 10 | ||||
-rw-r--r-- | actionpack/lib/action_pack/version.rb | 13 | ||||
-rw-r--r-- | actionpack/test/abstract/callbacks_test.rb | 2 | ||||
-rw-r--r-- | actionpack/test/abstract/collector_test.rb | 2 | ||||
-rw-r--r-- | actionpack/test/controller/base_test.rb | 12 | ||||
-rw-r--r-- | actionpack/test/dispatch/routing_test.rb | 37 | ||||
-rw-r--r-- | actionpack/test/dispatch/url_generation_test.rb | 8 |
14 files changed, 96 insertions, 46 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 97bd2bf97f..8eac446603 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,15 @@ ## Rails 4.0.0 (unreleased) ## +* Raise an `ArgumentError` when a clashing named route is defined. + + *Trevor Turk* + +* Allow default url options to accept host with protocol such as `http://` + + config.action_mailer.default_url_options = { host: "http://mydomain.com" } + + *Richard Schneeman* + * Ensure that digest authentication responds with a 401 status when a basic header is received. diff --git a/actionpack/lib/action_controller/metal/hide_actions.rb b/actionpack/lib/action_controller/metal/hide_actions.rb index 2aa6b7adaf..af36ffa240 100644 --- a/actionpack/lib/action_controller/metal/hide_actions.rb +++ b/actionpack/lib/action_controller/metal/hide_actions.rb @@ -27,7 +27,7 @@ module ActionController end def visible_action?(action_name) - action_methods.include?(action_name) + not hidden_actions.include?(action_name) end # Overrides AbstractController::Base#action_methods to remove any methods diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index acad8a0799..23d70c9ea2 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -2,6 +2,7 @@ require 'active_support/core_ext/hash/indifferent_access' require 'active_support/core_ext/array/wrap' require 'active_support/rescuable' require 'action_dispatch/http/upload' +require 'stringio' module ActionController # Raised when a required parameter is missing. @@ -68,6 +69,8 @@ module ActionController # ActionController::UnpermittedParameters exception. The default value is <tt>:log</tt> # in test and development environments, +false+ otherwise. # + # Examples: + # # params = ActionController::Parameters.new # params.permitted? # => false # @@ -418,7 +421,7 @@ module ActionController # Declaration { comment_ids: [] }. array_of_permitted_scalars_filter(params, key) else - # Declaration { user: :name } or { user: [:name, :age, { adress: ... }] }. + # Declaration { user: :name } or { user: [:name, :age, { address: ... }] }. params[key] = each_element(value) do |element| if element.is_a?(Hash) element = self.class.new(element) unless element.respond_to?(:permit) diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb index 0d6015d993..f9b278349e 100644 --- a/actionpack/lib/action_dispatch/http/cache.rb +++ b/actionpack/lib/action_dispatch/http/cache.rb @@ -92,7 +92,7 @@ module ActionDispatch LAST_MODIFIED = "Last-Modified".freeze ETAG = "ETag".freeze CACHE_CONTROL = "Cache-Control".freeze - SPESHUL_KEYS = %w[extras no-cache max-age public must-revalidate] + SPECIAL_KEYS = %w[extras no-cache max-age public must-revalidate] def cache_control_segments if cache_control = self[CACHE_CONTROL] @@ -108,7 +108,7 @@ module ActionDispatch cache_control_segments.each do |segment| directive, argument = segment.split('=', 2) - if SPESHUL_KEYS.include? directive + if SPECIAL_KEYS.include? directive key = directive.tr('-', '_') cache_control[key.to_sym] = argument || true else diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index aff2172788..ebd87c40b5 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -158,29 +158,27 @@ module ActionDispatch # Returns the +String+ full path including params of the last URL requested. # - # app.get "/articles" - # app.request.fullpath # => "/articles" + # # get "/articles" + # request.fullpath # => "/articles" # - # app.get "/articles?page=2" - # app.request.fullpath # => "/articles?page=2" + # # get "/articles?page=2" + # request.fullpath # => "/articles?page=2" def fullpath @fullpath ||= super end - # Returns the original request URL as a +String+ + # Returns the original request URL as a +String+. # - # app.get "/articles?page=2" - # app.request.original_url - # # => "http://www.example.com/articles?page=2" + # # get "/articles?page=2" + # request.original_url # => "http://www.example.com/articles?page=2" def original_url base_url + original_fullpath end - # The +String+ MIME type of the request + # The +String+ MIME type of the request. # - # app.get "/articles" - # app.request.media_type - # # => "application/x-www-form-urlencoded" + # # get "/articles" + # request.media_type # => "application/x-www-form-urlencoded" def media_type content_mime_type.to_s end diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 97ac462411..ab5399c8ea 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -59,8 +59,9 @@ module ActionDispatch result = "" unless options[:only_path] + protocol = extract_protocol(options) unless options[:protocol] == false - result << (options[:protocol] || "http") + result << protocol result << ":" unless result.match(%r{:|//}) end result << "//" unless result.match("//") @@ -83,6 +84,16 @@ module ActionDispatch end end + # Extracts protocol http:// or https:// from options[:host] + # needs to be called whether the :protocol is being used or not + def extract_protocol(options) + if options[:host] && match = options[:host].match(/(^.*:\/\/)(.*)/) + options[:protocol] ||= match[1] + options[:host] = match[2] + end + options[:protocol] || "http" + end + def host_or_subdomain_and_domain(options) return options[:host] if !named_host?(options[:host]) || (options[:subdomain].nil? && options[:domain].nil?) diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb index 93a2b52996..8879291dbd 100644 --- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -101,7 +101,7 @@ module ActionDispatch (([0-9A-Fa-f]{1,4}:){0,4}:([0-9A-Fa-f]{1,4}:){1}((\b((25[0-5])|(1\d{2})|(2[0-4]\d)|(\d{1,2}))\b)\.){3}(\b((25[0-5])|(1\d{2})|(2[0-4]\d)|(\d{1,2}))\b)) | # ip v6 with compatible to v4 (::([0-9A-Fa-f]{1,4}:){0,5}((\b((25[0-5])|(1\d{2})|(2[0-4]\d) |(\d{1,2}))\b)\.){3}(\b((25[0-5])|(1\d{2})|(2[0-4]\d)|(\d{1,2}))\b)) | # ip v6 with compatible to v4 ([0-9A-Fa-f]{1,4}::([0-9A-Fa-f]{1,4}:){0,5}[0-9A-Fa-f]{1,4}) | # ip v6 with compatible to v4 - (::([0-9A-Fa-f]{1,4}:){0,6}[0-9A-Fa-f]{1,4}) | # ip v6 with double colon at the begining + (::([0-9A-Fa-f]{1,4}:){0,6}[0-9A-Fa-f]{1,4}) | # ip v6 with double colon at the beginning (([0-9A-Fa-f]{1,4}:){1,7}:) # ip v6 without ending )$) }x diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 619dd22ec1..7fb4719fa0 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -403,11 +403,19 @@ module ActionDispatch def add_route(app, conditions = {}, requirements = {}, defaults = {}, name = nil, anchor = true) raise ArgumentError, "Invalid route name: '#{name}'" unless name.blank? || name.to_s.match(/^[_a-z]\w*$/i) + if name && named_routes[name] + raise ArgumentError, "Invalid route name, already in use: '#{name}' \n" \ + "You may have defined two routes with the same name using the `:as` option, or " + "you may be overriding a route already defined by a resource with the same naming. " \ + "For the latter, you can restrict the routes created with `resources` as explained here: \n" \ + "http://guides.rubyonrails.org/routing.html#restricting-the-routes-created" + end + path = build_path(conditions.delete(:path_info), requirements, SEPARATORS, anchor) conditions = build_conditions(conditions, path.names.map { |x| x.to_sym }) route = @set.add_route(app, path, conditions, defaults, name) - named_routes[name] = route if name && !named_routes[name] + named_routes[name] = route if name route end diff --git a/actionpack/lib/action_pack/version.rb b/actionpack/lib/action_pack/version.rb index 5c87a9cd7c..b5e47d78d1 100644 --- a/actionpack/lib/action_pack/version.rb +++ b/actionpack/lib/action_pack/version.rb @@ -1,10 +1,11 @@ module ActionPack - module VERSION #:nodoc: - MAJOR = 4 - MINOR = 0 - TINY = 0 - PRE = "beta1" + # Returns the version of the currently loaded ActionPack as a Gem::Version + def self.version + Gem::Version.new "4.0.0.beta1" + end - STRING = [MAJOR, MINOR, TINY, PRE].compact.join('.') + module VERSION #:nodoc: + MAJOR, MINOR, TINY, PRE = ActionPack.version.segments + STRING = ActionPack.version.to_s end end diff --git a/actionpack/test/abstract/callbacks_test.rb b/actionpack/test/abstract/callbacks_test.rb index 1090af3060..8cba049485 100644 --- a/actionpack/test/abstract/callbacks_test.rb +++ b/actionpack/test/abstract/callbacks_test.rb @@ -259,7 +259,7 @@ module AbstractController end class TestCallbacksWithArgs < ActiveSupport::TestCase - test "callbacks still work when invoking process with multiple args" do + test "callbacks still work when invoking process with multiple arguments" do controller = CallbacksWithArgs.new controller.process(:index, " Howdy!") assert_equal "Hello world Howdy!", controller.response_body diff --git a/actionpack/test/abstract/collector_test.rb b/actionpack/test/abstract/collector_test.rb index c14d24905b..5709ad0378 100644 --- a/actionpack/test/abstract/collector_test.rb +++ b/actionpack/test/abstract/collector_test.rb @@ -42,7 +42,7 @@ module AbstractController end end - test "generated methods call custom with args received" do + test "generated methods call custom with arguments received" do collector = MyCollector.new collector.html collector.text(:foo) diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index 9b42e7631f..d4f18d55a6 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -68,6 +68,12 @@ class RecordIdentifierWithoutDeprecationController < ActionController::Base include ActionView::RecordIdentifier end +class ActionMissingController < ActionController::Base + def action_missing(action) + render :text => "Response for #{action}" + end +end + class ControllerClassTests < ActiveSupport::TestCase def test_controller_path @@ -186,6 +192,12 @@ class PerformActionTest < ActionController::TestCase assert_raise(AbstractController::ActionNotFound) { get :hidden_action } assert_raise(AbstractController::ActionNotFound) { get :another_hidden_action } end + + def test_action_missing_should_work + use_controller ActionMissingController + get :arbitrary_action + assert_equal "Response for arbitrary_action", @response.body + end end class UrlOptionsTest < ActionController::TestCase diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 2bf7056ff7..df359ba77d 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -2577,22 +2577,6 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_raises(ActionController::UrlGenerationError){ list_todo_path(:list_id => '2', :id => '1') } end - def test_named_routes_collision_is_avoided_unless_explicitly_given_as - draw do - scope :as => "routes" do - get "/c/:id", :as => :collision, :to => "collision#show" - get "/collision", :to => "collision#show" - get "/no_collision", :to => "collision#show", :as => nil - - get "/fc/:id", :as => :forced_collision, :to => "forced_collision#show" - get "/forced_collision", :as => :forced_collision, :to => "forced_collision#show" - end - end - - assert_equal "/c/1", routes_collision_path(1) - assert_equal "/fc/1", routes_forced_collision_path(1) - end - def test_redirect_argument_error routes = Class.new { include ActionDispatch::Routing::Redirection }.new assert_raises(ArgumentError) { routes.redirect Object.new } @@ -2604,9 +2588,6 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get "/c/:id", :as => :collision, :to => "collision#show" get "/collision", :to => "collision#show" get "/no_collision", :to => "collision#show", :as => nil - - get "/fc/:id", :as => :forced_collision, :to => "forced_collision#show" - get "/forced_collision", :as => :forced_collision, :to => "forced_collision#show" end end @@ -2657,6 +2638,24 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + def test_duplicate_route_name_raises_error + assert_raise(ArgumentError) do + draw do + get '/collision', :to => 'collision#show', :as => 'collision' + get '/duplicate', :to => 'duplicate#show', :as => 'collision' + end + end + end + + def test_duplicate_route_name_via_resources_raises_error + assert_raise(ArgumentError) do + draw do + resources :collisions + get '/collision', :to => 'collision#show', :as => 'collision' + end + end + end + def test_nested_route_in_nested_resource draw do resources :posts, :only => [:index, :show] do diff --git a/actionpack/test/dispatch/url_generation_test.rb b/actionpack/test/dispatch/url_generation_test.rb index e56e8ddc57..4123529092 100644 --- a/actionpack/test/dispatch/url_generation_test.rb +++ b/actionpack/test/dispatch/url_generation_test.rb @@ -48,6 +48,14 @@ module TestUrlGeneration https! assert_equal "http://www.example.com/foo", foo_url(:protocol => "http") end + + test "extracting protocol from host when protocol not present" do + assert_equal "httpz://www.example.com/foo", foo_url(host: "httpz://www.example.com", protocol: nil) + end + + test "formatting host when protocol is present" do + assert_equal "http://www.example.com/foo", foo_url(host: "httpz://www.example.com", protocol: "http://") + end end end |