From 7f1fe5f091db3b05c3970e7b7a7c602922729642 Mon Sep 17 00:00:00 2001
From: Jeremy Evans <code@jeremyevans.net>
Date: Fri, 5 Jul 2024 08:15:47 -0700
Subject: [PATCH] Raise a TypeError for Thread#thread_variable{?,_get} for
 non-symbol

Previously, a TypeError was not raised if there were no thread
variables, because the conversion to symbol was done after that
check.  Convert to symbol before checking for whether thread
variables are set to make the behavior consistent.

Fixes [Bug #20606]
---
 .../core/thread/thread_variable_get_spec.rb   | 19 +++++++++++++------
 spec/ruby/core/thread/thread_variable_spec.rb | 19 +++++++++++++------
 thread.c                                      |  6 ++++--
 3 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/spec/ruby/core/thread/thread_variable_get_spec.rb b/spec/ruby/core/thread/thread_variable_get_spec.rb
index 67017771fd..e8e03e3d18 100644
--- a/spec/ruby/core/thread/thread_variable_get_spec.rb
+++ b/spec/ruby/core/thread/thread_variable_get_spec.rb
@@ -41,13 +41,20 @@ describe "Thread#thread_variable_get" do
     @t.thread_variable_get(:a).should be_nil
   end
 
-  it "does not raise a TypeError if the key is neither Symbol nor String, nor responds to #to_str" do
-    @t.thread_variable_get(123).should be_nil
+  it "raises a TypeError if the key is neither Symbol nor String when thread variables are already set" do
+    @t.thread_variable_set(:a, 49)
+    -> { @t.thread_variable_get(123) }.should raise_error(TypeError, "123 is not a symbol")
   end
 
-  it "does not try to convert the key with #to_sym" do
-    key = mock('key')
-    key.should_not_receive(:to_sym)
-    @t.thread_variable_get(key).should be_nil
+  ruby_version_is '3.4' do
+    it "raises a TypeError if the key is neither Symbol nor String when no thread variables are set" do
+      -> { @t.thread_variable_get(123) }.should raise_error(TypeError, "123 is not a symbol")
+    end
+
+    it "raises a TypeError if the key is neither Symbol nor String without calling #to_sym" do
+      key = mock('key')
+      key.should_not_receive(:to_sym)
+      -> { @t.thread_variable_get(key) }.should raise_error(TypeError, "#{key.inspect} is not a symbol")
+    end
   end
 end
diff --git a/spec/ruby/core/thread/thread_variable_spec.rb b/spec/ruby/core/thread/thread_variable_spec.rb
index d64e6ec63d..465b985365 100644
--- a/spec/ruby/core/thread/thread_variable_spec.rb
+++ b/spec/ruby/core/thread/thread_variable_spec.rb
@@ -41,13 +41,20 @@ describe "Thread#thread_variable?" do
     @t.thread_variable?(:a).should be_false
   end
 
-  it "does not raise a TypeError if the key is neither Symbol nor String, nor responds to #to_str" do
-    @t.thread_variable?(123).should be_false
+  it "raises a TypeError if the key is neither Symbol nor String when thread variables are already set" do
+    @t.thread_variable_set(:a, 49)
+    -> { @t.thread_variable?(123) }.should raise_error(TypeError, "123 is not a symbol")
   end
 
-  it "does not try to convert the key with #to_sym" do
-    key = mock('key')
-    key.should_not_receive(:to_sym)
-    @t.thread_variable?(key).should be_false
+  ruby_version_is '3.4' do
+    it "raises a TypeError if the key is neither Symbol nor String when no thread variables are set" do
+      -> { @t.thread_variable?(123) }.should raise_error(TypeError, "123 is not a symbol")
+    end
+
+    it "raises a TypeError if the key is neither Symbol nor String without calling #to_sym" do
+      key = mock('key')
+      key.should_not_receive(:to_sym)
+      -> { @t.thread_variable?(key) }.should raise_error(TypeError, "#{key.inspect} is not a symbol")
+    end
   end
 end
diff --git a/thread.c b/thread.c
index 56f1399d41..45aa0565ca 100644
--- a/thread.c
+++ b/thread.c
@@ -3756,12 +3756,13 @@ static VALUE
 rb_thread_variable_get(VALUE thread, VALUE key)
 {
     VALUE locals;
+    VALUE symbol = rb_to_symbol(key);
 
     if (LIKELY(!THREAD_LOCAL_STORAGE_INITIALISED_P(thread))) {
         return Qnil;
     }
     locals = rb_thread_local_storage(thread);
-    return rb_hash_aref(locals, rb_to_symbol(key));
+    return rb_hash_aref(locals, symbol);
 }
 
 /*
@@ -3912,13 +3913,14 @@ static VALUE
 rb_thread_variable_p(VALUE thread, VALUE key)
 {
     VALUE locals;
+    VALUE symbol = rb_to_symbol(key);
 
     if (LIKELY(!THREAD_LOCAL_STORAGE_INITIALISED_P(thread))) {
         return Qfalse;
     }
     locals = rb_thread_local_storage(thread);
 
-    return RBOOL(rb_hash_lookup(locals, rb_to_symbol(key)) != Qnil);
+    return RBOOL(rb_hash_lookup(locals, symbol) != Qnil);
 }
 
 /*