Skip to content

Commit

Permalink
Merge pull request #50714 from jonathanhefner/follow-up-50677
Browse files Browse the repository at this point in the history
Prevent `CurrentAttributes` defaults from leaking
  • Loading branch information
jonathanhefner committed Jan 11, 2024
2 parents c5f3a81 + aa98bc3 commit 41a57cf
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 13 deletions.
20 changes: 7 additions & 13 deletions activesupport/lib/active_support/current_attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def attribute(*names, default: nil)

singleton_class.delegate(*names.flat_map { |name| [name, "#{name}="] }, to: :instance, as: self)

defaults.merge! names.index_with { default }
self.defaults = defaults.merge(names.index_with { default })
end

# Calls this callback before #reset is called on the instance. Used for resetting external collaborators that depend on current values.
Expand Down Expand Up @@ -188,12 +188,12 @@ def method_added(name)
end
end

class_attribute :defaults, instance_writer: false, default: {}
class_attribute :defaults, instance_writer: false, default: {}.freeze

attr_accessor :attributes

def initialize
@attributes = merge_defaults!({})
@attributes = resolve_defaults
end

# Expose one or more attributes within a block. Old values are returned after the block concludes.
Expand All @@ -213,20 +213,14 @@ def set(attributes, &block)
# Reset all attributes. Should be called before and after actions, when used as a per-request singleton.
def reset
run_callbacks :reset do
self.attributes = merge_defaults!({})
self.attributes = resolve_defaults
end
end

private
def merge_defaults!(attributes)
defaults.each_with_object(attributes) do |(name, default), values|
value =
case default
when Proc then default.call
else default.dup
end

values[name] = value
def resolve_defaults
defaults.transform_values do |value|
Proc === value ? value.call : value.dup
end
end
end
Expand Down
7 changes: 7 additions & 0 deletions activesupport/test/current_attributes_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,13 @@ def after_teardown
assert_equal true, Current.respond_to?("respond_to_test")
end

test "CurrentAttributes defaults do not leak between classes" do
Class.new(ActiveSupport::CurrentAttributes) { attribute :counter_integer, default: 100 }
Current.reset

assert_equal 0, Current.counter_integer
end

test "CurrentAttributes use fiber-local variables" do
previous_level = ActiveSupport::IsolatedExecutionState.isolation_level
ActiveSupport::IsolatedExecutionState.isolation_level = :fiber
Expand Down

0 comments on commit 41a57cf

Please sign in to comment.