mirror of
https://github.com/discourse/discourse.git
synced 2026-08-08 20:18:23 -05:00
DEV: Only write session cookie when contents is changed (#33828)
By default, Rack/Rails will include the session cookie in every response, even if its content hasn't changed. This makes race conditions very likely when multiple requests are made in parallel.
This commit is contained in:
@@ -5,6 +5,13 @@ class ActionDispatch::Session::DiscourseCookieStore < ActionDispatch::Session::C
|
||||
super(app, options)
|
||||
end
|
||||
|
||||
# By default, Rack/Rails will include the session cookie in every response,
|
||||
# even if its content hasn't changed. This makes race conditions very likely when
|
||||
# multiple requests are made in parallel
|
||||
def commit_session?(request, session, options)
|
||||
super(request, session, options) && session_has_changed?(request, session)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def set_cookie(request, session_id, cookie)
|
||||
@@ -16,4 +23,10 @@ class ActionDispatch::Session::DiscourseCookieStore < ActionDispatch::Session::C
|
||||
end
|
||||
cookie_jar(request)[@key] = cookie
|
||||
end
|
||||
|
||||
def session_has_changed?(request, session)
|
||||
_, original_session = load_session(request)
|
||||
new_session = session.to_hash
|
||||
original_session != new_session
|
||||
end
|
||||
end
|
||||
|
||||
@@ -0,0 +1,16 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
describe ActionDispatch::Session::DiscourseCookieStore, type: :request do
|
||||
it "only writes session cookie when changed" do
|
||||
get "/session/csrf.json"
|
||||
expect(response.status).to eq(200)
|
||||
expect(response.cookies["_forum_session"]).to be_present
|
||||
csrf_token = session[:_csrf_token]
|
||||
expect(csrf_token).to be_present
|
||||
|
||||
get "/session/csrf.json"
|
||||
expect(response.status).to eq(200)
|
||||
expect(response.cookies["_forum_session"]).not_to be_present
|
||||
expect(session[:_csrf_token]).to eq(csrf_token)
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user