Skip to content

Commit 3635d2d

Browse files
committed
YAMLColumn: use YAML.safe_dump if available
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.
1 parent d6eec53 commit 3635d2d

File tree

4 files changed

+40
-5
lines changed

4 files changed

+40
-5
lines changed

Gemfile.lock

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ GEM
333333
activesupport (>= 7.0.0)
334334
rack
335335
railties (>= 7.0.0)
336-
psych (5.0.1)
336+
psych (5.1.0)
337337
stringio
338338
public_suffix (5.0.1)
339339
puma (6.0.2)

activerecord/CHANGELOG.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
* YAML columns use `YAML.safe_dump` is available.
2+
3+
As of `psych 5.1.0`, `YAML.safe_dump` can now apply the same permitted
4+
types restrictions than `YAML.safe_load`.
5+
6+
It's preferable to ensure the payload only use allowed types when we first
7+
try to serialize it, otherwise you may end up with invalid records in the
8+
database.
9+
10+
*Jean Boussier*
11+
112
* `ActiveRecord::QueryLogs` better handle broken encoding.
213

314
It's not uncommon when building queries with BLOB fields to contain

activerecord/lib/active_record/coders/yaml_column.rb

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,24 @@ def init_with(coder) # :nodoc:
2626
@unsafe_load = coder["unsafe_load"]
2727
end
2828

29-
def dump(obj)
30-
return if obj.nil?
29+
if Gem::Version.new(Psych::VERSION) >= "5.1"
30+
def dump(obj)
31+
return if obj.nil?
3132

32-
assert_valid_value(obj, action: "dump")
33-
YAML.dump obj
33+
assert_valid_value(obj, action: "dump")
34+
if unsafe_load?
35+
YAML.dump(obj)
36+
else
37+
YAML.safe_dump(obj, permitted_classes: permitted_classes, aliases: true)
38+
end
39+
end
40+
else
41+
def dump(obj)
42+
return if obj.nil?
43+
44+
assert_valid_value(obj, action: "dump")
45+
YAML.dump obj
46+
end
3447
end
3548

3649
def load(yaml)

activerecord/test/cases/coders/yaml_column_test.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,17 @@ def test_yaml_column_permitted_classes_are_consumed_by_safe_load
9797
end
9898
end
9999

100+
def test_yaml_column_permitted_classes_are_consumed_by_safe_dump
101+
if Gem::Version.new(Psych::VERSION) < "5.1"
102+
skip "YAML.safe_dump is either missing on unavailable on #{Psych::VERSION}"
103+
end
104+
105+
coder = YAMLColumn.new("attr_name")
106+
assert_raises(Psych::DisallowedClass) do
107+
coder.dump([Time.new])
108+
end
109+
end
110+
100111
def test_yaml_column_permitted_classes_option
101112
ActiveRecord.yaml_column_permitted_classes = [Symbol]
102113

0 commit comments

Comments
 (0)