Short Adventures In Old Code
I have the privilege of updating (primarily) Rails applications from old, deprecated versions to new, shinier, supported versions. Maintenance like this falls lower down in priority as an application grows larger, gets more features, and needs to preserve stability, causing dependency updates and upgrades to stay in the "nice-to-have" or "when we have time" categories of work.
Today I was working on updating an application from Rails 5.1 to 5.2.8.1 (the latest of 5.2.x at the time of writing), and it was going fairly smoothly. Maintenance is infinitely easier when there is even decent test coverage and this application has great coverage. I saw a familiar failure show up nearly a hundred times when running the tests: Psych::DisallowedError.
The psych gem is notorious for it's 4.0 upgrade that came with major breaking changes where it defaulted to using safe_load when loading YAML files. This requires the user to explicitly allow classes to be loaded beyond the strict defaults. It's simplest to see from an example:
class CoolBlog; end
test = CoolBlog.new
yaml_dump = YAML.dump(test)
YAML.safe_load(yaml_dump)
#=> Tried to load unspecified class: CoolBlog (Psych::DisallowedClass)
YAML.safe_load(test_dumped, permitted_classes: [CoolBlog])
#=> #<CoolBlog:0x000000011e4b9910>But this application didn't even have psych in the Gemfile, and the Ruby version was still at the ancient comfortably old 2.3.4. So psych wasn't even close to 4.0. It was still built-in to ruby at v2.1.0. So, how is there an error here?
What was happening was what we in the biz call an Undocumented Incompatibility.
The source of the error is [here](https://github.com/rails/rails/blob/v5.2.8.1/activerecord/lib/active_record/coders/yaml_column.rb#L49):
def yaml_load(payload)
if !ActiveRecord::Base.use_yaml_unsafe_load
YAML.safe_load(payload, permitted_classes: ActiveRecord::Base.yaml_column_permitted_classes, aliases: true)
else
# ...
end
endDo you see it?
If so, wow. [Here's what you need](https://github.com/ruby/psych/blob/v2.1.0/lib/psych.rb#L292), psych 2.1.0 's safe_load method:
def self.safe_load yaml, whitelist_classes = [], whitelist_symbols = [], aliases = false, filename = nil
# ... You actually only need the method signature
endThis is an Undocumented Incompatibility because Rails 5.2.8.1 expects to be able to use keyword arguments for permitted_classes and aliases when they are not available on such an old version of psych
Now, an issue. Wouldn't calling safe_load with those kwargs raise ArgumentError: wrong number of arguments ? Yes, it should.. right?
Luckily I have access to irb with Ruby 2.3.4:
irb(main):01:0> class Test
irb(main):02:1> def self.safe_load yaml, whitelist_classes = [], whitelist_symbols = [], aliases = false, filename = nil
irb(main):03:2> puts "oops"
irb(main):04:2> end
irb(main):05:1> end
=> :safe_load
irb(main):06:0> Test.safe_load("my yaml payload", permitted_classes: [], aliases: true)
oops
=> nil
(I added indentation to the body of the class after-the-fact by the way, old irb didn't do that for you, check out new irb in ruby 4.0 it's even got code-complete suggestions)
Unfortunately I have not actually had the time to figure out why this doesn't raise an error, but it does explain why Rails's configurationActiveRecord::Base.yaml_column_permitted_classes doesn't work unless you update psych
The fix for this in an application is actually so simple that a lot of people would probably try it first before digging in for the whole picture: bundle update psych