Fix loading database.yml file with aliases
#950
Conversation
| @@ -1,5 +1,7 @@ | |||
| # frozen_string_literal: true | |||
|
|
|||
| require 'yaml' | |||
There was a problem hiding this comment.
We need it for YAML.safe_load? Is it required somewhere else?
| @@ -207,6 +206,14 @@ def database_yaml | |||
| nil | |||
| end | |||
|
|
|||
| def load_yaml(src) | |||
| if Psych::VERSION > '4.0' | |||
There was a problem hiding this comment.
Based on #516 link, So I'm not sure Psych::VERSION > 4.0 is better than respond_to?(:unsafe_load_file).
| @@ -207,6 +206,14 @@ def database_yaml | |||
| nil | |||
| end | |||
|
|
|||
| def load_yaml(src) | |||
| if Psych::VERSION > '4.0' | |||
| YAML.safe_load(src, aliases: true) | |||
There was a problem hiding this comment.
Can you use YAML.safe_load_file('config/database.yml', aliases: true) instead of YAML.safe_load(src, aliases: true)?
There was a problem hiding this comment.
I checked the sources for psych, and seems like :aliases are supported only by safe_load, not safe_load_file (which is a bit surprising to me).
There was a problem hiding this comment.
Hm, it seems that keyword args are delegated. So :aliases will also be delegated from safe_load_file to safe_load.
https://github.com/ruby/psych/blob/v4.0.0/lib/psych.rb#L580-L589
There was a problem hiding this comment.
Thanks! Made an update.
fatkodima
force-pushed
the
fix-loading-database-yml-with-aliases
branch
from
df1fd92 to
ca1bc98
Compare
| @@ -192,11 +192,12 @@ def database_from_yaml | |||
| def database_yaml | |||
| return nil unless File.exist?('config/database.yml') | |||
|
|
|||
| yaml = if YAML.respond_to?(:unsafe_load_file) | |||
| YAML.unsafe_load_file('config/database.yml') | |||
There was a problem hiding this comment.
Hm, I wondered if the issue would be fixed with this change. Because YAML.unsafe_load_file can accept aliases:
$ cat example.yaml
default: &default
item: 42
foo:
<<: *default$ ruby -v
ruby 3.3.0dev (2023-03-17T00:50:41Z master f29c9d6d36) [x86_64-darwin19]
$ ruby -ryaml -e "p YAML.unsafe_load_file('./example.yaml')"
{"default"=>{"item"=>42}, "foo"=>{"item"=>42}}$ ruby -ryaml -e "p YAML.load_file('./example.yaml')"
/Users/koic/.rbenv/versions/3.3.0-dev/lib/ruby/3.3.0+0/psych/visitors/to_ruby.rb:432:in `visit_Psych_Nodes_Alias': Alias parsing was not enabled. To enable it, pass `aliases: true` to `Psych::load` or `Psych::safe_load`. (Psych::AliasesNotEnabled)
from /Users/koic/.rbenv/versions/3.3.0-dev/lib/ruby/3.3.0+0/psych/visitors/visitor.rb:30:in `visit'I know that YAML.safe_load_file with aliases: true option can accept aliases as well, but YAML.unsafe_load_file should suffice:
$ ruby -ryaml -e "p YAML.safe_load_file('./example.yaml', aliases: true)"
{"default"=>{"item"=>42}, "foo"=>{"item"=>42}}There was a problem hiding this comment.
I do not remember how I reproduced it before, but reverted this PR changes and it works.
I think, the user just forgot default: &default section in the database.yml, because in this case it raises the exact same error as in the issue.
Fixes #947.
From that issue, it is clear that the error is related to parsing yaml with aliases.
Based on the solution from sidekiq/sidekiq#5141.