diff options
21 files changed, 115 insertions, 191 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 72d6c46782..8e36df1bcc 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -15,7 +15,7 @@ f.microphone :none f.usb :none f.fullscreen :self - f.payment :self, "https://secure-example.com" + f.payment :self, "https://secure.example.com" end ``` diff --git a/actionpack/lib/action_controller/metal/feature_policy.rb b/actionpack/lib/action_controller/metal/feature_policy.rb index eecca20dda..a627eabea6 100644 --- a/actionpack/lib/action_controller/metal/feature_policy.rb +++ b/actionpack/lib/action_controller/metal/feature_policy.rb @@ -19,7 +19,7 @@ module ActionController #:nodoc: # f.microphone :none # f.usb :none # f.fullscreen :self - # f.payment :self, "https://secure-example.com" + # f.payment :self, "https://secure.example.com" # end # # # Controller level policy diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index f2f57e6a36..504b1d3e98 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,8 @@ +* annotated_source_code returns an empty array so TemplateErrors without a + template in the backtrace are surfaced properly by DebugExceptions. + + *Guilherme Mansur*, *Kasper Timm Hansen* + * Add autoload for SyntaxErrorInTemplate so syntax errors are correctly raised by DebugExceptions. *Guilherme Mansur*, *Gannon McGibbon* diff --git a/actionview/lib/action_view/template/error.rb b/actionview/lib/action_view/template/error.rb index feceef15f9..7fc74a5502 100644 --- a/actionview/lib/action_view/template/error.rb +++ b/actionview/lib/action_view/template/error.rb @@ -81,8 +81,8 @@ module ActionView end end - def source_extract(indentation = 0, output = :console) - return unless num = line_number + def source_extract(indentation = 0) + return [] unless num = line_number num = num.to_i source_code = @template.source.split("\n") @@ -91,9 +91,9 @@ module ActionView end_on_line = [ num + SOURCE_CODE_RADIUS - 1, source_code.length].min indent = end_on_line.to_s.size + indentation - return unless source_code = source_code[start_on_line..end_on_line] + return [] unless source_code = source_code[start_on_line..end_on_line] - formatted_code_for(source_code, start_on_line, indent, output) + formatted_code_for(source_code, start_on_line, indent) end def sub_template_of(template_path) @@ -122,15 +122,11 @@ module ActionView end + file_name end - def formatted_code_for(source_code, line_counter, indent, output) - start_value = (output == :html) ? {} : [] - source_code.inject(start_value) do |result, line| + def formatted_code_for(source_code, line_counter, indent) + indent_template = "%#{indent}s: %s" + source_code.map do |line| line_counter += 1 - if output == :html - result.update(line_counter.to_s => "%#{indent}s %s\n" % ["", line]) - else - result << "%#{indent}s: %s" % [line_counter, line] - end + indent_template % [line_counter, line] end end end diff --git a/actionview/test/template/template_error_test.rb b/actionview/test/template/template_error_test.rb index c4dc88e4aa..643c29602b 100644 --- a/actionview/test/template/template_error_test.rb +++ b/actionview/test/template/template_error_test.rb @@ -34,4 +34,20 @@ class TemplateErrorTest < ActiveSupport::TestCase assert_equal "#<ActionView::Template::Error: original>", error.inspect end + + def test_annotated_source_code_returns_empty_array_if_source_cant_be_found + template = Class.new do + def identifier + "something" + end + end.new + + error = begin + raise + rescue + raise ActionView::Template::Error.new(template) rescue $! + end + + assert_equal [], error.annotated_source_code + end end diff --git a/activemodel/lib/active_model/error.rb b/activemodel/lib/active_model/error.rb index f7267fc7bf..ea141fe107 100644 --- a/activemodel/lib/active_model/error.rb +++ b/activemodel/lib/active_model/error.rb @@ -58,9 +58,9 @@ module ActiveModel end def strict_match?(attribute, type, **options) - return false unless match?(attribute, type, **options) + return false unless match?(attribute, type) - full_message == Error.new(@base, attribute, type, **options).full_message + options == @options.except(*CALLBACKS_OPTIONS + MESSAGE_OPTIONS) end def ==(other) @@ -74,7 +74,7 @@ module ActiveModel protected def attributes_for_hash - [@base, @attribute, @raw_type, @options] + [@base, @attribute, @raw_type, @options.except(*CALLBACKS_OPTIONS)] end end end diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 3de999308b..e9f92dba82 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -198,7 +198,7 @@ module ActiveModel matches.each do |error| @errors.delete(error) end - matches.map(&:message) + matches.map(&:message).presence end # When passed a symbol or a name of a method, returns an array of errors diff --git a/activemodel/test/cases/errors_test.rb b/activemodel/test/cases/errors_test.rb index baaf404f2e..9d31ed0a99 100644 --- a/activemodel/test/cases/errors_test.rb +++ b/activemodel/test/cases/errors_test.rb @@ -274,6 +274,28 @@ class ErrorsTest < ActiveModel::TestCase assert_equal [msg], person.errors[:name] end + test "added? when attribute was added through a collection" do + person = Person.new + person.errors.add(:"family_members.name", :too_long, count: 25) + assert person.errors.added?(:"family_members.name", :too_long, count: 25) + assert_not person.errors.added?(:"family_members.name", :too_long) + assert_not person.errors.added?(:"family_members.name", :too_long, name: "hello") + end + + test "added? ignores callback option" do + person = Person.new + + person.errors.add(:name, :too_long, if: -> { true }) + assert person.errors.added?(:name, :too_long) + end + + test "added? ignores message option" do + person = Person.new + + person.errors.add(:name, :too_long, message: proc { "foo" }) + assert person.errors.added?(:name, :too_long) + end + test "added? detects indifferent if a specific error was added to the object" do person = Person.new person.errors.add(:name, "cannot be blank") @@ -573,6 +595,12 @@ class ErrorsTest < ActiveModel::TestCase assert_not_equal errors_dup.details, errors.details end + test "delete returns nil when no errors were deleted" do + errors = ActiveModel::Errors.new(Person.new) + + assert_nil(errors.delete(:name)) + end + test "delete removes details on given attribute" do errors = ActiveModel::Errors.new(Person.new) errors.add(:name, :invalid) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 2af6d09b53..282c9fcf30 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -12,7 +12,6 @@ require "active_support/core_ext/hash/slice" require "active_support/core_ext/string/behavior" require "active_support/core_ext/kernel/singleton_class" require "active_support/core_ext/module/introspection" -require "active_support/core_ext/object/duplicable" require "active_support/core_ext/class/subclasses" require "active_record/attribute_decorators" require "active_record/define_callbacks" diff --git a/activerecord/test/models/ship.rb b/activerecord/test/models/ship.rb index 7973219a79..6bab7a1eb9 100644 --- a/activerecord/test/models/ship.rb +++ b/activerecord/test/models/ship.rb @@ -27,7 +27,8 @@ class ShipWithoutNestedAttributes < ActiveRecord::Base has_many :prisoners, inverse_of: :ship, foreign_key: :ship_id has_many :parts, class_name: "ShipPart", foreign_key: :ship_id - validates :name, presence: true + validates :name, presence: true, if: -> { true } + validates :name, presence: true, if: -> { true } end class Prisoner < ActiveRecord::Base diff --git a/activestorage/test/analyzer/video_analyzer_test.rb b/activestorage/test/analyzer/video_analyzer_test.rb index d30f49315a..57e094908a 100644 --- a/activestorage/test/analyzer/video_analyzer_test.rb +++ b/activestorage/test/analyzer/video_analyzer_test.rb @@ -13,7 +13,7 @@ class ActiveStorage::Analyzer::VideoAnalyzerTest < ActiveSupport::TestCase assert_equal 640, metadata[:width] assert_equal 480, metadata[:height] assert_equal [4, 3], metadata[:display_aspect_ratio] - assert_equal 5.166648, metadata[:duration] + assert_equal true, metadata[:duration].between?(4, 6) assert_not_includes metadata, :angle end @@ -24,7 +24,6 @@ class ActiveStorage::Analyzer::VideoAnalyzerTest < ActiveSupport::TestCase assert_equal 480, metadata[:width] assert_equal 640, metadata[:height] assert_equal [4, 3], metadata[:display_aspect_ratio] - assert_equal 5.227975, metadata[:duration] assert_equal 90, metadata[:angle] end diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index e055135bb4..a5063d0784 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -678,18 +678,15 @@ module ActiveSupport end def instrument(operation, key, options = nil) - log { "Cache #{operation}: #{normalize_key(key, options)}#{options.blank? ? "" : " (#{options.inspect})"}" } + if logger && logger.debug? && !silence? + logger.debug "Cache #{operation}: #{normalize_key(key, options)}#{options.blank? ? "" : " (#{options.inspect})"}" + end payload = { key: key } payload.merge!(options) if options.is_a?(Hash) ActiveSupport::Notifications.instrument("cache_#{operation}.active_support", payload) { yield(payload) } end - def log - return unless logger && logger.debug? && !silence? - logger.debug(yield) - end - def handle_expired_entry(entry, key, options) if entry && entry.expired? race_ttl = options[:race_condition_ttl].to_i diff --git a/activesupport/lib/active_support/cache/strategy/local_cache.rb b/activesupport/lib/active_support/cache/strategy/local_cache.rb index 39b32fc7f6..8e80946fbb 100644 --- a/activesupport/lib/active_support/cache/strategy/local_cache.rb +++ b/activesupport/lib/active_support/cache/strategy/local_cache.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require "active_support/core_ext/object/duplicable" require "active_support/core_ext/string/inflections" require "active_support/per_thread_registry" @@ -75,7 +74,10 @@ module ActiveSupport end def fetch_entry(key, options = nil) # :nodoc: - @data.fetch(key) { @data[key] = yield } + entry = @data.fetch(key) { @data[key] = yield } + dup_entry = entry.dup + dup_entry&.dup_value! + dup_entry end end diff --git a/activesupport/lib/active_support/core_ext/object/duplicable.rb b/activesupport/lib/active_support/core_ext/object/duplicable.rb index c78ee6bbfc..3ebcdca02b 100644 --- a/activesupport/lib/active_support/core_ext/object/duplicable.rb +++ b/activesupport/lib/active_support/core_ext/object/duplicable.rb @@ -28,96 +28,6 @@ class Object end end -class NilClass - begin - nil.dup - rescue TypeError - - # +nil+ is not duplicable: - # - # nil.duplicable? # => false - # nil.dup # => TypeError: can't dup NilClass - def duplicable? - false - end - end -end - -class FalseClass - begin - false.dup - rescue TypeError - - # +false+ is not duplicable: - # - # false.duplicable? # => false - # false.dup # => TypeError: can't dup FalseClass - def duplicable? - false - end - end -end - -class TrueClass - begin - true.dup - rescue TypeError - - # +true+ is not duplicable: - # - # true.duplicable? # => false - # true.dup # => TypeError: can't dup TrueClass - def duplicable? - false - end - end -end - -class Symbol - begin - :symbol.dup - - # Some symbols couldn't be duped in Ruby 2.4.0 only, due to a bug. - # This feature check catches any regression. - "symbol_from_string".to_sym.dup - rescue TypeError - - # Symbols are not duplicable: - # - # :my_symbol.duplicable? # => false - # :my_symbol.dup # => TypeError: can't dup Symbol - def duplicable? - false - end - end -end - -class Numeric - begin - 1.dup - rescue TypeError - - # Numbers are not duplicable: - # - # 3.duplicable? # => false - # 3.dup # => TypeError: can't dup Integer - def duplicable? - false - end - end -end - -require "bigdecimal" -class BigDecimal - # BigDecimals are duplicable: - # - # BigDecimal("1.2").duplicable? # => true - # BigDecimal("1.2").dup # => #<BigDecimal:...,'0.12E1',18(18)> - def duplicable? - true - end -end - class Method # Methods are not duplicable: # @@ -128,32 +38,12 @@ class Method end end -class Complex - begin - Complex(1).dup - rescue TypeError - - # Complexes are not duplicable: - # - # Complex(1).duplicable? # => false - # Complex(1).dup # => TypeError: can't copy Complex - def duplicable? - false - end - end -end - -class Rational - begin - Rational(1).dup - rescue TypeError - - # Rationals are not duplicable: - # - # Rational(1).duplicable? # => false - # Rational(1).dup # => TypeError: can't copy Rational - def duplicable? - false - end +class UnboundMethod + # Unbound methods are not duplicable: + # + # method(:puts).unbind.duplicable? # => false + # method(:puts).unbind.dup # => TypeError: allocator undefined for UnboundMethod + def duplicable? + false end end diff --git a/activesupport/test/cache/behaviors/local_cache_behavior.rb b/activesupport/test/cache/behaviors/local_cache_behavior.rb index baa38ba6ac..6f5d53c190 100644 --- a/activesupport/test/cache/behaviors/local_cache_behavior.rb +++ b/activesupport/test/cache/behaviors/local_cache_behavior.rb @@ -46,6 +46,15 @@ module LocalCacheBehavior end end + def test_local_cache_of_read_returns_a_copy_of_the_entry + @cache.with_local_cache do + @cache.write(:foo, type: "bar") + value = @cache.read(:foo) + assert_equal("bar", value.delete(:type)) + assert_equal({ type: "bar" }, @cache.read(:foo)) + end + end + def test_local_cache_of_read @cache.write("foo", "bar") @cache.with_local_cache do diff --git a/activesupport/test/core_ext/object/duplicable_test.rb b/activesupport/test/core_ext/object/duplicable_test.rb index c9af2cb624..a577c30c40 100644 --- a/activesupport/test/core_ext/object/duplicable_test.rb +++ b/activesupport/test/core_ext/object/duplicable_test.rb @@ -6,7 +6,7 @@ require "active_support/core_ext/object/duplicable" require "active_support/core_ext/numeric/time" class DuplicableTest < ActiveSupport::TestCase - RAISE_DUP = [method(:puts)] + RAISE_DUP = [method(:puts), method(:puts).unbind] ALLOW_DUP = ["1", "symbol_from_string".to_sym, Object.new, /foo/, [], {}, Time.now, Class.new, Module.new, BigDecimal("4.56"), nil, false, true, 1, 2.3, Complex(1), Rational(1)] def test_duplicable diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md index 8cb49ca6ae..f36cacfe8d 100644 --- a/guides/source/active_support_core_extensions.md +++ b/guides/source/active_support_core_extensions.md @@ -155,15 +155,6 @@ Complex(1).duplicable? # => true 1.method(:+).duplicable? # => false ``` -`duplicable?` matches the current Ruby version's `dup` behavior, -so results will vary according the version of Ruby you're using. -In Ruby 2.4, for example, Complex and Rational are not duplicable: - -```ruby -Rational(1).duplicable? # => false -Complex(1).duplicable? # => false -``` - WARNING: Any class can disallow duplication by removing `dup` and `clone` or raising exceptions from them. Thus only `rescue` can tell whether a given arbitrary object is duplicable. `duplicable?` depends on the hard-coded list above, but it is much faster than `rescue`. Use it only if you know the hard-coded list is enough in your use case. NOTE: Defined in `active_support/core_ext/object/duplicable.rb`. @@ -2358,10 +2349,6 @@ There's also a related idiom that uses the splat operator: [*object] ``` -which in Ruby 1.8 returns `[nil]` for `nil`, and calls to `Array(object)` otherwise. (Please if you know the exact behavior in 1.9 contact fxn.) - -Thus, in this case the behavior is different for `nil`, and the differences with `Kernel#Array` explained above apply to the rest of `object`s. - NOTE: Defined in `active_support/core_ext/array/wrap.rb`. ### Duplicating diff --git a/guides/source/layouts_and_rendering.md b/guides/source/layouts_and_rendering.md index 39935cd2ef..ce90a60e36 100644 --- a/guides/source/layouts_and_rendering.md +++ b/guides/source/layouts_and_rendering.md @@ -149,25 +149,6 @@ Rails knows that this view belongs to a different controller because of the embe render template: "products/show" ``` -#### Rendering an Arbitrary File - -The `render` method can also use a view that's entirely outside of your application: - -```ruby -render file: "/u/apps/warehouse_app/current/app/views/products/show" -``` - -The `:file` option takes an absolute file-system path. Of course, you need to have rights -to the view that you're using to render the content. - -NOTE: Using the `:file` option in combination with users input can lead to security problems -since an attacker could use this action to access security sensitive files in your file system. - -NOTE: By default, the file is rendered using the current layout. - -TIP: If you're running Rails on Microsoft Windows, you should use the `:file` option to -render a file, because Windows filenames do not have the same format as Unix filenames. - #### Wrapping it up The above three ways of rendering (rendering another template within the controller, rendering a template within another controller, and rendering an arbitrary file on the file system) are actually variants of the same action. @@ -178,17 +159,9 @@ In fact, in the BooksController class, inside of the update action where we want render :edit render action: :edit render "edit" -render "edit.html.erb" render action: "edit" -render action: "edit.html.erb" render "books/edit" -render "books/edit.html.erb" render template: "books/edit" -render template: "books/edit.html.erb" -render "/path/to/rails/app/views/books/edit" -render "/path/to/rails/app/views/books/edit.html.erb" -render file: "/path/to/rails/app/views/books/edit" -render file: "/path/to/rails/app/views/books/edit.html.erb" ``` Which one you use is really a matter of style and convention, but the rule of thumb is to use the simplest one that makes sense for the code you are writing. @@ -287,6 +260,23 @@ time. NOTE: Unless overridden, your response returned from this render option will be `text/plain`, as that is the default content type of Action Dispatch response. +#### Rendering raw file + +Rails can render a raw file from an absolute path. This is useful for +conditionally rendering static files like error pages. + +```ruby +render file: "#{Rails.root}/public/404.html", layout: false +``` + +This renders the raw file (it doesn't support ERB or other handlers). By +default it is rendered within the current layout. + +WARNING: Using the `:file` option in combination with users input can lead to security problems +since an attacker could use this action to access security sensitive files in your file system. + +TIP: `send_file` is often a faster and better option if a layout isn't required. + #### Options for `render` Calls to the `render` method generally accept five options: @@ -303,7 +293,7 @@ Calls to the `render` method generally accept five options: By default, Rails will serve the results of a rendering operation with the MIME content-type of `text/html` (or `application/json` if you use the `:json` option, or `application/xml` for the `:xml` option.). There are times when you might like to change this, and you can do so by setting the `:content_type` option: ```ruby -render file: filename, content_type: "application/rss" +render template: "feed", content_type: "application/rss" ``` ##### The `:layout` Option diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index 43c85fe16f..50d43ff69e 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -210,7 +210,7 @@ module Rails yaml = Pathname.new(path) erb = DummyERB.new(yaml.read) - YAML.load(erb.result) + YAML.load(erb.result) || {} else {} end diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/feature_policy.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/feature_policy.rb.tt index 355c7bd62a..a1c46695d2 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/feature_policy.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/feature_policy.rb.tt @@ -7,5 +7,5 @@ # f.microphone :none # f.usb :none # f.fullscreen :self -# f.payment :self, "https://secure-example.com" +# f.payment :self, "https://secure.example.com" # end diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index f6bec3242a..a05d86f738 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1793,6 +1793,11 @@ module ApplicationTests assert_equal [X, D], C.descendants end + test "load_database_yaml returns blank hash if configuration file is blank" do + app_file "config/database.yml", "" + app "development" + assert_equal({}, Rails.application.config.load_database_yaml) + end test "raises with proper error message if no database configuration found" do FileUtils.rm("#{app_path}/config/database.yml") |