Merge pull request #487 from ruby/default-unsafe

Use Psych.safe_load by default

5 files changed, 56 insertions, 129 deletions

diff --git a/lib/psych.rb b/lib/psych.rb
index 34d2218..7c6b8d0 100644
--- a/lib/psych.rb
+++ b/lib/psych.rb
@@ -234,9 +234,6 @@ require 'psych/class_loader'
 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
@@ -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"
@@ -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)
@@ -327,27 +319,7 @@ module Psych
   #   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
 
@@ -364,6 +336,46 @@ module Psych
   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
   # raised.
@@ -382,22 +394,12 @@ module Psych
   #   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
 
   ###
@@ -446,12 +448,7 @@ module Psych
   # 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
@@ -552,12 +549,7 @@ module Psych
   #   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)
@@ -595,6 +587,7 @@ module Psych
       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
diff --git a/lib/psych/versions.rb b/lib/psych/versions.rb
index acb2133..f7cfc96 100644
--- a/lib/psych/versions.rb
+++ b/lib/psych/versions.rb
@@ -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
diff --git a/test/psych/test_exception.rb b/test/psych/test_exception.rb
index d2ae76a..c1e69ab 100644
--- a/test/psych/test_exception.rb
+++ b/test/psych/test_exception.rb
@@ -53,12 +53,6 @@ module Psych
         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
@@ -86,12 +80,6 @@ module Psych
         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
@@ -141,12 +129,6 @@ module Psych
         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
diff --git a/test/psych/test_psych.rb b/test/psych/test_psych.rb
index 912bcb9..256ed91 100644
--- a/test/psych/test_psych.rb
+++ b/test/psych/test_psych.rb
@@ -78,10 +78,6 @@ class TestPsych < Psych::TestCase
     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")
@@ -239,11 +235,11 @@ class TestPsych < Psych::TestCase
   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
diff --git a/test/psych/test_safe_load.rb b/test/psych/test_safe_load.rb
index d13ce7c..b52d604 100644
--- a/test/psych/test_safe_load.rb
+++ b/test/psych/test_safe_load.rb
@@ -31,8 +31,6 @@ module Psych
       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
@@ -48,9 +46,6 @@ module Psych
           permitted_symbols: [:foo]
         )
       )
-
-      # deprecated interface
-      assert_equal(:foo, Psych.safe_load(yml, [Symbol], [:foo]))
     end
 
     def test_symbol
@@ -61,17 +56,9 @@ module Psych
         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
@@ -79,18 +66,10 @@ module Psych
         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)
@@ -122,27 +101,6 @@ module Psych
       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
@@ -159,8 +117,6 @@ module Psych
 
     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: []