aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--actionpack/CHANGELOG.md18
-rw-r--r--actionpack/lib/action_dispatch/routing/redirection.rb18
-rw-r--r--actionpack/test/dispatch/prefix_generation_test.rb100
-rw-r--r--activemodel/CHANGELOG.md4
-rw-r--r--activemodel/lib/active_model/secure_password.rb2
-rw-r--r--activemodel/test/cases/secure_password_test.rb8
-rw-r--r--activerecord/lib/active_record/associations/join_dependency.rb77
-rw-r--r--activerecord/lib/active_record/associations/join_dependency/join_part.rb25
-rw-r--r--activerecord/lib/active_record/relation/finder_methods.rb2
-rw-r--r--activerecord/lib/active_record/relation/merger.rb2
-rw-r--r--activerecord/lib/active_record/relation/query_methods.rb6
-rw-r--r--guides/source/active_record_validations.md2
-rw-r--r--railties/lib/rails/cli.rb1
13 files changed, 192 insertions, 73 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 7af9165605..9fb914ac40 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,21 @@
+* Respect `SCRIPT_NAME` when using `redirect` with a relative path
+
+ Example:
+ # application routes.rb
+ mount BlogEngine => '/blog'
+
+ # engine routes.rb
+ get '/admin' => redirect('admin/dashboard')
+
+ This now redirects to the path `/blog/admin/dashboard`, whereas before it would've
+ generated an invalid url because there would be no slash between the host name and
+ the path. It also allows redirects to work where the application is deployed to a
+ subdirectory of a website.
+
+ Fixes #7977
+
+ *Andrew White*
+
* Fixing repond_with working directly on the options hash
This fixes an issue where the respond_with worked directly with the given
options hash, so that if a user relied on it after calling respond_with,
diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb
index 68094f129f..3e54c7e71c 100644
--- a/actionpack/lib/action_dispatch/routing/redirection.rb
+++ b/actionpack/lib/action_dispatch/routing/redirection.rb
@@ -30,6 +30,10 @@ module ActionDispatch
uri.host ||= req.host
uri.port ||= req.port unless req.standard_port?
+ if relative_path?(uri.path)
+ uri.path = "#{req.script_name}/#{uri.path}"
+ end
+
body = %(<html><body>You are being <a href="#{ERB::Util.h(uri.to_s)}">redirected</a>.</body></html>)
headers = {
@@ -48,6 +52,11 @@ module ActionDispatch
def inspect
"redirect(#{status})"
end
+
+ private
+ def relative_path?(path)
+ path && !path.empty? && path[0] != '/'
+ end
end
class PathRedirect < Redirect
@@ -81,6 +90,11 @@ module ActionDispatch
url_options[:path] = (url_options[:path] % escape_path(params))
end
+ if relative_path?(url_options[:path])
+ url_options[:path] = "/#{url_options[:path]}"
+ url_options[:script_name] = request.script_name
+ end
+
ActionDispatch::Http::URL.url_for url_options
end
@@ -104,6 +118,10 @@ module ActionDispatch
#
# get 'docs/:article', to: redirect('/wiki/%{article}')
#
+ # Note that if you return a path without a leading slash then the url is prefixed with the
+ # current SCRIPT_NAME environment variable. This is typically '/' but may be different in
+ # a mounted engine or where the application is deployed to a subdirectory of a website.
+ #
# Alternatively you can use one of the other syntaxes:
#
# The block version of redirect allows for the easy encapsulation of any logic associated with
diff --git a/actionpack/test/dispatch/prefix_generation_test.rb b/actionpack/test/dispatch/prefix_generation_test.rb
index 113608ecf4..e519fff51e 100644
--- a/actionpack/test/dispatch/prefix_generation_test.rb
+++ b/actionpack/test/dispatch/prefix_generation_test.rb
@@ -31,6 +31,14 @@ module TestGenerationPrefix
get "/polymorphic_path_for_engine", :to => "inside_engine_generating#polymorphic_path_for_engine"
get "/conflicting_url", :to => "inside_engine_generating#conflicting"
get "/foo", :to => "never#invoked", :as => :named_helper_that_should_be_invoked_only_in_respond_to_test
+
+ get "/relative_path_redirect", :to => redirect("foo")
+ get "/relative_option_redirect", :to => redirect(:path => "foo")
+ get "/relative_custom_redirect", :to => redirect { |params, request| "foo" }
+
+ get "/absolute_path_redirect", :to => redirect("/foo")
+ get "/absolute_option_redirect", :to => redirect(:path => "/foo")
+ get "/absolute_custom_redirect", :to => redirect { |params, request| "/foo" }
end
routes
@@ -182,6 +190,48 @@ module TestGenerationPrefix
assert_equal "engine", last_response.body
end
+ test "[ENGINE] relative path redirect uses SCRIPT_NAME from request" do
+ get "/awesome/blog/relative_path_redirect"
+ assert_equal 301, last_response.status
+ assert_equal "http://example.org/awesome/blog/foo", last_response.headers["Location"]
+ assert_equal %(<html><body>You are being <a href="http://example.org/awesome/blog/foo">redirected</a>.</body></html>), last_response.body
+ end
+
+ test "[ENGINE] relative option redirect uses SCRIPT_NAME from request" do
+ get "/awesome/blog/relative_option_redirect"
+ assert_equal 301, last_response.status
+ assert_equal "http://example.org/awesome/blog/foo", last_response.headers["Location"]
+ assert_equal %(<html><body>You are being <a href="http://example.org/awesome/blog/foo">redirected</a>.</body></html>), last_response.body
+ end
+
+ test "[ENGINE] relative custom redirect uses SCRIPT_NAME from request" do
+ get "/awesome/blog/relative_custom_redirect"
+ assert_equal 301, last_response.status
+ assert_equal "http://example.org/awesome/blog/foo", last_response.headers["Location"]
+ assert_equal %(<html><body>You are being <a href="http://example.org/awesome/blog/foo">redirected</a>.</body></html>), last_response.body
+ end
+
+ test "[ENGINE] absolute path redirect doesn't use SCRIPT_NAME from request" do
+ get "/awesome/blog/absolute_path_redirect"
+ assert_equal 301, last_response.status
+ assert_equal "http://example.org/foo", last_response.headers["Location"]
+ assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+ end
+
+ test "[ENGINE] absolute option redirect doesn't use SCRIPT_NAME from request" do
+ get "/awesome/blog/absolute_option_redirect"
+ assert_equal 301, last_response.status
+ assert_equal "http://example.org/foo", last_response.headers["Location"]
+ assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+ end
+
+ test "[ENGINE] absolute custom redirect doesn't use SCRIPT_NAME from request" do
+ get "/awesome/blog/absolute_custom_redirect"
+ assert_equal 301, last_response.status
+ assert_equal "http://example.org/foo", last_response.headers["Location"]
+ assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+ end
+
# Inside Application
test "[APP] generating engine's route includes prefix" do
get "/generate"
@@ -281,6 +331,14 @@ module TestGenerationPrefix
routes = ActionDispatch::Routing::RouteSet.new
routes.draw do
get "/posts/:id", :to => "posts#show", :as => :post
+
+ get "/relative_path_redirect", :to => redirect("foo")
+ get "/relative_option_redirect", :to => redirect(:path => "foo")
+ get "/relative_custom_redirect", :to => redirect { |params, request| "foo" }
+
+ get "/absolute_path_redirect", :to => redirect("/foo")
+ get "/absolute_option_redirect", :to => redirect(:path => "/foo")
+ get "/absolute_custom_redirect", :to => redirect { |params, request| "/foo" }
end
routes
@@ -331,5 +389,47 @@ module TestGenerationPrefix
get "/posts/1"
assert_equal "/posts/1", last_response.body
end
+
+ test "[ENGINE] relative path redirect uses SCRIPT_NAME from request" do
+ get "/relative_path_redirect"
+ assert_equal 301, last_response.status
+ assert_equal "http://example.org/foo", last_response.headers["Location"]
+ assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+ end
+
+ test "[ENGINE] relative option redirect uses SCRIPT_NAME from request" do
+ get "/relative_option_redirect"
+ assert_equal 301, last_response.status
+ assert_equal "http://example.org/foo", last_response.headers["Location"]
+ assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+ end
+
+ test "[ENGINE] relative custom redirect uses SCRIPT_NAME from request" do
+ get "/relative_custom_redirect"
+ assert_equal 301, last_response.status
+ assert_equal "http://example.org/foo", last_response.headers["Location"]
+ assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+ end
+
+ test "[ENGINE] absolute path redirect doesn't use SCRIPT_NAME from request" do
+ get "/absolute_path_redirect"
+ assert_equal 301, last_response.status
+ assert_equal "http://example.org/foo", last_response.headers["Location"]
+ assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+ end
+
+ test "[ENGINE] absolute option redirect doesn't use SCRIPT_NAME from request" do
+ get "/absolute_option_redirect"
+ assert_equal 301, last_response.status
+ assert_equal "http://example.org/foo", last_response.headers["Location"]
+ assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+ end
+
+ test "[ENGINE] absolute custom redirect doesn't use SCRIPT_NAME from request" do
+ get "/absolute_custom_redirect"
+ assert_equal 301, last_response.status
+ assert_equal "http://example.org/foo", last_response.headers["Location"]
+ assert_equal %(<html><body>You are being <a href="http://example.org/foo">redirected</a>.</body></html>), last_response.body
+ end
end
end
diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md
index eb21b69163..e8602ecbcf 100644
--- a/activemodel/CHANGELOG.md
+++ b/activemodel/CHANGELOG.md
@@ -1,3 +1,7 @@
+* Fix `has_secure_password` to honor bcrypt-ruby's cost attribute.
+
+ *T.J. Schuck*
+
* Updated the `ActiveModel::Dirty#changed_attributes` method to be indifferent between using
symbols and strings as keys.
diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb
index 17fafe4be9..f87c36e39e 100644
--- a/activemodel/lib/active_model/secure_password.rb
+++ b/activemodel/lib/active_model/secure_password.rb
@@ -103,7 +103,7 @@ module ActiveModel
def password=(unencrypted_password)
unless unencrypted_password.blank?
@password = unencrypted_password
- cost = ActiveModel::SecurePassword.min_cost ? BCrypt::Engine::MIN_COST : BCrypt::Engine::DEFAULT_COST
+ cost = ActiveModel::SecurePassword.min_cost ? BCrypt::Engine::MIN_COST : BCrypt::Engine.cost
self.password_digest = BCrypt::Password.create(unencrypted_password, cost: cost)
end
end
diff --git a/activemodel/test/cases/secure_password_test.rb b/activemodel/test/cases/secure_password_test.rb
index 98e5c747d5..41d0b2263e 100644
--- a/activemodel/test/cases/secure_password_test.rb
+++ b/activemodel/test/cases/secure_password_test.rb
@@ -82,6 +82,14 @@ class SecurePasswordTest < ActiveModel::TestCase
assert_equal BCrypt::Engine::DEFAULT_COST, @user.password_digest.cost
end
+ test "Password digest cost honors bcrypt cost attribute when min_cost is false" do
+ ActiveModel::SecurePassword.min_cost = false
+ BCrypt::Engine.cost = 5
+
+ @user.password = "secret"
+ assert_equal BCrypt::Engine.cost, @user.password_digest.cost
+ end
+
test "Password digest cost can be set to bcrypt min cost to speed up tests" do
ActiveModel::SecurePassword.min_cost = true
diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb
index e2d4cf4378..6e08f67286 100644
--- a/activerecord/lib/active_record/associations/join_dependency.rb
+++ b/activerecord/lib/active_record/associations/join_dependency.rb
@@ -61,32 +61,26 @@ module ActiveRecord
build tree, @join_root, Arel::InnerJoin
end
- def graft(*associations)
- associations.reject { |join_node|
- find_node join_node
- }.each { |join_node|
- parent = find_node(join_node.parent) || join_root
- reflection = join_node.reflection
- type = join_node.join_type
-
- next if parent.children.find { |j| j.reflection == reflection }
- build_scalar reflection, parent, type
- }
- self
- end
-
def reflections
join_root.drop(1).map!(&:reflection)
end
- def join_relation(relation)
- nodes = join_root.drop 1
- nodes.each { |n| n.join_type = Arel::OuterJoin }
- relation.joins(nodes)
+ def merge_outer_joins!(other)
+ left = join_root
+ right = other.join_root
+
+ if left.match? right
+ merge_node left, right
+ else
+ # If the roots aren't the same, then deep copy the RHS to the LHS
+ left.children.concat right.children.map { |node|
+ deep_copy left, node
+ }
+ end
end
def join_constraints
- join_root.flat_map(&:join_constraints)
+ join_root.children.flat_map { |c| c.flat_map(&:join_constraints) }
end
def columns
@@ -103,39 +97,34 @@ module ActiveRecord
parents = {}
type_caster = result_set.column_type primary_key
- assoc = join_root.children
records = result_set.map { |row_hash|
primary_id = type_caster.type_cast row_hash[primary_key]
parent = parents[primary_id] ||= join_root.instantiate(row_hash)
- construct(parent, assoc, row_hash, result_set)
+ construct(parent, join_root, row_hash, result_set)
parent
}.uniq
- remove_duplicate_results!(base_klass, records, assoc)
+ remove_duplicate_results!(base_klass, records, join_root.children)
records
end
private
- def find_node(target_node)
- stack = target_node.parents << target_node
-
- left = [join_root]
- right = stack.shift
+ def merge_node(left, right)
+ intersection, missing = right.children.map { |node1|
+ [left.children.find { |node2| node1.match? node2 }, node1]
+ }.partition(&:first)
- loop {
- match = left.find { |l| l.match? right }
+ intersection.each { |l,r| merge_node l, r }
- if match
- return match if stack.empty?
+ left.children.concat missing.map { |_,node| deep_copy left, node }
+ end
- left = match.children
- right = stack.shift
- else
- return nil
- end
- }
+ def deep_copy(parent, node)
+ dup = build_join_association(node.reflection, parent, Arel::OuterJoin)
+ dup.children.concat node.children.map { |n| deep_copy dup, n }
+ dup
end
def remove_duplicate_results!(base, records, associations)
@@ -195,16 +184,16 @@ module ActiveRecord
JoinAssociation.new(reflection, join_root.to_a.length, parent, join_type, alias_tracker)
end
- def construct(parent, nodes, row, rs)
- nodes.sort_by { |k| k.name }.each do |node|
- association = construct_association(parent, node, row, rs)
- construct(association, node.children, row, rs) if association
+ def construct(ar_parent, parent, row, rs)
+ parent.children.each do |node|
+ association = construct_association(ar_parent, parent, node, row, rs)
+ construct(association, node, row, rs) if association
end
end
- def construct_association(record, join_part, row, rs)
- caster = rs.column_type(join_part.parent.aliased_primary_key)
- row_id = caster.type_cast row[join_part.parent.aliased_primary_key]
+ def construct_association(record, parent, join_part, row, rs)
+ caster = rs.column_type(parent.aliased_primary_key)
+ row_id = caster.type_cast row[parent.aliased_primary_key]
return if record.id != row_id
diff --git a/activerecord/lib/active_record/associations/join_dependency/join_part.rb b/activerecord/lib/active_record/associations/join_dependency/join_part.rb
index d536eaf613..d39ce94c99 100644
--- a/activerecord/lib/active_record/associations/join_dependency/join_part.rb
+++ b/activerecord/lib/active_record/associations/join_dependency/join_part.rb
@@ -29,9 +29,6 @@ module ActiveRecord
@children = []
end
- def join_constraints; []; end
- def join_relation(rel); rel; end
-
def name
reflection.name
end
@@ -40,25 +37,9 @@ module ActiveRecord
self.class == other.class
end
- def parents
- parents = []
- node = parent
- while node
- parents.unshift node
- node = node.parent
- end
- parents
- end
-
- def each
+ def each(&block)
yield self
- iter = lambda { |list|
- list.each { |item|
- yield item
- iter.call item.children
- }
- }
- iter.call children
+ children.each { |child| child.each(&block) }
end
def aliased_table
@@ -90,7 +71,7 @@ module ActiveRecord
unless @column_names_with_alias
@column_names_with_alias = []
- ([primary_key] + (column_names - [primary_key])).compact.each_with_index do |column_name, i|
+ column_names.each_with_index do |column_name, i|
@column_names_with_alias << [column_name, "#{aliased_prefix}_r#{i}"]
end
end
diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb
index 0132a02f83..d5eb1edf77 100644
--- a/activerecord/lib/active_record/relation/finder_methods.rb
+++ b/activerecord/lib/active_record/relation/finder_methods.rb
@@ -267,7 +267,7 @@ module ActiveRecord
def apply_join_dependency(relation, join_dependency)
relation = relation.except(:includes, :eager_load, :preload)
- relation = join_dependency.join_relation(relation)
+ relation = relation.joins join_dependency
if using_limitable_reflections?(join_dependency.reflections)
relation
diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb
index c05632e688..182b9ed89c 100644
--- a/activerecord/lib/active_record/relation/merger.rb
+++ b/activerecord/lib/active_record/relation/merger.rb
@@ -94,7 +94,7 @@ module ActiveRecord
[])
relation.joins! rest
- @relation = join_dependency.join_relation(relation)
+ @relation = relation.joins join_dependency
end
end
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index e1efc6a189..9c9690215a 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -928,7 +928,7 @@ module ActiveRecord
:string_join
when Hash, Symbol, Array
:association_join
- when ActiveRecord::Associations::JoinDependency::JoinAssociation
+ when ActiveRecord::Associations::JoinDependency
:stashed_join
when Arel::Nodes::Join
:join_node
@@ -950,7 +950,9 @@ module ActiveRecord
join_list
)
- join_dependency.graft(*stashed_association_joins)
+ stashed_association_joins.each do |dep|
+ join_dependency.merge_outer_joins! dep
+ end
joins = join_dependency.join_constraints
diff --git a/guides/source/active_record_validations.md b/guides/source/active_record_validations.md
index 339612ebc5..797b996357 100644
--- a/guides/source/active_record_validations.md
+++ b/guides/source/active_record_validations.md
@@ -528,7 +528,7 @@ If you validate the presence of an object associated via a `has_one` or
Since `false.blank?` is true, if you want to validate the presence of a boolean
field you should use `validates :field_name, inclusion: { in: [true, false] }`.
-The default error message is _"can't be empty"_.
+The default error message is _"can't be blank"_.
### `absence`
diff --git a/railties/lib/rails/cli.rb b/railties/lib/rails/cli.rb
index 20313a2608..dd70c272c6 100644
--- a/railties/lib/rails/cli.rb
+++ b/railties/lib/rails/cli.rb
@@ -1,4 +1,3 @@
-require 'rbconfig'
require 'rails/app_rails_loader'
# If we are inside a Rails application this method performs an exec and thus
generated by cgit v1.2.3 (git 2.25.1) at 2025-10-02 21:08:01 +0000