From 117310bdc00236c0a7676616ce25b5106775dabc Mon Sep 17 00:00:00 2001
From: Jeremy Evans <code@jeremyevans.net>
Date: Tue, 8 Jun 2021 10:19:08 -0700
Subject: [PATCH] Make ENV.clone warn and ENV.dup raise

ENV.dup returned a plain Object, since all of ENV's behavior is
defined in ENV's singleton class.  So using dup makes no sense.

ENV.clone works and is used in some gems, but it doesn't do what
the user expects, since modifying ENV.clone also modifies ENV.
Add a deprecation warning pointing the user to use ENV.to_h
instead.

This also undefines some private initialize* methods in ENV,
since they are not needed.

Fixes [Bug #17767]
---
 hash.c                | 63 +++++++++++++++++++++++++++++++++++++++++++
 test/ruby/test_env.rb | 40 +++++++++++++++++++++++++++
 2 files changed, 103 insertions(+)

diff --git a/hash.c b/hash.c
index 80cf54f7e4..88e0771f51 100644
--- a/hash.c
+++ b/hash.c
@@ -6487,6 +6487,61 @@ env_update(VALUE env, VALUE hash)
     return env;
 }
 
+/*
+ * call-seq:
+ *   ENV.clone(freeze: nil) -> copy of ENV
+ *
+ * Returns a clone of ENV, but warns because the ENV data is shared with the
+ * clone.
+ * If +freeze+ keyword is given and not +nil+ or +false+, raises ArgumentError.
+ * If +freeze+ keyword is given and +true+, raises TypeError, as ENV storage
+ * cannot be frozen.
+ */
+static VALUE
+env_clone(int argc, VALUE *argv, VALUE obj)
+{
+    if (argc) {
+        static ID keyword_ids[1];
+        VALUE opt, kwfreeze;
+
+        if (!keyword_ids[0]) {
+            CONST_ID(keyword_ids[0], "freeze");
+        }
+        rb_scan_args(argc, argv, "0:", &opt);
+        if (!NIL_P(opt)) {
+            rb_get_kwargs(opt, keyword_ids, 0, 1, &kwfreeze);
+            switch(kwfreeze) {
+              case Qtrue:
+                rb_raise(rb_eTypeError, "cannot freeze ENV");
+                break;
+              default:
+                rb_raise(rb_eArgError, "invalid value for freeze keyword");
+                break;
+              case Qnil:
+              case Qfalse:
+                break;
+            }
+        }
+    }
+
+    rb_warn_deprecated("ENV.clone", "ENV.to_h");
+    return envtbl;
+}
+
+NORETURN(static VALUE env_dup(VALUE));
+/*
+ * call-seq:
+ *   ENV.dup # raises TypeError
+ *
+ * Raises TypeError, because ENV is a singleton object.
+ * Use #to_h to get a copy of ENV data as a hash.
+ */
+static VALUE
+env_dup(VALUE obj)
+{
+    rb_raise(rb_eTypeError, "Cannot dup ENV, use ENV.to_h to get a copy of ENV as a hash");
+}
+
 /*
  *  A \Hash maps each of its unique keys to a specific value.
  *
@@ -7193,6 +7248,14 @@ Init_Hash(void)
     rb_define_singleton_method(envtbl, "to_h", env_to_h, 0);
     rb_define_singleton_method(envtbl, "assoc", env_assoc, 1);
     rb_define_singleton_method(envtbl, "rassoc", env_rassoc, 1);
+    rb_define_singleton_method(envtbl, "clone", env_clone, -1);
+    rb_define_singleton_method(envtbl, "dup", env_dup, 0);
+
+    VALUE envtbl_class = rb_singleton_class(envtbl);
+    rb_undef_method(envtbl_class, "initialize");
+    rb_undef_method(envtbl_class, "initialize_clone");
+    rb_undef_method(envtbl_class, "initialize_copy");
+    rb_undef_method(envtbl_class, "initialize_dup");
 
     /*
      * ENV is a Hash-like accessor for environment variables.
diff --git a/test/ruby/test_env.rb b/test/ruby/test_env.rb
index 6779c94dde..583b432e3d 100644
--- a/test/ruby/test_env.rb
+++ b/test/ruby/test_env.rb
@@ -62,6 +62,46 @@ class TestEnv < Test::Unit::TestCase
     }
   end
 
+  def test_dup
+    assert_raise(TypeError) {
+      ENV.dup
+    }
+  end
+
+  def test_clone
+    warning = /ENV\.clone is deprecated; use ENV\.to_h instead/
+    clone = assert_deprecated_warning(warning) {
+      ENV.clone
+    }
+    assert_same(ENV, clone)
+
+    clone = assert_deprecated_warning(warning) {
+      ENV.clone(freeze: false)
+    }
+    assert_same(ENV, clone)
+
+    clone = assert_deprecated_warning(warning) {
+      ENV.clone(freeze: nil)
+    }
+    assert_same(ENV, clone)
+
+    assert_raise(TypeError) {
+      ENV.clone(freeze: true)
+    }
+    assert_raise(ArgumentError) {
+      ENV.clone(freeze: 1)
+    }
+    assert_raise(ArgumentError) {
+      ENV.clone(foo: false)
+    }
+    assert_raise(ArgumentError) {
+      ENV.clone(1)
+    }
+    assert_raise(ArgumentError) {
+      ENV.clone(1, foo: false)
+    }
+  end
+
   def test_has_value
     val = 'a'
     val.succ! while ENV.has_value?(val) || ENV.has_value?(val.upcase)