From 71e25f2bcb4171bc5040c2a9909f9f0be83592e5 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 90df199d5..9b79248e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,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 1e834e7b3..465cd2532 100644 --- a/src/main/ruby/jruby/rack/booter.rb +++ b/src/main/ruby/jruby/rack/booter.rb @@ -131,6 +131,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 cd7cc4df0..7629ab353 100644 --- a/src/spec/ruby/jruby/rack/booter_spec.rb +++ b/src/spec/ruby/jruby/rack/booter_spec.rb @@ -20,15 +20,24 @@ end 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 @@ -93,8 +102,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! @@ -102,49 +112,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 @@ -156,6 +192,7 @@ booter.boot! expect(ENV['GEM_PATH']).to eq "/blah/gems" + expect(Gem.path).to start_with ["/blah/gems"] end before { $loaded_init_rb = nil } @@ -209,11 +246,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 64863f341..0fdc4163c 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 bbbad305553e796016edb898715121885047924d 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 | 8 +++- src/main/ruby/jruby/rack/booter.rb | 60 +++++++++++-------------- src/spec/ruby/jruby/rack/booter_spec.rb | 24 +++++----- 4 files changed, 47 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b79248e3..3a93419e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,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 63257bc90..93e709473 100644 --- a/README.md +++ b/README.md @@ -233,7 +233,13 @@ as context init parameters in web.xml or as VM-wide system properties. this option to en empty string (or 'false') it acts as if the ENV hash was cleared out (similar to the now removed `jruby.rack.ignore.env` option). - `jruby.runtime.env.rubyopt`: Set to true to cause ENV['RUBYOPT'] - to be retained even when using `jruby.runtime.env` to override environemnt (similar to how the removed `jruby.rack.ignore.env` option behaved by default). + to be retained even when using `jruby.runtime.env` to override the environment. +- `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 465cd2532..20bff3e6a 100644 --- a/src/main/ruby/jruby/rack/booter.rb +++ b/src/main/ruby/jruby/rack/booter.rb @@ -98,55 +98,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 7629ab353..4058aa826 100644 --- a/src/spec/ruby/jruby/rack/booter_spec.rb +++ b/src/spec/ruby/jruby/rack/booter_spec.rb @@ -101,16 +101,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 @@ -159,6 +149,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 c13e05c78a464a02418249f19bdce43445823eb3 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 3a93419e3..32a95bd6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,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 d36f88c8bd9295d8f901634348e65c589eabcb1d 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 8285aeb0f..3ac61f61f 100644 --- a/src/main/ruby/jruby/rack.rb +++ b/src/main/ruby/jruby/rack.rb @@ -60,7 +60,6 @@ def logdev; ServletLog.new(context) 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'