junegunn commented on PR #8052:
URL: https://github.com/apache/hbase/pull/8052#issuecomment-4220858907

   Interesting, I'm not sure how many users rely on loading a script file this 
way, but we should definitely update the code.
   
   _Here's my understanding of the issue._
   
   In previous versions, the `SCRIPTFILE` argument of `hbase shell` was not 
limited to the actual file path. It also resolved relative paths against 
`$LOAD_PATH`, so users could set the `RUBYLIB` environment variable to add 
custom directories and load scripts from them.
   
   ```sh
   mkdir -p scripts
   echo "list; exit" > scripts/list.rb
   
   RUBYLIB=scripts bin/hbase shell -n list.rb
   ```
   
   However, this no longer works on the latest master:
   
   ```
   ERROR NoMethodError: undefined method `findFileForLoad' for 
#<Java::OrgJrubyRuntimeLoad::LoadService:0x48788853>
   Did you mean?  findFileInClasspath
   ```
   
   I can confirm the patch fixes the problem.
   
   ---
   
   That said, I'm not sure loading script files from arbitrary `$LOAD_PATH` 
entries is what we intended or is desirable. It seems to pose a security risk 
similar to why adding `.` to `$PATH` is discouraged.
   
   Script resolution from `$LOAD_PATH` was never documented, so we can consider 
restricting this to actual file paths.
   
   - https://hbase.apache.org/book.html#scripting
   
   And by default, the shell doesn't add extra directories to `$LOAD_PATH`.
   
   ```
   bin/hbase shell
   HBase Shell
   Use "help" to get list of supported commands.
   Use "exit" to quit this interactive shell.
   For Reference, please visit: https://hbase.apache.org/docs/shell
   Version 4.0.0-alpha-1-SNAPSHOT, r859dc18330c15f15ad1b4f791bf0871793eeb228, 
Sun Apr  5 09:36:47 KST 2026
   Took 0.0009 seconds
   hbase:001:0> $LOAD_PATH
   =>
   [#<Pathname:/Users/ad02514045/github/hbase/bin/../hbase-shell/src/main/ruby>,
    "uri:classloader:/META-INF/jruby.home/lib/ruby/3.1/site_ruby",
    "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib"]
   ```
   
   ---
   
   The patch is fine and is good to merge, but I think this is a good chance to 
discuss the concern.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to