From 969f9798cd7edfaef02c3735b28c235fa43a2d36 Mon Sep 17 00:00:00 2001 From: arvindksi274-ksolves Date: Fri, 13 Mar 2026 16:01:37 +0530 Subject: [PATCH 1/2] HBASE-28660 list_namespace not working after an incorrect user input Fixes an issue where mistakenly parsed local variables shadow HBase shell commands. Added cleanup logic in IRB eval loop to replace polluted bindings, along with a warning message for the user. Added relevant test cases in general_test_cluster.rb. --- hbase-shell/src/main/ruby/irb/hirb.rb | 17 ++++++ .../test/ruby/shell/general_test_cluster.rb | 59 +++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/hbase-shell/src/main/ruby/irb/hirb.rb b/hbase-shell/src/main/ruby/irb/hirb.rb index 73b0ee91a11d..c86223e35787 100644 --- a/hbase-shell/src/main/ruby/irb/hirb.rb +++ b/hbase-shell/src/main/ruby/irb/hirb.rb @@ -163,6 +163,23 @@ def eval_input else exc = nil next + ensure + # HBASE-28660: Prevent command shadowing by incorrectly parsed local variables + cmd_names = ::Shell.commands.keys.map(&:to_sym) + workspace_binding = @context.workspace.binding + shadowing_vars = workspace_binding.local_variables & cmd_names + + if shadowing_vars.any? + shadowing_vars.each do |var| + puts "WARN: '#{var}' is a reserved HBase command. Local variable assignment ignored." + end + + new_binding = @context.workspace.main.get_binding + (workspace_binding.local_variables - shadowing_vars).each do |var| + new_binding.local_variable_set(var, workspace_binding.local_variable_get(var)) + end + @context.instance_variable_set(:@workspace, ::IRB::WorkSpace.new(new_binding)) + end end handle_exception(exc) @context.workspace.local_variable_set(:_, exc) diff --git a/hbase-shell/src/test/ruby/shell/general_test_cluster.rb b/hbase-shell/src/test/ruby/shell/general_test_cluster.rb index e1461fd935bd..7778d5a387dc 100644 --- a/hbase-shell/src/test/ruby/shell/general_test_cluster.rb +++ b/hbase-shell/src/test/ruby/shell/general_test_cluster.rb @@ -19,6 +19,7 @@ require 'hbase_constants' require 'hbase_shell' +require 'irb/hirb' class ShellTest < Test::Unit::TestCase include Hbase::TestHelpers @@ -149,4 +150,62 @@ class TestException < RuntimeError; end # create a table that exists @shell.command('create', 'nothrow_table', 'family_1') end + + #----------------------------------------------------------------------------- + + class MockInputMethod < IRB::InputMethod + def initialize(lines) + super() + @lines = lines + end + def gets + @lines.shift + end + def eof? + @lines.empty? + end + def encoding + Encoding::UTF_8 + end + def readable_after_eof? + false + end + end + + define_test 'Shell::Shell should prevent HBase commands from being shadowed by local variables (HBASE-28660)' do + workspace = @shell.get_workspace + IRB.setup(__FILE__) unless IRB.conf[:IRB_NAME] + + lines = [ + "list = 10\n", + "list_namespace, 'ns.*'\n", + "my_var = 5\n" + ] + + input_method = MockInputMethod.new(lines) + hirb = IRB::HIRB.new(workspace, true, input_method) + + hirb.context.prompt_i = "" + hirb.context.prompt_s = "" + hirb.context.prompt_c = "" + hirb.context.prompt_n = "" + hirb.context.return_format = "" + hirb.context.echo = false + + output = capture_stdout do + hirb.eval_input + end + + final_workspace = hirb.context.workspace + final_vars = final_workspace.binding.local_variables + + assert(final_vars.include?(:my_var), "Valid variables should be preserved") + assert_equal(5, final_workspace.binding.local_variable_get(:my_var)) + + assert(!final_vars.include?(:list), "Command 'list' should not be shadowed") + assert(!final_vars.include?(:list_namespace), "Command 'list_namespace' should not be shadowed") + + assert_match(/WARN: 'list' is a reserved HBase command/, output) + assert_match(/WARN: 'list_namespace' is a reserved HBase command/, output) + end end From 84f4673c0cda5c930553b8e1dea7718890a1f504 Mon Sep 17 00:00:00 2001 From: arvindksi274-ksolves Date: Fri, 13 Mar 2026 22:17:47 +0530 Subject: [PATCH 2/2] Apply Copilot review suggestions: use warn, memoize command names, and safe workspace setter --- hbase-shell/src/main/ruby/irb/hirb.rb | 18 +++++++++++++++--- .../test/ruby/shell/general_test_cluster.rb | 17 +++++++++++++---- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/hbase-shell/src/main/ruby/irb/hirb.rb b/hbase-shell/src/main/ruby/irb/hirb.rb index c86223e35787..34638f228a9e 100644 --- a/hbase-shell/src/main/ruby/irb/hirb.rb +++ b/hbase-shell/src/main/ruby/irb/hirb.rb @@ -23,6 +23,10 @@ module IRB # Subclass of IRB so can intercept methods class HIRB < Irb + def self.command_names + @command_names ||= ::Shell.commands.keys.map(&:to_sym).freeze + end + def initialize(workspace = nil, interactive = true, input_method = nil) # This is ugly. Our 'help' method above provokes the following message # on irb construction: 'irb: warn: can't alias help from irb_help.' @@ -53,6 +57,14 @@ def initialize(workspace = nil, interactive = true, input_method = nil) $stdout = STDOUT end + def set_context_workspace(workspace) + if @context.respond_to?(:workspace=) + @context.workspace = workspace + else + @context.instance_variable_set(:@workspace, workspace) + end + end + def output_value(omit = false) # Suppress output if last_value is 'nil' # Otherwise, when user types help, get ugly 'nil' @@ -165,20 +177,20 @@ def eval_input next ensure # HBASE-28660: Prevent command shadowing by incorrectly parsed local variables - cmd_names = ::Shell.commands.keys.map(&:to_sym) + cmd_names = self.class.command_names workspace_binding = @context.workspace.binding shadowing_vars = workspace_binding.local_variables & cmd_names if shadowing_vars.any? shadowing_vars.each do |var| - puts "WARN: '#{var}' is a reserved HBase command. Local variable assignment ignored." + warn "WARN: '#{var}' is a reserved HBase command. Local variable assignment ignored." end new_binding = @context.workspace.main.get_binding (workspace_binding.local_variables - shadowing_vars).each do |var| new_binding.local_variable_set(var, workspace_binding.local_variable_get(var)) end - @context.instance_variable_set(:@workspace, ::IRB::WorkSpace.new(new_binding)) + set_context_workspace(::IRB::WorkSpace.new(new_binding)) end end handle_exception(exc) diff --git a/hbase-shell/src/test/ruby/shell/general_test_cluster.rb b/hbase-shell/src/test/ruby/shell/general_test_cluster.rb index 7778d5a387dc..3c99fe8e17e8 100644 --- a/hbase-shell/src/test/ruby/shell/general_test_cluster.rb +++ b/hbase-shell/src/test/ruby/shell/general_test_cluster.rb @@ -20,6 +20,7 @@ require 'hbase_constants' require 'hbase_shell' require 'irb/hirb' +require 'stringio' class ShellTest < Test::Unit::TestCase include Hbase::TestHelpers @@ -192,8 +193,16 @@ def readable_after_eof? hirb.context.return_format = "" hirb.context.echo = false - output = capture_stdout do - hirb.eval_input + old_stderr = $stderr + $stderr = StringIO.new + err_output = "" + begin + capture_stdout do + hirb.eval_input + end + ensure + err_output = $stderr.string + $stderr = old_stderr end final_workspace = hirb.context.workspace @@ -205,7 +214,7 @@ def readable_after_eof? assert(!final_vars.include?(:list), "Command 'list' should not be shadowed") assert(!final_vars.include?(:list_namespace), "Command 'list_namespace' should not be shadowed") - assert_match(/WARN: 'list' is a reserved HBase command/, output) - assert_match(/WARN: 'list_namespace' is a reserved HBase command/, output) + assert_match(/WARN: 'list' is a reserved HBase command/, err_output) + assert_match(/WARN: 'list_namespace' is a reserved HBase command/, err_output) end end