Skip to content

Commit

Permalink
Merge pull request #487 from ruby/default-unsafe
Browse files Browse the repository at this point in the history 
Use Psych.safe_load by default
  • Loading branch information
@tenderlove
tenderlove authored May 13, 2021
2 parents 4de7e9c + 0767227 commit 216f94f
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 129 deletions.
113 changes: 53 additions & 60 deletions lib/psych.rb
View file
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
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
View file
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
View file
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
View file
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
View file
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

0 comments on commit 216f94f

Please sign in to comment.