From 65e692e68826bd0d5a31bfa66090fb7e5662de8b Mon Sep 17 00:00:00 2001 From: Craig Blanchette Date: Thu, 13 Aug 2026 20:31:47 -0400 Subject: [PATCH 1/6] Dispatch methods on collections in get_attribute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The no-arguments branch subscripted Hash and Array with the attribute name instead of calling it, which is only ever reached when the key/index lookup above has already missed — so the subscript could only miss too. On an Array it raised TypeError ({{ list.any? }}), and on a Hash it returned nil and silently rendered nothing. Collections now dispatch methods like any other object, while a real key or index still wins over a method of the same name. Also public_send rather than send: the respond_to? guard above already excludes private and protected methods, but an object with a loose respond_to_missing? can report true for one, and templates should only reach the public interface. Co-Authored-By: Claude Opus 5 --- lib/twig/extension/core.rb | 21 ++++++----- spec/lib/twig/extension/core_spec.rb | 52 ++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/lib/twig/extension/core.rb b/lib/twig/extension/core.rb index 2fc9b5f..c59aa8e 100644 --- a/lib/twig/extension/core.rb +++ b/lib/twig/extension/core.rb @@ -925,19 +925,22 @@ def self.get_attribute( kwargs = kwargs.transform_keys(&:to_sym) + # public_send, not send: respond_to? above already excludes private and + # protected methods, but an object with a loose respond_to_missing? + # can report true for one. Templates only reach the public interface. if positional.length.positive? && kwargs.empty? - object.send(attribute, *positional, &) + object.public_send(attribute, *positional, &) elsif positional.empty? && kwargs.length.positive? - object.send(attribute, **kwargs, &) + object.public_send(attribute, **kwargs, &) elsif positional.length.positive? && kwargs.length.positive? - object.send(attribute, *positional, **kwargs, &) + object.public_send(attribute, *positional, **kwargs, &) else - case object - when Hash, Array - object[attribute] - else - object.send(attribute, &) - end + # Reached only when the object responds to the attribute and the + # key/index lookup above did not match, so subscripting a Hash or + # Array here could only ever miss — it raised TypeError on arrays + # ({{ list.any? }}) and returned nil on hashes, silently rendering + # nothing. Collections dispatch methods like any other object. + object.public_send(attribute, &) end # Constant could be nil but we should return if we find it elsif (constant = get_constant(object, attribute.to_s)) && constant[0] == :found diff --git a/spec/lib/twig/extension/core_spec.rb b/spec/lib/twig/extension/core_spec.rb index df31e59..dcb9595 100644 --- a/spec/lib/twig/extension/core_spec.rb +++ b/spec/lib/twig/extension/core_spec.rb @@ -518,5 +518,57 @@ def foo ) ).to eq(3.14) end + + def attribute_of(object, attribute) + described_class.get_attribute( + environment, + instance_double(Twig::Source), + object, + attribute, + Twig::Template::METHOD_CALL + ) + end + + context 'with a collection' do + it 'calls methods on an array rather than subscripting with the name' do + expect(attribute_of(%w[a b], :any?)).to be(true) + expect(attribute_of(%w[a b], :size)).to eq(2) + expect(attribute_of([], :any?)).to be(false) + end + + it 'calls methods on a hash rather than returning nil' do + expect(attribute_of({ 'a' => 1 }, :any?)).to be(true) + expect(attribute_of({}, :any?)).to be(false) + end + + it 'still prefers a real key over a method of the same name' do + expect(attribute_of({ 'size' => 'from key' }, 'size')).to eq('from key') + expect(attribute_of({ 'first' => 'Ada' }, 'first')).to eq('Ada') + end + + it 'still prefers a real index over a method name' do + expect(attribute_of(%w[a b], 0)).to eq('a') + end + end + + context 'with a method the object only claims to respond to' do + let(:liar) do + Class.new do + def respond_to_missing?(name, include_all = false) + name.to_sym == :hidden || super + end + + private + + def hidden + 'private' + end + end.new + end + + it 'does not reach private methods' do + expect { attribute_of(liar, :hidden) }.to raise_error(NoMethodError) + end + end end end From 7b0d21463f82763d28d409db3c86bd9d9bf427e7 Mon Sep 17 00:00:00 2001 From: Craig Blanchette Date: Thu, 13 Aug 2026 20:33:18 -0400 Subject: [PATCH 2/6] Treat an out-of-range negative array index as missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bounds check was only an upper one, so every negative index passed it. Ruby indexes from the end with a negative, so -1 is genuinely the last element, but -99 on a three-element array is as absent as 99 is — and it fell through to `object[attribute] || object[attribute.to_s]`, where the String subscript raised TypeError out of the array. Because the exception came from inside the lookup rather than the strict check, `??` and `default()` couldn't suppress it either. Negative indices within range still resolve from the end; out of range now reports the same "can't find key" as an out-of-range positive, or returns nil when strict_variables is off. Co-Authored-By: Claude Opus 5 --- lib/twig/extension/core.rb | 10 +++++++- spec/lib/twig/extension/core_spec.rb | 34 ++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/lib/twig/extension/core.rb b/lib/twig/extension/core.rb index c59aa8e..ebd1675 100644 --- a/lib/twig/extension/core.rb +++ b/lib/twig/extension/core.rb @@ -867,7 +867,15 @@ def self.get_attribute( ) if type == Template::ARRAY_CALL || object.respond_to?(:[]) if object.respond_to?(:[]) && ( - (object.is_a?(Array) && attribute.is_a?(Integer) && attribute < object.length) || + # Ruby indexes from the end with a negative, so -1 is the last + # element and anything below -length is as absent as anything above + # length. Checking only the upper bound let every negative through + # to the lookup below, where an out-of-range one fell out of the + # array and into a String subscript: TypeError, not a Twig error. + ( + object.is_a?(Array) && attribute.is_a?(Integer) && + attribute >= -object.length && attribute < object.length + ) || ( object.respond_to?(:key?) && ( object.key?(attribute) || diff --git a/spec/lib/twig/extension/core_spec.rb b/spec/lib/twig/extension/core_spec.rb index dcb9595..f1417e0 100644 --- a/spec/lib/twig/extension/core_spec.rb +++ b/spec/lib/twig/extension/core_spec.rb @@ -529,6 +529,40 @@ def attribute_of(object, attribute) ) end + def subscript_of(object, attribute, env = environment) + described_class.get_attribute( + env, + instance_double(Twig::Source), + object, + attribute, + Twig::Template::ARRAY_CALL + ) + end + + context 'with an array index' do + let(:strict) { Twig::Environment.new(loader, { strict_variables: true }) } + + it 'reads from the end with a negative index' do + expect(subscript_of(%w[a b c], -1)).to eq('c') + expect(subscript_of(%w[a b c], -3)).to eq('a') + end + + it 'treats an out-of-range negative index as missing, not as a TypeError' do + expect { subscript_of(%w[a b c], -99, strict) }. + to raise_error(Twig::Error::Runtime, /-99/) + end + + it 'treats an out-of-range positive index as missing' do + expect { subscript_of(%w[a b c], 99, strict) }. + to raise_error(Twig::Error::Runtime, /99/) + end + + it 'returns nil for an out-of-range index when not strict' do + expect(subscript_of(%w[a b c], -99)).to be_nil + expect(subscript_of(%w[a b c], 99)).to be_nil + end + end + context 'with a collection' do it 'calls methods on an array rather than subscripting with the name' do expect(attribute_of(%w[a b], :any?)).to be(true) From 5ad260f9f8f89810e4012228f1be6d271d4c4676 Mon Sep 17 00:00:00 2001 From: Craig Blanchette Date: Thu, 13 Aug 2026 20:34:57 -0400 Subject: [PATCH 3/6] Preserve a key that holds nil or false in get_attribute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `object[attribute] || object[attribute.to_s]` cannot distinguish a key that is absent from a key that is present and holds a falsy value, so it fell through to the other form of the name and returned whatever that gave — usually nothing. { 'billable' => false } read as h['billable'] came back nil, and so did { billable: false } read as h[:billable]. The cases that appeared to work only did so by accident: when the name's form mismatched the key's, the fallback lookup happened to be the real one. The guard already knew which key matched; it just threw that away and re-derived it by fetching. subscript_key now returns the matching key (or a KEY_NOT_FOUND sentinel, since nil is both a valid key and a valid value) and the caller subscripts once with it. Where both forms of a name exist, the one the template asked for now wins rather than whichever is truthy. The array bounds check moves into the same helper, since it answers the same question. Co-Authored-By: Claude Opus 5 --- lib/twig/extension/core.rb | 62 +++++++++++++++++++--------- spec/lib/twig/extension/core_spec.rb | 40 ++++++++++++++++++ 2 files changed, 83 insertions(+), 19 deletions(-) diff --git a/lib/twig/extension/core.rb b/lib/twig/extension/core.rb index ebd1675..bf0e17c 100644 --- a/lib/twig/extension/core.rb +++ b/lib/twig/extension/core.rb @@ -5,6 +5,10 @@ module Extension class Core < Base DEFAULT_TRIM_CHARS = " \t\n\r\0\x0B" + # Returned by subscript_key when nothing matched. A sentinel rather than + # nil, because nil is a perfectly good hash key and a perfectly good value. + KEY_NOT_FOUND = Object.new.freeze + class << self include ActiveSupport::NumberHelper end @@ -860,33 +864,53 @@ def self.matches(regexp, string) raise Error::Runtime, "Invalid regular expression passed to matches: #{e.message}" end + # The key or index that +attribute+ matches on a subscriptable object, or + # KEY_NOT_FOUND when it matches none. + # + # Returning the key rather than the value is the point: it lets the caller + # subscript exactly once. Fetching first and testing the result instead — + # as `object[attribute] || object[attribute.to_s]` did — can't tell a key + # holding nil or false from a key that isn't there, so { 'billable' => + # false } came back as nil. + # + # @return [Object] the matching key, or KEY_NOT_FOUND + def self.subscript_key(object, attribute) + if object.is_a?(Array) + return KEY_NOT_FOUND unless attribute.is_a?(Integer) + + # Ruby indexes from the end with a negative, so -1 is the last element + # while anything below -length is as absent as anything above length. + in_bounds = attribute >= -object.length && attribute < object.length + + return in_bounds ? attribute : KEY_NOT_FOUND + end + + return KEY_NOT_FOUND unless object.respond_to?(:key?) + return attribute if object.key?(attribute) + + # A template writes h.foo, h['foo'] and h[:foo] for the same Ruby hash, + # so both forms of the name count as a match. + symbol = attribute.to_sym if attribute.respond_to?(:to_sym) + return symbol if !symbol.nil? && object.key?(symbol) + + string = attribute.to_s if attribute.respond_to?(:to_s) + return string if !string.nil? && object.key?(string) + + KEY_NOT_FOUND + end + # @param [Environment] environment def self.get_attribute( environment, source, object, attribute, type, arguments: {}, defined_test: false, ignore_strict_check: false, lineno: -1, & ) if type == Template::ARRAY_CALL || object.respond_to?(:[]) - if object.respond_to?(:[]) && ( - # Ruby indexes from the end with a negative, so -1 is the last - # element and anything below -length is as absent as anything above - # length. Checking only the upper bound let every negative through - # to the lookup below, where an out-of-range one fell out of the - # array and into a String subscript: TypeError, not a Twig error. - ( - object.is_a?(Array) && attribute.is_a?(Integer) && - attribute >= -object.length && attribute < object.length - ) || - ( - object.respond_to?(:key?) && ( - object.key?(attribute) || - (attribute.respond_to?(:to_sym) && object.key?(attribute.to_sym)) || - (attribute.respond_to?(:to_s) && object.key?(attribute.to_s)) - ) - ) - ) + key = object.respond_to?(:[]) ? subscript_key(object, attribute) : KEY_NOT_FOUND + + unless key.equal?(KEY_NOT_FOUND) return true if defined_test - return object[attribute] || (attribute.is_a?(String) ? object[attribute.to_sym] : object[attribute.to_s]) + return object[key] end if type == Template::ARRAY_CALL diff --git a/spec/lib/twig/extension/core_spec.rb b/spec/lib/twig/extension/core_spec.rb index f1417e0..1cb8304 100644 --- a/spec/lib/twig/extension/core_spec.rb +++ b/spec/lib/twig/extension/core_spec.rb @@ -539,6 +539,46 @@ def subscript_of(object, attribute, env = environment) ) end + context 'with a key holding a falsy value' do + it 'returns false for a string key reached by a string name' do + expect(subscript_of({ 'flag' => false }, 'flag')).to be(false) + expect(attribute_of({ 'flag' => false }, 'flag')).to be(false) + end + + it 'returns false for a symbol key reached by a symbol name' do + expect(subscript_of({ flag: false }, :flag)).to be(false) + end + + it 'returns false across the string/symbol divide' do + expect(subscript_of({ flag: false }, 'flag')).to be(false) + expect(subscript_of({ 'flag' => false }, :flag)).to be(false) + end + + it 'returns nil for a key that genuinely holds nil' do + expect(subscript_of({ 'flag' => nil }, 'flag')).to be_nil + end + + it 'prefers the key that matches over the other form of the name' do + # Both keys exist; the one the template asked for wins, even though it + # holds nil and the other holds something truthy. + expect(subscript_of({ 'a' => nil, a: 1 }, 'a')).to be_nil + expect(subscript_of({ 'a' => 1, a: nil }, :a)).to be_nil + end + + it 'reports a falsy value as defined' do + expect( + described_class.get_attribute( + environment, + instance_double(Twig::Source), + { 'flag' => false }, + 'flag', + Twig::Template::ARRAY_CALL, + defined_test: true + ) + ).to be(true) + end + end + context 'with an array index' do let(:strict) { Twig::Environment.new(loader, { strict_variables: true }) } From 6c208f8d6e450618a2754ee831a5c7bdd76f102a Mon Sep 17 00:00:00 2001 From: Craig Blanchette Date: Thu, 13 Aug 2026 20:37:03 -0400 Subject: [PATCH 4/6] Report a missing key the same way for bracket and dot access MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two paths out of get_attribute described the same failure differently. Dot access produced Twig's own wording — Key "x" for sequence/mapping with keys "a, b" does not exist — while bracket access produced "Can't find key x in " followed by the whole object inspected, which for anything the size of a request's parameters buries the one absent key under everything else. It also raised the bare message, so unlike its neighbour it carried no lineno or source. Both now build the message through key_not_found_message and raise with the source attached. Also drops the double space the old wording left when the object has no keys to list, as an array doesn't. Co-Authored-By: Claude Opus 5 --- lib/twig/extension/core.rb | 20 +++++++++-- spec/lib/twig/extension/core_spec.rb | 52 +++++++++++++++++++--------- 2 files changed, 52 insertions(+), 20 deletions(-) diff --git a/lib/twig/extension/core.rb b/lib/twig/extension/core.rb index bf0e17c..44c8854 100644 --- a/lib/twig/extension/core.rb +++ b/lib/twig/extension/core.rb @@ -899,6 +899,17 @@ def self.subscript_key(object, attribute) KEY_NOT_FOUND end + # Names the keys that do exist rather than dumping the object, which for + # anything the size of a request's parameters buries the one absent key + # under everything else. + # + # @return [String] + def self.key_not_found_message(object, attribute) + keys = object.respond_to?(:keys) ? " with keys \"#{object.keys.join(', ')}\"" : '' + + "Key \"#{attribute}\" for sequence/mapping#{keys} does not exist." + end + # @param [Environment] environment def self.get_attribute( environment, source, object, attribute, type, arguments: {}, defined_test: false, @@ -922,7 +933,11 @@ def self.get_attribute( return end - raise Error::Runtime, "Can't find key #{attribute} in #{object.inspect}." + raise Error::Runtime.new( + key_not_found_message(object, attribute), + lineno, + source + ) end end @@ -987,8 +1002,7 @@ def self.get_attribute( message = if object.nil? "Impossible to access an attribute (\"#{attribute}\") on a null variable." elsif object.respond_to?(:[]) && !object.is_a?(String) - keys = object.respond_to?(:keys) ? "with keys \"#{object.keys.join(', ')}\"" : '' - "Key \"#{attribute}\" for sequence/mapping #{keys} does not exist." + key_not_found_message(object, attribute) else "Impossible to access an attribute (\"#{attribute}\") on a #{object.class} " \ "variable (\"#{object}\")." diff --git a/spec/lib/twig/extension/core_spec.rb b/spec/lib/twig/extension/core_spec.rb index 1cb8304..3173a5a 100644 --- a/spec/lib/twig/extension/core_spec.rb +++ b/spec/lib/twig/extension/core_spec.rb @@ -482,6 +482,10 @@ def each describe '#get_attribute' do let(:loader) { Twig::Loader::Hash.new({}) } let(:environment) { Twig::Environment.new(loader) } + let(:strict) { Twig::Environment.new(loader, { strict_variables: true }) } + # A missing key raises with the source attached, so the double has to answer + # what Error::Base asks of it. + let(:source) { instance_double(Twig::Source, path: nil, name: nil) } it 'does not use bracket access if doing a method call' do klass = Class.new do @@ -519,24 +523,40 @@ def foo ).to eq(3.14) end - def attribute_of(object, attribute) - described_class.get_attribute( - environment, - instance_double(Twig::Source), - object, - attribute, - Twig::Template::METHOD_CALL - ) + def attribute_of(object, attribute, env = environment) + described_class.get_attribute(env, source, object, attribute, Twig::Template::METHOD_CALL) end def subscript_of(object, attribute, env = environment) - described_class.get_attribute( - env, - instance_double(Twig::Source), - object, - attribute, - Twig::Template::ARRAY_CALL - ) + described_class.get_attribute(env, source, object, attribute, Twig::Template::ARRAY_CALL) + end + + context 'when the key is missing' do + it 'names the keys that exist rather than dumping the object' do + expect { subscript_of({ 'a' => 'x', 'b' => 'y' }, 'c', strict) }. + to raise_error(Twig::Error::Runtime, %r{Key "c" for sequence/mapping with keys "a, b" does not exist}) + end + + it 'reports the same way for bracket and dot access' do + object = { 'a' => 'x' } + bracket = begin + subscript_of(object, 'c', strict) + rescue Twig::Error::Runtime => e + e.message + end + dot = begin + attribute_of(object, 'c', strict) + rescue Twig::Error::Runtime => e + e.message + end + + expect(bracket).to eq(dot) + end + + it 'does not double the space when the object has no keys' do + expect { subscript_of(%w[a b], 5, strict) }. + to raise_error(Twig::Error::Runtime, %r{sequence/mapping does not exist}) + end end context 'with a key holding a falsy value' do @@ -580,8 +600,6 @@ def subscript_of(object, attribute, env = environment) end context 'with an array index' do - let(:strict) { Twig::Environment.new(loader, { strict_variables: true }) } - it 'reads from the end with a negative index' do expect(subscript_of(%w[a b c], -1)).to eq('c') expect(subscript_of(%w[a b c], -3)).to eq('a') From 6afd2584223870df2ee3bd866861320c804a6cda Mon Sep 17 00:00:00 2001 From: Craig Blanchette Date: Fri, 14 Aug 2026 09:09:39 -0400 Subject: [PATCH 5/6] Read the version from lib/twig/version.rb The version was a literal in the gemspec, so it existed in exactly one place that nothing else could see: the library couldn't report its own version at runtime, and neither could anything depending on it. Twig::VERSION now holds it and the gemspec reads that. require_relative rather than requiring the gem, since this needs one constant and evaluating the whole library would pull ActiveSupport in at packaging time. The existing loader globs lib/twig/*.rb, so the constant is there at runtime without anything else being wired up. Co-Authored-By: Claude Opus 5 --- lib/twig/version.rb | 5 +++++ twig-ruby.gemspec | 7 ++++++- 2 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 lib/twig/version.rb diff --git a/lib/twig/version.rb b/lib/twig/version.rb new file mode 100644 index 0000000..4fe648c --- /dev/null +++ b/lib/twig/version.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +module Twig + VERSION = '0.0.8' +end diff --git a/twig-ruby.gemspec b/twig-ruby.gemspec index 3788f76..5bba5a6 100644 --- a/twig-ruby.gemspec +++ b/twig-ruby.gemspec @@ -1,8 +1,13 @@ # frozen_string_literal: true +# require_relative rather than requiring the gem: this needs the version and +# nothing else, and evaluating the whole library to read one constant would pull +# ActiveSupport in at packaging time. +require_relative 'lib/twig/version' + Gem::Specification.new do |s| s.name = 'twig_ruby' - s.version = '0.0.8' + s.version = Twig::VERSION s.summary = 'Twig Templating for Ruby' s.description = '' s.authors = ['Craig Blanchette', 'Fabian Potencier'] From 85aaa8cf01bde2e69ef6b4492d8d7d1c5706d865 Mon Sep 17 00:00:00 2001 From: Craig Blanchette Date: Fri, 14 Aug 2026 09:09:46 -0400 Subject: [PATCH 6/6] Bump version to 0.0.9 --- lib/twig/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/twig/version.rb b/lib/twig/version.rb index 4fe648c..4c7b495 100644 --- a/lib/twig/version.rb +++ b/lib/twig/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module Twig - VERSION = '0.0.8' + VERSION = '0.0.9' end