aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack
diff options
context:
space:
mode:
Diffstat (limited to 'actionpack')
-rw-r--r--actionpack/CHANGELOG.md10
-rw-r--r--actionpack/lib/action_controller/metal/hide_actions.rb2
-rw-r--r--actionpack/lib/action_controller/metal/strong_parameters.rb5
-rw-r--r--actionpack/lib/action_dispatch/http/cache.rb4
-rw-r--r--actionpack/lib/action_dispatch/http/request.rb22
-rw-r--r--actionpack/lib/action_dispatch/http/url.rb13
-rw-r--r--actionpack/lib/action_dispatch/middleware/remote_ip.rb2
-rw-r--r--actionpack/lib/action_dispatch/routing/route_set.rb10
-rw-r--r--actionpack/lib/action_pack/version.rb13
-rw-r--r--actionpack/test/abstract/callbacks_test.rb2
-rw-r--r--actionpack/test/abstract/collector_test.rb2
-rw-r--r--actionpack/test/controller/base_test.rb12
-rw-r--r--actionpack/test/dispatch/routing_test.rb37
-rw-r--r--actionpack/test/dispatch/url_generation_test.rb8
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