Skip to content

Ensure default_sort_keys_proc isn't reassigned. - #1080

Merged
byroot merged 1 commit into
ruby:masterfrom
byroot:reject-default-sorting-proc-change
Sep 28, 2026
Merged

byroot merged 1 commit into
ruby:masterfrom
byroot:reject-default-sorting-proc-change

Conversation

@byroot

@byroot byroot commented Sep 26, 2026

Copy link
Copy Markdown
Member

It's assignable from Ruby simply for the gem's own initialization, but it should never be re-assigned later.

Fix: #1079

@eightbitraptor does that solve your problem?

@eightbitraptor eightbitraptor left a comment

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.

Thanks @byroot - This looks like a much simpler approach.

Comment thread ext/json/ext/generator/generator.c Outdated
rb_raise(rb_eTypeError, "sort_key_proc must be a Proc");
}
if (default_sort_keys_proc) {
rb_raise(rb_eArgError, "sort_key_proc can't only be set once");

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.

Do you mean "can only be set once"?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes 🤦

It's assignable from Ruby simply for the gem's own initialization,
but it should never be re-assigned later.

Fix: ruby#1079
@byroot
byroot force-pushed the reject-default-sorting-proc-change branch from 5804089 to 2ecb368 Compare September 28, 2026 13:26
@byroot
byroot merged commit cc32ef5 into ruby:master Sep 28, 2026
42 checks passed
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