diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index cd1acd8d3..f1049c7c1 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -2,9 +2,9 @@ name: Java CI on: push: - branches: [ "master", "*release*", "*stable*" ] + branches: [ "master", "*-stable" ] pull_request: - branches: [ "master", "*release*", "*stable*" ] + branches: [ "master", "*-stable" ] env: # Default versions for canonical release build diff --git a/History.md b/History.md index 218da2ea7..2f200d7a5 100644 --- a/History.md +++ b/History.md @@ -4,14 +4,16 @@ - Drop unnecessary jruby.compat.version and RackConfig.getCompatVersion() API - Drop JMS support - update (bundled) rack to 2.2.17 +- Fix Rails 7.1 CSRF protection when working with `JavaServletStore` sessions ## 1.2.4 (UNRELEASED) -- update (bundled) rack to 2.2.16 +- update (bundled) rack to 2.2.17 +- Fix Rails 7.1 CSRF protection when working with `JavaServletStore` sessions ## 1.2.3 -- avoid warnings due usage of `File.exists?` +- avoid warnings due to usage of `File.exists?` - Fix Rails 7.1 compatibility by ensuring active_support is required before railtie - Workaround logger require issues with concurrent-ruby 1.3.5 and older Rails versions - Workaround NameError frozen string literal issues with JRuby 9.3 and Rails 5.2/6.0 diff --git a/gemfiles/rails50.gemfile.lock b/gemfiles/rails50.gemfile.lock index 1b3ec0755..1455a3072 100644 --- a/gemfiles/rails50.gemfile.lock +++ b/gemfiles/rails50.gemfile.lock @@ -65,7 +65,7 @@ GEM method_source (1.1.0) mini_mime (1.1.5) minitest (5.25.5) - net-imap (0.5.8) + net-imap (0.5.9) date net-protocol net-pop (0.1.2) @@ -78,7 +78,7 @@ GEM nokogiri (1.18.8-java) racc (~> 1.4) racc (1.8.1-java) - rack (2.2.16) + rack (2.2.17) rack-test (0.6.3) rack (>= 1.0) rails (5.0.7.2) @@ -111,7 +111,7 @@ GEM rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) rspec-mocks (~> 3.13.0) - rspec-core (3.13.4) + rspec-core (3.13.5) rspec-support (~> 3.13.0) rspec-expectations (3.13.5) diff-lcs (>= 1.2.0, < 2.0) diff --git a/gemfiles/rails52.gemfile.lock b/gemfiles/rails52.gemfile.lock index 70c75e6d7..d7742fa03 100644 --- a/gemfiles/rails52.gemfile.lock +++ b/gemfiles/rails52.gemfile.lock @@ -71,7 +71,7 @@ GEM method_source (1.1.0) mini_mime (1.1.5) minitest (5.25.5) - net-imap (0.5.8) + net-imap (0.5.9) date net-protocol net-pop (0.1.2) @@ -84,7 +84,7 @@ GEM nokogiri (1.18.8-java) racc (~> 1.4) racc (1.8.1-java) - rack (2.2.16) + rack (2.2.17) rack-test (2.2.0) rack (>= 1.3) rails (5.2.8.1) @@ -118,7 +118,7 @@ GEM rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) rspec-mocks (~> 3.13.0) - rspec-core (3.13.4) + rspec-core (3.13.5) rspec-support (~> 3.13.0) rspec-expectations (3.13.5) diff-lcs (>= 1.2.0, < 2.0) diff --git a/gemfiles/rails60.gemfile.lock b/gemfiles/rails60.gemfile.lock index 5107ef0c6..d6da09632 100644 --- a/gemfiles/rails60.gemfile.lock +++ b/gemfiles/rails60.gemfile.lock @@ -84,7 +84,7 @@ GEM method_source (1.1.0) mini_mime (1.1.5) minitest (5.25.5) - net-imap (0.5.8) + net-imap (0.5.9) date net-protocol net-pop (0.1.2) @@ -97,7 +97,7 @@ GEM nokogiri (1.18.8-java) racc (~> 1.4) racc (1.8.1-java) - rack (2.2.16) + rack (2.2.17) rack-test (2.2.0) rack (>= 1.3) rails (6.0.6.1) @@ -133,7 +133,7 @@ GEM rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) rspec-mocks (~> 3.13.0) - rspec-core (3.13.4) + rspec-core (3.13.5) rspec-support (~> 3.13.0) rspec-expectations (3.13.5) diff-lcs (>= 1.2.0, < 2.0) @@ -159,7 +159,7 @@ GEM base64 websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) - zeitwerk (2.7.3) + zeitwerk (2.6.18) PLATFORMS universal-java-1.8 diff --git a/gemfiles/rails61.gemfile.lock b/gemfiles/rails61.gemfile.lock index 276ee10da..e03f4411e 100644 --- a/gemfiles/rails61.gemfile.lock +++ b/gemfiles/rails61.gemfile.lock @@ -88,7 +88,7 @@ GEM method_source (1.1.0) mini_mime (1.1.5) minitest (5.25.5) - net-imap (0.5.8) + net-imap (0.5.9) date net-protocol net-pop (0.1.2) @@ -101,7 +101,7 @@ GEM nokogiri (1.18.8-java) racc (~> 1.4) racc (1.8.1-java) - rack (2.2.16) + rack (2.2.17) rack-test (2.2.0) rack (>= 1.3) rails (6.1.7.10) @@ -137,7 +137,7 @@ GEM rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) rspec-mocks (~> 3.13.0) - rspec-core (3.13.4) + rspec-core (3.13.5) rspec-support (~> 3.13.0) rspec-expectations (3.13.5) diff-lcs (>= 1.2.0, < 2.0) @@ -162,7 +162,7 @@ GEM base64 websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) - zeitwerk (2.7.3) + zeitwerk (2.6.18) PLATFORMS universal-java-1.8 diff --git a/gemfiles/rails70.gemfile.lock b/gemfiles/rails70.gemfile.lock index d33f3c66d..933c49868 100644 --- a/gemfiles/rails70.gemfile.lock +++ b/gemfiles/rails70.gemfile.lock @@ -93,7 +93,7 @@ GEM method_source (1.1.0) mini_mime (1.1.5) minitest (5.25.5) - net-imap (0.5.8) + net-imap (0.5.9) date net-protocol net-pop (0.1.2) @@ -106,7 +106,7 @@ GEM nokogiri (1.18.8-java) racc (~> 1.4) racc (1.8.1-java) - rack (2.2.16) + rack (2.2.17) rack-test (2.2.0) rack (>= 1.3) rails (7.0.8.7) @@ -142,7 +142,7 @@ GEM rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) rspec-mocks (~> 3.13.0) - rspec-core (3.13.4) + rspec-core (3.13.5) rspec-support (~> 3.13.0) rspec-expectations (3.13.5) diff-lcs (>= 1.2.0, < 2.0) @@ -159,7 +159,7 @@ GEM base64 websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) - zeitwerk (2.7.3) + zeitwerk (2.6.18) PLATFORMS universal-java-1.8 diff --git a/gemfiles/rails71.gemfile.lock b/gemfiles/rails71.gemfile.lock index 521e903d5..5cca01327 100644 --- a/gemfiles/rails71.gemfile.lock +++ b/gemfiles/rails71.gemfile.lock @@ -84,15 +84,17 @@ GEM thor (>= 0.14.0) base64 (0.3.0) benchmark (0.4.1) - bigdecimal (3.2.1-java) + bigdecimal (3.2.2-java) builder (3.3.0) + cgi (0.5.0-java) concurrent-ruby (1.3.5) connection_pool (2.5.3) crass (1.0.6) date (3.4.1-java) diff-lcs (1.6.2) drb (2.2.3) - erb (5.0.1-java) + erb (4.0.4-java) + cgi (>= 0.3.3) erubi (1.13.1) globalid (1.2.1) activesupport (>= 6.1) @@ -117,7 +119,7 @@ GEM mini_mime (1.1.5) minitest (5.25.5) mutex_m (0.3.0) - net-imap (0.5.8) + net-imap (0.5.9) date net-protocol net-pop (0.1.2) @@ -136,7 +138,7 @@ GEM date jar-dependencies (>= 0.1.7) racc (1.8.1-java) - rack (2.2.16) + rack (2.2.17) rack-session (1.0.2) rack (< 3) rack-test (2.2.0) @@ -174,7 +176,7 @@ GEM thor (~> 1.0, >= 1.2.2) zeitwerk (~> 2.6) rake (13.3.0) - rdoc (6.14.0) + rdoc (6.14.2) erb psych (>= 4.0.0) reline (0.6.1) @@ -183,7 +185,7 @@ GEM rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) rspec-mocks (~> 3.13.0) - rspec-core (3.13.4) + rspec-core (3.13.5) rspec-support (~> 3.13.0) rspec-expectations (3.13.5) diff-lcs (>= 1.2.0, < 2.0) @@ -202,7 +204,7 @@ GEM base64 websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) - zeitwerk (2.7.3) + zeitwerk (2.6.18) PLATFORMS universal-java-1.8 diff --git a/gemfiles/rails72.gemfile.lock b/gemfiles/rails72.gemfile.lock index 75335f228..a225de6ca 100644 --- a/gemfiles/rails72.gemfile.lock +++ b/gemfiles/rails72.gemfile.lock @@ -78,15 +78,17 @@ GEM thor (>= 0.14.0) base64 (0.3.0) benchmark (0.4.1) - bigdecimal (3.2.1-java) + bigdecimal (3.2.2-java) builder (3.3.0) + cgi (0.5.0-java) concurrent-ruby (1.3.5) connection_pool (2.5.3) crass (1.0.6) date (3.4.1-java) diff-lcs (1.6.2) drb (2.2.3) - erb (5.0.1-java) + erb (4.0.4-java) + cgi (>= 0.3.3) erubi (1.13.1) globalid (1.2.1) activesupport (>= 6.1) @@ -110,7 +112,7 @@ GEM marcel (1.0.4) mini_mime (1.1.5) minitest (5.25.5) - net-imap (0.5.8) + net-imap (0.5.9) date net-protocol net-pop (0.1.2) @@ -129,7 +131,7 @@ GEM date jar-dependencies (>= 0.1.7) racc (1.8.1-java) - rack (2.2.16) + rack (2.2.17) rack-session (1.0.2) rack (< 3) rack-test (2.2.0) @@ -167,7 +169,7 @@ GEM thor (~> 1.0, >= 1.2.2) zeitwerk (~> 2.6) rake (13.3.0) - rdoc (6.14.0) + rdoc (6.14.2) erb psych (>= 4.0.0) reline (0.6.1) @@ -176,7 +178,7 @@ GEM rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) rspec-mocks (~> 3.13.0) - rspec-core (3.13.4) + rspec-core (3.13.5) rspec-support (~> 3.13.0) rspec-expectations (3.13.5) diff-lcs (>= 1.2.0, < 2.0) @@ -196,7 +198,7 @@ GEM base64 websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) - zeitwerk (2.7.3) + zeitwerk (2.6.18) PLATFORMS universal-java-1.8 diff --git a/gemfiles/rails80.gemfile.lock b/gemfiles/rails80.gemfile.lock index c4ef5e011..3ff384da1 100644 --- a/gemfiles/rails80.gemfile.lock +++ b/gemfiles/rails80.gemfile.lock @@ -78,7 +78,7 @@ GEM thor (>= 0.14.0) base64 (0.3.0) benchmark (0.4.1) - bigdecimal (3.2.1-java) + bigdecimal (3.2.2-java) builder (3.3.0) concurrent-ruby (1.3.5) connection_pool (2.5.3) @@ -110,7 +110,7 @@ GEM marcel (1.0.4) mini_mime (1.1.5) minitest (5.25.5) - net-imap (0.5.8) + net-imap (0.5.9) date net-protocol net-pop (0.1.2) @@ -129,7 +129,7 @@ GEM date jar-dependencies (>= 0.1.7) racc (1.8.1-java) - rack (2.2.16) + rack (2.2.17) rack-session (1.0.2) rack (< 3) rack-test (2.2.0) @@ -167,7 +167,7 @@ GEM thor (~> 1.0, >= 1.2.2) zeitwerk (~> 2.6) rake (13.3.0) - rdoc (6.14.0) + rdoc (6.14.2) erb psych (>= 4.0.0) reline (0.6.1) @@ -176,7 +176,7 @@ GEM rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) rspec-mocks (~> 3.13.0) - rspec-core (3.13.4) + rspec-core (3.13.5) rspec-support (~> 3.13.0) rspec-expectations (3.13.5) diff-lcs (>= 1.2.0, < 2.0) diff --git a/src/main/ruby/jruby/rack/session_store.rb b/src/main/ruby/jruby/rack/session_store.rb index 9aeb3f473..eaa13f48e 100644 --- a/src/main/ruby/jruby/rack/session_store.rb +++ b/src/main/ruby/jruby/rack/session_store.rb @@ -5,7 +5,7 @@ # See the file LICENSE.txt for details. #++ -require 'rack/session/abstract/id' unless defined?(::Rack::Session::Abstract::ID) +require 'rack/session/abstract/id' unless defined?(::Rack::Session::Abstract::Persisted) module JRuby::Rack module Session @@ -28,7 +28,7 @@ def method_missing(method, *args, &block) end # Rack based SessionStore implementation but compatible with (older) AbstractStore. - class SessionStore < ::Rack::Session::Abstract::ID + class SessionStore < ::Rack::Session::Abstract::Persisted ENV_SERVLET_SESSION_KEY = 'java.servlet_session'.freeze RAILS_SESSION_KEY = "__current_rails_session".freeze @@ -37,13 +37,6 @@ def initialize(app, options={}) super(app, options.merge!(:cookie_only => false, :defer => true)) end - def context(env, app = @app) - req = make_request env - prepare_session(req) - status, headers, body = app.call(req.env) - commit_session(req, status, headers, body) - end - # (public) servlet specific methods : def get_servlet_session(env, create = false) @@ -69,7 +62,7 @@ def get_servlet_session(env, create = false) servlet_session end - private # Rack::Session::Abstract::ID overrides : + private # Rack::Session::Abstract::Persisted overrides : def session_class ::JRuby::Rack::Session::SessionHash @@ -83,6 +76,7 @@ def generate_sid(secure = @sid_secure) nil # dummy method - no session id generation with servlet API end + # Alternative to overriding find_session(req) def load_session(req) # session_id arg for get_session alias session_id, session = false, {} if servlet_session = get_servlet_session(req.env) @@ -122,40 +116,49 @@ def loaded_session?(session) ! session.is_a?(::JRuby::Rack::Session::SessionHash) || session.loaded? end - def commit_session(req, status, headers, body) - session = req.env[::Rack::RACK_SESSION] - options = req.env[::Rack::RACK_SESSION_OPTIONS] + # Overridden from Rack, removing support for deferral and unnecessary cookie support when using Java Servlet sessions. + def commit_session(req, _res) + session = req.get_header ::Rack::RACK_SESSION + options = session.options if options[:drop] || options[:renew] - destroy_session(req.env, options[:id], options) + delete_session(req, session.id, options) end - return [status, headers, body] if options[:drop] || options[:skip] + return if options[:drop] || options[:skip] if loaded_session?(session) - session_id = session.respond_to?(:id=) ? session.id : options[:id] - session_data = session.to_hash.delete_if { |_,v| v.nil? } - unless set_session(req.env, session_id, session_data, options) - req.env["rack.errors"].puts("WARNING #{self.class.name} failed to save session. Content dropped.") + # Mirror behaviour of Rails ActionDispatch::Session::AbstractStore#commit_session for Rails 7.1+ compatibility + commit_csrf_token(req, session) + + session_id ||= session.id + session_data = session.to_hash.delete_if { |k, v| v.nil? } + + unless write_session(req, session_id, session_data, options) + req.get_header(::Rack::RACK_ERRORS).puts("Warning! #{self.class.name} failed to save session. Content dropped.") end end + end - [status, headers, body] + def commit_csrf_token(req, session_hash) + csrf_token = req.env[::ActionController::RequestForgeryProtection::CSRF_TOKEN] if defined?(::ActionController::RequestForgeryProtection::CSRF_TOKEN) + session_hash[:_csrf_token] = csrf_token if csrf_token end - def set_session(env, session_id, hash, options) - if session_id.nil? && hash.empty? - destroy_session(env) + def write_session(req, session_id, session_hash, _options) + if session_id.nil? && session_hash.empty? + delete_session(req) return true end - if servlet_session = get_servlet_session(env, true) + + if servlet_session = get_servlet_session(req.env, true) begin servlet_session.synchronized do keys = servlet_session.getAttributeNames - keys.select { |key| ! hash.has_key?(key) }.each do |key| + keys.select { |key| ! session_hash.has_key?(key) }.each do |key| servlet_session.removeAttribute(key) end - hash.delete_if do |key,value| + session_hash.delete_if do |key,value| if String === key case value when String, Numeric, true, false, nil @@ -171,8 +174,8 @@ def set_session(env, session_id, hash, options) end end end - if ! hash.empty? - marshalled_string = Marshal.dump(hash) + if ! session_hash.empty? + marshalled_string = Marshal.dump(session_hash) marshalled_bytes = marshalled_string.to_java_bytes servlet_session.setAttribute(RAILS_SESSION_KEY, marshalled_bytes) elsif servlet_session.getAttribute(RAILS_SESSION_KEY) @@ -188,9 +191,9 @@ def set_session(env, session_id, hash, options) end end - def destroy_session(env, session_id = nil, options = nil) - # session_id and options arg defaults for destory alias - (session = get_servlet_session(env)) && session.synchronized { session.invalidate } + def delete_session(req, _session_id = nil, _options = nil) + # session_id and options arg defaults for delete alias + (session = get_servlet_session(req.env)) && session.synchronized { session.invalidate } rescue java.lang.IllegalStateException # if session already invalid nil end diff --git a/src/spec/ruby/action_controller/session/java_servlet_store_spec.rb b/src/spec/ruby/action_controller/session/java_servlet_store_spec.rb index 444abf75c..2d67be26d 100644 --- a/src/spec/ruby/action_controller/session/java_servlet_store_spec.rb +++ b/src/spec/ruby/action_controller/session/java_servlet_store_spec.rb @@ -3,21 +3,10 @@ describe "ActionController::Session::JavaServletStore" do before :all do - require 'active_support' require 'action_controller' - begin # help Rails 3.0 up - require 'action_dispatch/middleware/session/abstract_store' - rescue LoadError - end - begin # a Rails 2.3 require - require 'action_controller/session/abstract_store' - rescue LoadError - end - - require 'jruby/rack/session_store' - require 'action_controller/session/java_servlet_store' + require 'jruby/rack/session_store' end before :each do @@ -283,6 +272,22 @@ expect( new_session.send(:getAttribute, "_csrf_token") ).to_not be nil end + it "propagates rails csrf token to session during commit" do + skip "Only runs on Rails 7.1+" unless defined? ::ActionController::RequestForgeryProtection::CSRF_TOKEN + session = double_http_session + @request.should_receive(:getSession).and_return(session) + + @app.should_receive(:call) do |env| + env['rack.session']['foo'] = 'bar' + env[::ActionController::RequestForgeryProtection::CSRF_TOKEN] = 'some_token' + end + @session_store.call(@env) + + # CSRF token propagated from env to underlying session + expect( session.send(:getAttribute, '_csrf_token') ).to eq 'some_token' + expect( session.send(:getAttribute, 'foo') ).to eq 'bar' + end + it "handles the skip session option" do @request.should_receive(:getSession).with(false).and_return @session @session.should_not_receive(:setAttribute)