Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Psych.safe_load by default #487

Merged
merged 3 commits into from
May 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 53 additions & 60 deletions lib/psych.rb
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,6 @@
module Psych
# The version of libyaml Psych is using
LIBYAML_VERSION = Psych.libyaml_version.join('.').freeze
# Deprecation guard
NOT_GIVEN = Object.new.freeze
private_constant :NOT_GIVEN

###
# Load +yaml+ in to a Ruby data structure. If multiple documents are
Expand All @@ -249,11 +246,11 @@ module Psych
#
# Example:
#
# Psych.load("--- a") # => 'a'
# Psych.load("---\n - a\n - b") # => ['a', 'b']
# Psych.unsafe_load("--- a") # => 'a'
# Psych.unsafe_load("---\n - a\n - b") # => ['a', 'b']
#
# begin
# Psych.load("--- `", filename: "file.txt")
# Psych.unsafe_load("--- `", filename: "file.txt")
# rescue Psych::SyntaxError => ex
# ex.file # => 'file.txt'
# ex.message # => "(file.txt): found character that cannot start any token"
Expand All @@ -262,21 +259,16 @@ module Psych
# When the optional +symbolize_names+ keyword argument is set to a
# true value, returns symbols for keys in Hash objects (default: strings).
#
# Psych.load("---\n foo: bar") # => {"foo"=>"bar"}
# Psych.load("---\n foo: bar", symbolize_names: true) # => {:foo=>"bar"}
# Psych.unsafe_load("---\n foo: bar") # => {"foo"=>"bar"}
# Psych.unsafe_load("---\n foo: bar", symbolize_names: true) # => {:foo=>"bar"}
#
# Raises a TypeError when `yaml` parameter is NilClass
#
# NOTE: This method *should not* be used to parse untrusted documents, such as
# YAML documents that are supplied via user input. Instead, please use the
# safe_load method.
# load method or the safe_load method.
#
def self.unsafe_load yaml, legacy_filename = NOT_GIVEN, filename: nil, fallback: false, symbolize_names: false, freeze: false
if legacy_filename != NOT_GIVEN
warn_with_uplevel 'Passing filename with the 2nd argument of Psych.load is deprecated. Use keyword argument like Psych.load(yaml, filename: ...) instead.', uplevel: 1 if $VERBOSE
filename = legacy_filename
end

def self.unsafe_load yaml, filename: nil, fallback: false, symbolize_names: false, freeze: false
result = parse(yaml, filename: filename)
return fallback unless result
result.to_ruby(symbolize_names: symbolize_names, freeze: freeze)
Expand Down Expand Up @@ -327,27 +319,7 @@ class << self; alias :load :unsafe_load; end
# Psych.safe_load("---\n foo: bar") # => {"foo"=>"bar"}
# Psych.safe_load("---\n foo: bar", symbolize_names: true) # => {:foo=>"bar"}
#
def self.safe_load yaml, legacy_permitted_classes = NOT_GIVEN, legacy_permitted_symbols = NOT_GIVEN, legacy_aliases = NOT_GIVEN, legacy_filename = NOT_GIVEN, permitted_classes: [], permitted_symbols: [], aliases: false, filename: nil, fallback: nil, symbolize_names: false, freeze: false
if legacy_permitted_classes != NOT_GIVEN
warn_with_uplevel 'Passing permitted_classes with the 2nd argument of Psych.safe_load is deprecated. Use keyword argument like Psych.safe_load(yaml, permitted_classes: ...) instead.', uplevel: 1 if $VERBOSE
permitted_classes = legacy_permitted_classes
end

if legacy_permitted_symbols != NOT_GIVEN
warn_with_uplevel 'Passing permitted_symbols with the 3rd argument of Psych.safe_load is deprecated. Use keyword argument like Psych.safe_load(yaml, permitted_symbols: ...) instead.', uplevel: 1 if $VERBOSE
permitted_symbols = legacy_permitted_symbols
end

if legacy_aliases != NOT_GIVEN
warn_with_uplevel 'Passing aliases with the 4th argument of Psych.safe_load is deprecated. Use keyword argument like Psych.safe_load(yaml, aliases: ...) instead.', uplevel: 1 if $VERBOSE
aliases = legacy_aliases
end

if legacy_filename != NOT_GIVEN
warn_with_uplevel 'Passing filename with the 5th argument of Psych.safe_load is deprecated. Use keyword argument like Psych.safe_load(yaml, filename: ...) instead.', uplevel: 1 if $VERBOSE
filename = legacy_filename
end

def self.safe_load yaml, permitted_classes: [], permitted_symbols: [], aliases: false, filename: nil, fallback: nil, symbolize_names: false, freeze: false
result = parse(yaml, filename: filename)
return fallback unless result

Expand All @@ -363,6 +335,46 @@ def self.safe_load yaml, legacy_permitted_classes = NOT_GIVEN, legacy_permitted_
result
end

###
# Load +yaml+ in to a Ruby data structure. If multiple documents are
# provided, the object contained in the first document will be returned.
# +filename+ will be used in the exception message if any exception
# is raised while parsing. If +yaml+ is empty, it returns
# the specified +fallback+ return value, which defaults to +false+.
#
# Raises a Psych::SyntaxError when a YAML syntax error is detected.
#
# Example:
#
# Psych.load("--- a") # => 'a'
# Psych.load("---\n - a\n - b") # => ['a', 'b']
#
# begin
# Psych.load("--- `", filename: "file.txt")
# rescue Psych::SyntaxError => ex
# ex.file # => 'file.txt'
# ex.message # => "(file.txt): found character that cannot start any token"
# end
#
# When the optional +symbolize_names+ keyword argument is set to a
# true value, returns symbols for keys in Hash objects (default: strings).
#
# Psych.load("---\n foo: bar") # => {"foo"=>"bar"}
# Psych.load("---\n foo: bar", symbolize_names: true) # => {:foo=>"bar"}
#
# Raises a TypeError when `yaml` parameter is NilClass. This method is
# similar to `safe_load` except that `Symbol` objects are allowed by default.
#
def self.load yaml, permitted_classes: [Symbol], permitted_symbols: [], aliases: false, filename: nil, fallback: nil, symbolize_names: false, freeze: false
tenderlove marked this conversation as resolved.
Show resolved Hide resolved
safe_load yaml, permitted_classes: permitted_classes,
permitted_symbols: permitted_symbols,
aliases: aliases,
filename: filename,
fallback: fallback,
symbolize_names: symbolize_names,
freeze: freeze
end

###
# Parse a YAML string in +yaml+. Returns the Psych::Nodes::Document.
# +filename+ is used in the exception message if a Psych::SyntaxError is
Expand All @@ -382,22 +394,12 @@ def self.safe_load yaml, legacy_permitted_classes = NOT_GIVEN, legacy_permitted_
# end
#
# See Psych::Nodes for more information about YAML AST.
def self.parse yaml, legacy_filename = NOT_GIVEN, filename: nil, fallback: NOT_GIVEN
if legacy_filename != NOT_GIVEN
warn_with_uplevel 'Passing filename with the 2nd argument of Psych.parse is deprecated. Use keyword argument like Psych.parse(yaml, filename: ...) instead.', uplevel: 1 if $VERBOSE
filename = legacy_filename
end

def self.parse yaml, filename: nil
parse_stream(yaml, filename: filename) do |node|
return node
end

if fallback != NOT_GIVEN
warn_with_uplevel 'Passing the `fallback` keyword argument of Psych.parse is deprecated.', uplevel: 1 if $VERBOSE
fallback
else
false
end
false
end

###
Expand Down Expand Up @@ -446,12 +448,7 @@ def self.parser
# Raises a TypeError when NilClass is passed.
#
# See Psych::Nodes for more information about YAML AST.
def self.parse_stream yaml, legacy_filename = NOT_GIVEN, filename: nil, &block
if legacy_filename != NOT_GIVEN
warn_with_uplevel 'Passing filename with the 2nd argument of Psych.parse_stream is deprecated. Use keyword argument like Psych.parse_stream(yaml, filename: ...) instead.', uplevel: 1 if $VERBOSE
filename = legacy_filename
end

def self.parse_stream yaml, filename: nil, &block
if block_given?
parser = Psych::Parser.new(Handlers::DocumentStream.new(&block))
parser.parse yaml, filename
Expand Down Expand Up @@ -552,12 +549,7 @@ def self.to_json object
# end
# list # => ['foo', 'bar']
#
def self.load_stream yaml, legacy_filename = NOT_GIVEN, filename: nil, fallback: [], **kwargs
if legacy_filename != NOT_GIVEN
warn_with_uplevel 'Passing filename with the 2nd argument of Psych.load_stream is deprecated. Use keyword argument like Psych.load_stream(yaml, filename: ...) instead.', uplevel: 1 if $VERBOSE
filename = legacy_filename
end

def self.load_stream yaml, filename: nil, fallback: [], **kwargs
result = if block_given?
parse_stream(yaml, filename: filename) do |node|
yield node.to_ruby(**kwargs)
Expand Down Expand Up @@ -595,6 +587,7 @@ def self.safe_load_file filename, **kwargs
self.safe_load f, filename: filename, **kwargs
}
end
class << self; alias load_file safe_load_file end

# :stopdoc:
def self.add_domain_type domain, type_tag, &block
Expand Down
2 changes: 1 addition & 1 deletion lib/psych/versions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module Psych
# The version of Psych you are using
VERSION = '3.3.2'
VERSION = '4.0.0'

if RUBY_ENGINE == 'jruby'
DEFAULT_SNAKEYAML_VERSION = '1.28'.freeze
Expand Down
18 changes: 0 additions & 18 deletions test/psych/test_exception.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,6 @@ def test_load_takes_file
Psych.load '--- `', filename: 'meow'
end
assert_equal 'meow', ex.file

# deprecated interface
ex = assert_raise(Psych::SyntaxError) do
Psych.unsafe_load '--- `', 'deprecated'
end
assert_equal 'deprecated', ex.file
end

def test_psych_parse_stream_takes_file
Expand Down Expand Up @@ -86,12 +80,6 @@ def test_load_stream_takes_file
Psych.load_stream '--- `', filename: 'omg!'
end
assert_equal 'omg!', ex.file

# deprecated interface
ex = assert_raise(Psych::SyntaxError) do
Psych.load_stream '--- `', 'deprecated'
end
assert_equal 'deprecated', ex.file
end

def test_parse_file_exception
Expand Down Expand Up @@ -141,12 +129,6 @@ def test_psych_parse_takes_file
Psych.parse '--- `', filename: 'omg!'
end
assert_match 'omg!', ex.message

# deprecated interface
ex = assert_raise(Psych::SyntaxError) do
Psych.parse '--- `', 'deprecated'
end
assert_match 'deprecated', ex.message
end

def test_attributes
Expand Down
8 changes: 2 additions & 6 deletions test/psych/test_psych.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,6 @@ def test_parse_raises_on_bad_input
assert_raise(Psych::SyntaxError) { Psych.parse("--- `") }
end

def test_parse_with_fallback
assert_equal 42, Psych.parse("", fallback: 42)
end

def test_non_existing_class_on_deserialize
e = assert_raise(ArgumentError) do
Psych.unsafe_load("--- !ruby/object:NonExistent\nfoo: 1")
Expand Down Expand Up @@ -239,11 +235,11 @@ def test_load_with_fallback_hash
end

def test_load_with_fallback_for_nil
assert_nil Psych.unsafe_load("--- null", "file", fallback: 42)
assert_nil Psych.unsafe_load("--- null", filename: "file", fallback: 42)
end

def test_load_with_fallback_for_false
assert_equal false, Psych.unsafe_load("--- false", "file", fallback: 42)
assert_equal false, Psych.unsafe_load("--- false", filename: "file", fallback: 42)
end

def test_load_file
Expand Down
44 changes: 0 additions & 44 deletions test/psych/test_safe_load.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ def test_explicit_recursion
x = []
x << x
assert_equal(x, Psych.safe_load(Psych.dump(x), permitted_classes: [], permitted_symbols: [], aliases: true))
# deprecated interface
assert_equal(x, Psych.safe_load(Psych.dump(x), [], [], true))
end

def test_permitted_symbol
Expand All @@ -48,9 +46,6 @@ def test_permitted_symbol
permitted_symbols: [:foo]
)
)

# deprecated interface
assert_equal(:foo, Psych.safe_load(yml, [Symbol], [:foo]))
end

def test_symbol
Expand All @@ -61,36 +56,20 @@ def test_symbol
Psych.safe_load '--- !ruby/symbol foo', permitted_classes: []
end

# deprecated interface
assert_raise(Psych::DisallowedClass) do
Psych.safe_load '--- !ruby/symbol foo', []
end

assert_safe_cycle :foo, permitted_classes: [Symbol]
assert_safe_cycle :foo, permitted_classes: %w{ Symbol }
assert_equal :foo, Psych.safe_load('--- !ruby/symbol foo', permitted_classes: [Symbol])

# deprecated interface
assert_equal :foo, Psych.safe_load('--- !ruby/symbol foo', [Symbol])
end

def test_foo
assert_raise(Psych::DisallowedClass) do
Psych.safe_load '--- !ruby/object:Foo {}', permitted_classes: [Foo]
end

# deprecated interface
assert_raise(Psych::DisallowedClass) do
Psych.safe_load '--- !ruby/object:Foo {}', [Foo]
end

assert_raise(Psych::DisallowedClass) do
assert_safe_cycle Foo.new
end
assert_kind_of(Foo, Psych.safe_load(Psych.dump(Foo.new), permitted_classes: [Foo]))

# deprecated interface
assert_kind_of(Foo, Psych.safe_load(Psych.dump(Foo.new), [Foo]))
end

X = Struct.new(:x)
Expand Down Expand Up @@ -122,27 +101,6 @@ def test_anon_struct
end
end

def test_deprecated_anon_struct
assert Psych.safe_load(<<-eoyml, [Struct, Symbol])
--- !ruby/struct
foo: bar
eoyml

assert_raise(Psych::DisallowedClass) do
Psych.safe_load(<<-eoyml, [Struct])
--- !ruby/struct
foo: bar
eoyml
end

assert_raise(Psych::DisallowedClass) do
Psych.safe_load(<<-eoyml, [Symbol])
--- !ruby/struct
foo: bar
eoyml
end
end

def test_safe_load_default_fallback
assert_nil Psych.safe_load("")
end
Expand All @@ -159,8 +117,6 @@ def test_safe_load_raises_on_bad_input

def cycle object, permitted_classes: []
Psych.safe_load(Psych.dump(object), permitted_classes: permitted_classes)
# deprecated interface test
Psych.safe_load(Psych.dump(object), permitted_classes)
end

def assert_safe_cycle object, permitted_classes: []
Expand Down