Skip to content

Avoid calling >= on unknown receivers in ls command#1193

Open
dejmedus wants to merge 1 commit intoruby:masterfrom
Shopify:call-methods-on-owned-obj
Open

Avoid calling >= on unknown receivers in ls command#1193
dejmedus wants to merge 1 commit intoruby:masterfrom
Shopify:call-methods-on-owned-obj

Conversation

@dejmedus
Copy link
Copy Markdown

@dejmedus dejmedus commented Mar 27, 2026

If Ls#dump_methods is called with an obj whose ancestors alter >=, the comparison (c >= Object/c >= Class) may raise an error. For example:

class Foo; class << self; undef >=; end; end
foo = Foo.new
ls foo

Foo >= Object # NoMethodError     

To prevent this, we could defensively flip the comparison so that <= is called on known receivers

Previously, when calling comparison methods on
receivers we don't control (`c >= Object`), there
was a chance it could lack or have overridden the
method. This commit flips them to prevent this
`Object <= c`
@dejmedus dejmedus marked this pull request as ready for review March 27, 2026 23:19
ancestors = ancestors.reject { |c| c >= Object } if klass < Object
singleton_ancestors = (singleton_class&.ancestors || []).reject { |c| c >= Class }
ancestors = ancestors.reject { |c| Object <= c } if klass < Object
singleton_ancestors = (singleton_class&.ancestors || []).reject { |c| Class <= c }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To solve the problem, the change looks good. However, ls foo ls Foo doesn't work when some other methods are missing:

<
constants
class_variables
to_s
ancestors
public_instance_methods

Unlike Object instances that we need to consider BasicObject, I think we can assume Classes doesn't undef/change most of these methods: constants, class_variables, to_s, ancestors, public_instance_methods.
But for <, I guess it might be removed and need to change if klass < Object to if Object > klass in the situation you are assuming.

Can you share what kind of situation :<= is removed? Will :> be removed too?
Do you know an existing gem that creates a class without :<=?
The correct fix depends on the situation what you are assuming, and needs to be commented in test code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants