Re: [PR] HBASE-30070 Replace deprecated JRuby LoadService#findFileForLoad with $LOAD_PATH.resolve_feature_path [hbase]
junegunn merged PR #8052: URL: https://github.com/apache/hbase/pull/8052 -- 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]
Re: [PR] HBASE-30070 Replace deprecated JRuby LoadService#findFileForLoad with $LOAD_PATH.resolve_feature_path [hbase]
junegunn commented on PR #8052: URL: https://github.com/apache/hbase/pull/8052#issuecomment-4273745278 Okay, now I see the point. While I can't imagine an HBase shell user relying on `RUBYLIB`, it is probable that they bundle some scripts in a jar file and add it to the CLASSPATH like so: ```sh mkdir -p /tmp/scripts echo 'puts 12345' > /tmp/scripts/demo.rb jar -cf scripts.jar -C /tmp scripts/ bin/hbase shell -n scripts/demo.rb # ERROR LoadError: no such file to load -- scripts/demo.rb HBASE_CLASSPATH=scripts.jar bin/hbase shell -n scripts/demo.rb # hbase:001:0> puts 12345 # 12345 # Without fix HBASE_CLASSPATH=scripts.jar bin/hbase shell -n scripts/demo.rb # NoMethodError: undefined method `workspace' for # # Did you mean? get_workspace ``` The patch fixes the problem and it works as expected. Approved. -- 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]
Re: [PR] HBASE-30070 Replace deprecated JRuby LoadService#findFileForLoad with $LOAD_PATH.resolve_feature_path [hbase]
junegunn commented on PR #8052: URL: https://github.com/apache/hbase/pull/8052#issuecomment-4221083872 @NihalJain Hi, could you take a look at this? We still have some code that was removed in JRuby 9.4. -- 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]
Re: [PR] HBASE-30070 Replace deprecated JRuby LoadService#findFileForLoad with $LOAD_PATH.resolve_feature_path [hbase]
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 # 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 => [#, "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]
