Mash should load yaml file.

[arekt]

No description provided.

[arekt]

Reproducing problem with loading yaml file with aliases.

[michaelherold]

Thanks! I'll take a look.

[michaelherold]

Okay, I've narrowed it down to this commit: 5a6ffc7

I'm going to see what is in there that would cause this to be an issue.

[michaelherold]

TIL that YAML.safe_load doesn't load aliases by default! Thanks for the test and the checking for what broke and when - it made tracking down this regression a lot easier than it otherwise would have been.

Would you please test this out to make sure it works for the use case(s) which made you discover this regression? If so, we can get a patch release out to fix it for you in a released version.

[arekt]

Thanks a lot for quick fix. Looks like it works with our app.

[dblock]

Other than the regression, there's probably a reason YAML doesn't do aliases by default (security?). Maybe we should deprecate this behavior or expose it differently?

[michaelherold]

I'd be okay with adding the ability to toggle it on or off. The interface for Mash.load doesn't make it easy to do so. We could do something like this like:

class Mash
  def self.load(path, options = {})
    # ...
    parser_klass = options.fetch(:parser) { Hashie::Extensions::Parsers::YamlErbParser }
    parser = parser_klass.new(options.fetch(:parser_options, {}))
    @_mashes[path] = new(parser.perform(path)).freeze
  end
end

config = Mash.load("my_yaml.yml", parser_options: { aliases: true })

I'm not sure what the best approach for this would be. Prior to 3.6.0, we used YAML.load instead of YAML.safe_load so we weren't being particularly strong on the security front. I'd almost want to hedge on adding the aliases by default (for backward-compatibility) and then give people the option to turn off aliases with the above construct (or something like it) with the intent to reverse the behavior in 4.0.

An aside

One of my dreams is to deprecate Mash.load and remove it in 4.0 in favor of a hashie-persistance extension that I worked on a few years ago. I don't think I finished getting it to release quality, but would be willing to spend time on it for that purpose.

[arekt]

Sorry for silence, took holiday on Friday. From my point of view creating mash from YAML directly is not adding much value. Is nice to have it but I wouldn't spend much time to consider all possible use cases.
I don't think many people is using YAML in response from the server more as reading configuration files, so doesn't need to be safe by default and maybe it would be OK to keep old behavior.

One more way it could be to have safe_load, and load methods.

If you decide not merge this pull request and stay with non alias YAML version, I'm fine with just loading yaml manually and pass data as hash to Mash.new. (hard part was just to find what is breaking)

[attilahorvath]

Hi,

Has there been a decision on this?

We are currently loading YAML config files (with aliases) through Hashie::Mash.load and we can't update Hashie to the latest version because of this issue.

Should we switch to loading the YAML manually first to keep up-to-date?

[dblock]

Should we merge this @michaelherold ?

[michaelherold]

Yes, I think we will merge this. Sorry that we missed your follow-up, @arekt!

[arekt]

@michaelherold no worries, thanks for merge :)