Implement YAML.safe_dump to make safe_load more usable. #495
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In case where Psych is used as a two way serializers, e.g. to serialize some cache or config, it is preferable to have the same restrictions on both load and dump. Otherwise you might dump and persist some objects payloads that you later won't be able to read.
Actually that was super easy, so I added it as well. |
casperisfine
pushed a commit
to Shopify/psych
that referenced
this pull request
Jan 22, 2023
…bols Ref: ruby#495 That's how it works for `safe_load`: ```ruby >> YAML.safe_load(':foo', permitted_classes: [Symbol]) => :foo ``` So `safe_dump` should mirror that.
matzbot
pushed a commit
to ruby/ruby
that referenced
this pull request
Jan 23, 2023
…allow all symbols Ref: ruby/psych#495 That's how it works for `safe_load`: ```ruby >> YAML.safe_load(':foo', permitted_classes: [Symbol]) => :foo ``` So `safe_dump` should mirror that. ruby/psych@592a75a656
casperisfine
pushed a commit
to Shopify/rails
that referenced
this pull request
Feb 13, 2023
One particularly annoying thing with YAMLColumn type restriction is that it is only checked on load. Which means if your code insert data with unsupported types, the insert will work, but now you'll be unable to read the record, which makes it hard to fix etc. That's the reason why I implemented `YAML.safe_dump` (ruby/psych#495). It applies exactly the same restrictions than `safe_load`, which means if you attempt to store non-permitted fields, it will fail on insertion and not on further reads, so you won't create an invalid record in your database.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ref: #487
In case where Psych is used as a two way serializers, e.g. to serialize some cache or config, it is preferable to have the same restrictions on both load and dump.
Otherwise you might dump and persist some objects payloads that you later won't be able to read.
cc @pixeltrix as we discussed this earlier today. cc @tenderlove because #487
I also think that this hints at having some kind of "parser" instance that could hold the configuration, e.g.:
This could even allow to expose
YAML::Safe,YAML::Unsafeas more usable shorthands.NB: The dump might need to restrict aliases as well, but I need to have a deeper look to see how that could be possible.