From 8bbcbdfb4129bc340e8984af5fb1049fb1d1c2b0 Mon Sep 17 00:00:00 2001 From: Chad Wilson <29788154+chadlwilson@users.noreply.github.com> Date: Fri, 10 Oct 2025 23:43:19 +0800 Subject: [PATCH 1/4] [fix] Ensure that jruby-rack leaves ENV vars and Gem.path in consistent state As noted at https://github.com/jruby/warbler/pull/575 for warbler, it is similarly possible for jruby-rack to leave ENV['GEM_PATH'] and Gem.path in an inconsistent state after it updates them. This would make any later Gem.clear_paths or Bundler usage (driven from the ENV vars) unpredictable. This happens because jruby-rack alters `ENV['GEM_PATH']``, in some cases but does not force Rubygems to recalculate Gem.paths. Normally this is fine, however if Gem.paths had already been used and cached prior to this mangling `Gem.path` will contain a "stale" value with respect to ENV. (e.g possibly missing the `GEM_HOME`/`Gem.default_dir` value, or retaining values from an old `GEM_PATH`. Clearing the paths is the only way to ensure things are consistent for later usage. In particular this currently can happen on JRuby 9.4+ because often `Gem.path` is already initialized by the time we boot, due to the use of default gems such as `stringio`. Rather than assume a given state, it would be better to ensure it is consistent by clearing paths whenever we touch `ENV['GEM_PATH']`. --- CHANGELOG.md | 1 + src/main/ruby/jruby/rack/booter.rb | 2 + src/spec/ruby/jruby/rack/booter_spec.rb | 62 +++++++++++++++---- src/spec/ruby/jruby/rack/rails_booter_spec.rb | 8 +-- 4 files changed, 57 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4b0cafe5..2433b5682 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ - Add missing block-only signature for debug logging - update (bundled) rack to 2.2.20 +- Ensure rack boot process leaves ENV['GEM_PATH'] and Gem.paths in a consistent state ## 1.2.5 diff --git a/src/main/ruby/jruby/rack/booter.rb b/src/main/ruby/jruby/rack/booter.rb index 2d359d267..2e47b2ed5 100644 --- a/src/main/ruby/jruby/rack/booter.rb +++ b/src/main/ruby/jruby/rack/booter.rb @@ -137,6 +137,8 @@ def adjust_gem_path else # 'jruby.rack.env.gem_path' "forced" to an explicit value ENV['GEM_PATH'] = set_gem_path end + # Whenever we touch ENV['GEM_PATH`], ensure we clear any cached paths. All other cases should exit early. + Gem.clear_paths if defined?(Gem.clear_paths) end # @return whether to update Gem.path and/or the environment GEM_PATH diff --git a/src/spec/ruby/jruby/rack/booter_spec.rb b/src/spec/ruby/jruby/rack/booter_spec.rb index 6a8752710..4c100b0c8 100644 --- a/src/spec/ruby/jruby/rack/booter_spec.rb +++ b/src/spec/ruby/jruby/rack/booter_spec.rb @@ -17,15 +17,24 @@ after(:all) { JRuby::Rack.context = nil } before do - @rack_env = ENV['RACK_ENV'] - @gem_path = Gem.path.to_a - @env_gem_path = ENV['GEM_PATH'] + # start clean, in case another test was messing with paths + Gem.clear_paths + @original_rack_env = ENV['RACK_ENV'] + @original_gem_path = Gem.path.to_a + @original_env_gem_path = ENV['GEM_PATH'] end after do - @rack_env.nil? ? ENV.delete('RACK_ENV') : ENV['RACK_ENV'] = @rack_env - Gem.path.replace(@gem_path) - @env_gem_path.nil? ? ENV.delete('GEM_PATH') : ENV['GEM_PATH'] = @env_gem_path + # Ensure everything is reset how it was + @original_rack_env.nil? ? ENV.delete('RACK_ENV') : ENV['RACK_ENV'] = @original_rack_env + @original_env_gem_path.nil? ? ENV.delete('GEM_PATH') : ENV['GEM_PATH'] = @original_env_gem_path + Gem.clear_paths + Gem.path.replace(@original_gem_path) + + aggregate_failures("expected Gem.path to be restored after test") do + expect(ENV['GEM_PATH']).to eq @original_env_gem_path + expect(Gem.path).to eql @original_gem_path + end end it "should determine the public html root from the 'public.root' init parameter" do @@ -90,8 +99,9 @@ end it "prepends gem_path to Gem.path (when configured to not mangle with ENV)" do - expect(@rack_context).to receive(:getInitParameter).with("jruby.rack.env.gem_path").and_return 'false' Gem.path.replace ['/opt/gems'] + expect(@rack_context).to receive(:getInitParameter).with("jruby.rack.env.gem_path").and_return 'false' + booter.gem_path = "wsjar:file:/opt/deploy/sample.war!/WEB-INF/gems" booter.boot! @@ -99,49 +109,75 @@ end it "prepends gem_path to Gem.path if not already present" do - Gem.path.replace ["file:/home/gems", "/usr/local/gems"] + ENV['GEM_PATH'] = "file:/home/gems#{File::PATH_SEPARATOR}/usr/local/gems" + Gem.clear_paths + booter.gem_path = '/usr/local/gems' booter.boot! - expect(Gem.path).to eql ["file:/home/gems", "/usr/local/gems"] + expect(Gem.path).to start_with ["file:/home/gems", "/usr/local/gems"] + expect(ENV['GEM_PATH']).to eq "file:/home/gems#{File::PATH_SEPARATOR}/usr/local/gems" end it "does not change Gem.path if gem_path empty" do - Gem.path.replace ['/opt/gems'] + ENV['GEM_PATH'] = '/opt/gems' + Gem.clear_paths + booter.gem_path = "" booter.boot! - expect(Gem.path).to eql ['/opt/gems'] + expect(Gem.path).to start_with ['/opt/gems'] + expect(ENV['GEM_PATH']).to eq '/opt/gems' end it "prepends gem_path to ENV['GEM_PATH'] if jruby.rack.gem_path set to true" do expect(@rack_context).to receive(:getInitParameter).with("jruby.rack.env.gem_path").and_return 'true' ENV['GEM_PATH'] = '/opt/gems' + Gem.clear_paths expect(@rack_context).to receive(:getRealPath).with("/WEB-INF").and_return "/opt/deploy/sample.war!/WEB-INF" expect(@rack_context).to receive(:getRealPath).with("/WEB-INF/gems").and_return "/opt/deploy/sample.war!/WEB-INF/gems" booter.boot! expect(ENV['GEM_PATH']).to eq "/opt/deploy/sample.war!/WEB-INF/gems#{File::PATH_SEPARATOR}/opt/gems" + expect(Gem.path).to start_with ["/opt/deploy/sample.war!/WEB-INF/gems", "/opt/gems"] end it "does not prepend gem_path to ENV['GEM_PATH'] if jruby.rack.gem_path set not set" do expect(@rack_context).to receive(:getInitParameter).with("jruby.rack.env.gem_path").and_return '' ENV['GEM_PATH'] = '/opt/gems' + Gem.clear_paths expect(@rack_context).to receive(:getRealPath).with("/WEB-INF").and_return "/opt/deploy/sample.war!/WEB-INF" expect(@rack_context).to receive(:getRealPath).with("/WEB-INF/gems").and_return "/opt/deploy/sample.war!/WEB-INF/gems" booter.boot! expect(ENV['GEM_PATH']).to eq "/opt/gems" + expect(Gem.path).to start_with ["/opt/gems"] end it "prepends gem_path to ENV['GEM_PATH'] if not already present" do ENV['GEM_PATH'] = "/home/gems#{File::PATH_SEPARATOR}/usr/local/gems" + Gem.clear_paths booter.gem_path = '/usr/local/gems' booter.boot! expect(ENV['GEM_PATH']).to eq "/home/gems#{File::PATH_SEPARATOR}/usr/local/gems" + expect(Gem.path).to start_with ["/home/gems", "/usr/local/gems"] + end + + it "keeps ENV['GEM_PATH'] when gem_path is nil" do + ENV['GEM_PATH'] = '/usr/local/gems' + Gem.clear_paths + + booter.layout = layout = double('layout') + allow(layout).to receive(:app_path).and_return '.' + allow(layout).to receive(:public_path).and_return nil + expect(layout).to receive(:gem_path).and_return nil + booter.boot! + + expect(ENV['GEM_PATH']).to eq "/usr/local/gems" + expect(Gem.path).to start_with ["/usr/local/gems"] end it "sets ENV['GEM_PATH'] to the value of gem_path if ENV['GEM_PATH'] is not present" do @@ -153,6 +189,7 @@ booter.boot! expect(ENV['GEM_PATH']).to eq "/blah/gems" + expect(Gem.path).to start_with ["/blah/gems"] end it "creates a logger that writes messages to the servlet context (by default)" do @@ -214,11 +251,12 @@ # at RUBY.boot!(classpath:/jruby/rack/booter.rb:105) # at RUBY.(root)(classpath:/jruby/rack/boot/rack.rb:10) app_dir = "#{File.absolute_path Dir.pwd}/sample.war!/WEB-INF" - allow(File).to receive(:directory?).with(app_dir).and_return true allow(booter).to receive(:layout).and_return layout = double('layout') allow(layout).to receive(:app_path).and_return app_dir allow(layout).to receive(:gem_path) allow(layout).to receive(:public_path) + allow(File).to receive(:directory?).and_wrap_original { |m, *args| m.call(*args) } + expect(File).to receive(:directory?).with(app_dir).and_return true booter.boot! # expect to_not raise_error end diff --git a/src/spec/ruby/jruby/rack/rails_booter_spec.rb b/src/spec/ruby/jruby/rack/rails_booter_spec.rb index 63d4a8efd..1d2db501d 100644 --- a/src/spec/ruby/jruby/rack/rails_booter_spec.rb +++ b/src/spec/ruby/jruby/rack/rails_booter_spec.rb @@ -30,13 +30,13 @@ end before do - @rails_env = ENV['RAILS_ENV'] - @rack_env = ENV['RACK_ENV'] + @original_rails_env = ENV['RAILS_ENV'] + @original_rack_env = ENV['RACK_ENV'] end after do - @rails_env.nil? ? ENV.delete('RAILS_ENV') : ENV['RAILS_ENV'] = @rails_env - @rack_env.nil? ? ENV.delete('RACK_ENV') : ENV['RACK_ENV'] = @rack_env + @original_rails_env.nil? ? ENV.delete('RAILS_ENV') : ENV['RAILS_ENV'] = @original_rails_env + @original_rack_env.nil? ? ENV.delete('RACK_ENV') : ENV['RACK_ENV'] = @original_rack_env end it "should default rails path to /WEB-INF" do From ac038be9800e22f9547cb24bb6d710d6120a5e07 Mon Sep 17 00:00:00 2001 From: Chad Wilson <29788154+chadlwilson@users.noreply.github.com> Date: Sat, 11 Oct 2025 00:26:34 +0800 Subject: [PATCH 2/4] [fix] Remove the undocumented and unsafe jruby.rack.env.gem_path = false init parameter option Changes the `false` value to behave the same way as empty, and not touch ENV['GEM_PATH'] or Gem.path at all. Previously this allowed you to alter Gem.path without touching the ENV['GEM_PATH']'. This was unsafe with Bundler 1.6+ (so for a long time), because many things may cause Gem.path to be recalculated from env, including bundler initialisations, restarts and later changes within an application's boot hooks. Furthermore the behaviour varied wildly depending on whether Rubygems was initialised prior to the Rack booter, or afterwards, even without bundler causing indeterminism and issues similar to https://github.com/jruby/warbler/pull/575 I cannot find any reference to use of this value on GitHub, or discussion, and it would not work at all with modern JRuby/bundler so removing it, because it helps avoid further issues. Note that it was added in https://github.com/jruby/jruby-rack/commit/2fd826b233f288b5bc687d33e8d2ced15a9157a6 and then the default changed back in https://github.com/jruby/jruby-rack/commit/d3ee8f04302856db83c36f8825355584be23c4f4 shortly after when issues were discovered, so given it's undocumented/unsupported, it is unlikely to be relied upon by anyone. --- CHANGELOG.md | 1 + README.md | 6 +++ src/main/ruby/jruby/rack/booter.rb | 60 +++++++++++-------------- src/spec/ruby/jruby/rack/booter_spec.rb | 24 +++++----- 4 files changed, 46 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2433b5682..6e3452a3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ - Add missing block-only signature for debug logging - update (bundled) rack to 2.2.20 - Ensure rack boot process leaves ENV['GEM_PATH'] and Gem.paths in a consistent state +- Remove undocumented and unsafe jruby.rack.env.gem_path = false option (unusable on Bundler 1.6+) ## 1.2.5 diff --git a/README.md b/README.md index 35b0555f6..39b23ef8b 100644 --- a/README.md +++ b/README.md @@ -233,6 +233,12 @@ as context init parameters in web.xml or as VM-wide system properties. - `jruby.runtime.env.rubyopt`: This option is used for compatibility with the (deprecated) `jruby.rack.ignore.env` option since it cleared out the ENV after RUBYOPT has been processed, by setting it to true ENV['RUBYOPT'] will be kept. +- `jruby.rack.env.gem_path`: If set to `true` (the default) jruby-rack will + ensure ENV['GEM_PATH'] is altered to include the `gem.path` above. If you set it to a + value, this value will be used as GEM_PATH, overriding the environment and + ignoring `gem.path` etc. By setting this option to en empty string the ENV['GEM_PATH'] will + not be modified by jruby-rack at all and will retain its original values implied by + the process environment and `jruby.runtime.env` setting. - `jruby.rack.logging`: Specify the logging device to use. Defaults to `servlet_context`. See below. - `jruby.rack.request.size.initial.bytes`: Initial size for request body memory diff --git a/src/main/ruby/jruby/rack/booter.rb b/src/main/ruby/jruby/rack/booter.rb index 2e47b2ed5..958deee42 100644 --- a/src/main/ruby/jruby/rack/booter.rb +++ b/src/main/ruby/jruby/rack/booter.rb @@ -104,55 +104,45 @@ def boot! protected def adjust_gem_path - gem_path = self.gem_path + desired_gem_path = self.gem_path + case set_gem_path = env_gem_path - when true then - if env_path = ENV['GEM_PATH'] - if gem_path.nil? || gem_path.empty? - return # keep ENV['GEM_PATH'] as is - elsif env_path != gem_path - separator = File::PATH_SEPARATOR - unless env_path.split(separator).include?(gem_path) - ENV['GEM_PATH'] = "#{gem_path}#{separator}#{env_path}" - end - end - else - ENV['GEM_PATH'] = gem_path - end - when false then - begin - require 'rubygems' unless defined? Gem.path - rescue LoadError + when true then # default behaviour + if (current_env_gem_path = ENV['GEM_PATH']) + # keep ENV['GEM_PATH'] as is if we have nothing to do + return if desired_gem_path.nil? || desired_gem_path.empty? + return if current_env_gem_path == desired_gem_path + return if current_env_gem_path.split(File::PATH_SEPARATOR).include?(desired_gem_path) + + # need to prepend it + ENV['GEM_PATH'] = "#{desired_gem_path}#{File::PATH_SEPARATOR}#{current_env_gem_path}" else - return if gem_path.nil? || gem_path.empty? - Gem.path.unshift(gem_path) unless Gem.path.include?(gem_path) + ENV['GEM_PATH'] = desired_gem_path end - return false - when nil then # org.jruby.rack.RackLogger::DEBUG - if gem_path && ! gem_path.empty? && - ( ! defined?(Gem.path) || ! Gem.path.include?(gem_path) ) - @rack_context.log("Gem.path won't be updated although seems configured: #{gem_path}") + when nil then + if desired_gem_path && !desired_gem_path.empty? && (!defined?(Gem.path) || !Gem.path.include?(desired_gem_path) ) + @rack_context.log("Gem.path won't be updated although seems configured: #{desired_gem_path}") end - return nil - else # 'jruby.rack.env.gem_path' "forced" to an explicit value + return nil # do nothing to ENV['GEM_PATH'] + else # "forced" to an explicit value ENV['GEM_PATH'] = set_gem_path end # Whenever we touch ENV['GEM_PATH`], ensure we clear any cached paths. All other cases should exit early. Gem.clear_paths if defined?(Gem.clear_paths) end - # @return whether to update Gem.path and/or the environment GEM_PATH - # - true (default) forces ENV['GEM_PATH'] to be updated due compatibility - # Bundler 1.6 fails to revolve gems correctly when Gem.path is updated - # instead of the ENV['GEM_PATH'] environment variable - # - false disables ENV['GEM_PATH'] mangling for good (updates Gem.path) + # @return whether to update the environment GEM_PATH + # - true (default) forces ENV['GEM_PATH'] to be updated to include the `gem.path` above. + # If you set it to a non-empty value, GEM_PATH will be forced to an explicit value, + # overriding the environment and ignoring `gem.path`, `gem.home` etc. # - # - if not specified Gem.path will be updated based on setting + # - By setting this option to an empty string the ENV['GEM_PATH'] should not be modified + # at all and will retain its original values implied by the process environment and + # `jruby.runtime.env` setting. def env_gem_path gem_path = @rack_context.getInitParameter('jruby.rack.env.gem_path') return true if gem_path.nil? || gem_path.to_s == 'true' - return false if gem_path.to_s == 'false' - return nil if gem_path.empty? # set to an empty disables mangling + return nil if gem_path.empty? || gem_path.to_s == 'false' # treat false as "don't touch either ENV or Gem.path" gem_path end private :env_gem_path diff --git a/src/spec/ruby/jruby/rack/booter_spec.rb b/src/spec/ruby/jruby/rack/booter_spec.rb index 4c100b0c8..91743e651 100644 --- a/src/spec/ruby/jruby/rack/booter_spec.rb +++ b/src/spec/ruby/jruby/rack/booter_spec.rb @@ -98,16 +98,6 @@ expect(booter.rack_env).to eq 'production' end - it "prepends gem_path to Gem.path (when configured to not mangle with ENV)" do - Gem.path.replace ['/opt/gems'] - expect(@rack_context).to receive(:getInitParameter).with("jruby.rack.env.gem_path").and_return 'false' - - booter.gem_path = "wsjar:file:/opt/deploy/sample.war!/WEB-INF/gems" - booter.boot! - - expect(Gem.path).to eql ['wsjar:file:/opt/deploy/sample.war!/WEB-INF/gems', '/opt/gems'] - end - it "prepends gem_path to Gem.path if not already present" do ENV['GEM_PATH'] = "file:/home/gems#{File::PATH_SEPARATOR}/usr/local/gems" Gem.clear_paths @@ -156,6 +146,20 @@ expect(Gem.path).to start_with ["/opt/gems"] end + it "does not prepend gem_path to ENV['GEM_PATH'] if jruby.rack.gem_path set to false" do + ENV['GEM_PATH'] = '/opt/gems' + Gem.clear_paths + + expect(@rack_context).to receive(:getInitParameter).with("jruby.rack.env.gem_path").and_return 'false' + expect(@rack_context).to receive(:getRealPath).with("/WEB-INF").and_return "/opt/deploy/sample.war!/WEB-INF" + expect(@rack_context).to receive(:getRealPath).with("/WEB-INF/gems").and_return "/opt/deploy/sample.war!/WEB-INF/gems" + + booter.boot! + + expect(ENV['GEM_PATH']).to eq "/opt/gems" + expect(Gem.path).to start_with ["/opt/gems"] + end + it "prepends gem_path to ENV['GEM_PATH'] if not already present" do ENV['GEM_PATH'] = "/home/gems#{File::PATH_SEPARATOR}/usr/local/gems" Gem.clear_paths From 2bdcb4262e830d598bf1ccbcab53d94bfd8edc73 Mon Sep 17 00:00:00 2001 From: Chad Wilson <29788154+chadlwilson@users.noreply.github.com> Date: Sat, 11 Oct 2025 00:40:18 +0800 Subject: [PATCH 3/4] [fix] Avoid requiring 'stringio' so early in boot process On JRuby 9.4 this causes Rubygems initialisation which means realisation of init values. This screws with jruby-rack's own guarantees about env implied by its config values and can cause issues in boot hooks such as https://github.com/jruby/warbler/pull/575 due to Rubygems being in a different state to what they expect. --- CHANGELOG.md | 1 + src/main/ruby/jruby/rack/capture.rb | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e3452a3f..dc2ce17c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - update (bundled) rack to 2.2.20 - Ensure rack boot process leaves ENV['GEM_PATH'] and Gem.paths in a consistent state - Remove undocumented and unsafe jruby.rack.env.gem_path = false option (unusable on Bundler 1.6+) +- Fix unintended Rubygems initialization too early in boot process with JRuby 9.4+ ## 1.2.5 diff --git a/src/main/ruby/jruby/rack/capture.rb b/src/main/ruby/jruby/rack/capture.rb index c583f92c6..b4b95c00c 100644 --- a/src/main/ruby/jruby/rack/capture.rb +++ b/src/main/ruby/jruby/rack/capture.rb @@ -5,13 +5,11 @@ # See the file LICENSE.txt for details. #++ -require 'stringio' - module JRuby::Rack module Capture module Base def output - @output ||= begin; StringIO.new; end + @output ||= begin; require 'stringio' unless defined?(StringIO); StringIO.new; end end def capture @@ -34,6 +32,7 @@ def store module Exception def output @output ||= begin + require 'stringio' unless defined?(StringIO) io = StringIO.new io.puts "An exception happened during JRuby-Rack startup", self.to_s io From 7e0b44ba5f396a04ed3693a807d6cd7245391d89 Mon Sep 17 00:00:00 2001 From: Chad Wilson <29788154+chadlwilson@users.noreply.github.com> Date: Sat, 11 Oct 2025 00:56:53 +0800 Subject: [PATCH 4/4] [chore] Remove outdated comment --- src/main/ruby/jruby/rack.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/ruby/jruby/rack.rb b/src/main/ruby/jruby/rack.rb index 71f2fc551..aeb169a85 100644 --- a/src/main/ruby/jruby/rack.rb +++ b/src/main/ruby/jruby/rack.rb @@ -70,7 +70,6 @@ def context!; context || raise('no context available') end end end -# TODO remove require 'jruby/rack/version' from jruby-rack in 1.2 require 'jruby/rack/version' unless defined? JRuby::Rack::VERSION require 'jruby/rack/helpers' require 'jruby/rack/booter'