diff --git a/CHANGELOG.md b/CHANGELOG.md index 90df199d..32a95bd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,9 @@ - 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+) +- Fix unintended Rubygems initialization too early in boot process with JRuby 9.4+ ## 1.2.5 diff --git a/README.md b/README.md index 63257bc9..93e70947 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.rb b/src/main/ruby/jruby/rack.rb index 8285aeb0..3ac61f61 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' diff --git a/src/main/ruby/jruby/rack/booter.rb b/src/main/ruby/jruby/rack/booter.rb index 1e834e7b..20bff3e6 100644 --- a/src/main/ruby/jruby/rack/booter.rb +++ b/src/main/ruby/jruby/rack/booter.rb @@ -98,53 +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/main/ruby/jruby/rack/capture.rb b/src/main/ruby/jruby/rack/capture.rb index c583f92c..b4b95c00 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 diff --git a/src/spec/ruby/jruby/rack/booter_spec.rb b/src/spec/ruby/jruby/rack/booter_spec.rb index cd7cc4df..4058aa82 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 @@ -92,59 +101,90 @@ expect(booter.rack_env).to eq 'production' 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'] - 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 - 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 "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 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 +196,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 +250,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 64863f34..0fdc4163 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