Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ClassModule#superclass= accepts a ClassModule as an argument #1222

Merged

Conversation

flavorjones
Copy link
Collaborator

@flavorjones flavorjones commented Dec 2, 2024

It is necessary for ClassModule's instance variable @superclass to always be a String (or nil) so that the class can be saved with #marshal_dump and loaded with #marshal_load.

However, there's no type checking being done, which allows a bug like the one reported in #1221 (which was introduced in #1217) that sets superclass to a ClassModule. That bug requires:

  • setting a superclass to a NormalClass
  • marshal_save
  • marshal_load (which raises an exception)

With this change, passing a ClassModule to ClassModule#superclass= is explicitly allowed by saving the full name of the ClassModule in the @superclass instance variable.

Fixes #1221

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
It is necessary for ClassModule's instance variable @superclass to
always be a String (or nil) so that the class can be saved with
`#marshal_dump` and loaded with `#marshal_load`.

However, there's no type checking being done, which allows a bug like
the one reported in ruby#1221 (which was introduced in ruby#1217) that sets
superclass to a ClassModule. That bug requires:

- setting a superclass to a NormalClass
- marshal_save
- marshal_load (which raises an exception)

With this change, passing a ClassModule to ClassModule#superclass= is
explicitly allowed by saving the full name of the ClassModule in the
@superclass instance variable.
Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

Thanks for figuring out the issue. Does that mean find_c_enclosure can return those 3 types too? Should be document it?

@flavorjones
Copy link
Collaborator Author

Does that mean find_c_enclosure can return those 3 types too

My reading of that method is that it will return either nil or a ClassModule instance, and will not return a String/name.

@st0012 st0012 merged commit 9ced6d5 into ruby:master Dec 2, 2024
26 checks passed
matzbot pushed a commit to ruby/ruby that referenced this pull request Dec 2, 2024

Verified

This commit was signed with the committer’s verified signature.
hbomb79 Harry Felton
argument
(ruby/rdoc#1222)

It is necessary for ClassModule's instance variable @superclass to
always be a String (or nil) so that the class can be saved with
`#marshal_dump` and loaded with `#marshal_load`.

However, there's no type checking being done, which allows a bug like
the one reported in #1221 (which was introduced in #1217) that sets
superclass to a ClassModule. That bug requires:

- setting a superclass to a NormalClass
- marshal_save
- marshal_load (which raises an exception)

With this change, passing a ClassModule to ClassModule#superclass= is
explicitly allowed by saving the full name of the ClassModule in the
@superclass instance variable.

ruby/rdoc@9ced6d534c
@flavorjones flavorjones deleted the flavorjones-fix-superclass-marshalling branch December 3, 2024 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

#1217 is broken with make install in ruby/ruby
2 participants