Commit 415868f0 authored by Stan Hu's avatar Stan Hu

Merge branch 'fix-500-caused-by-array-range' into 'master'

Fix 500 error caused by range much bigger then actual array

See merge request gitlab-org/gitlab!21180
parents cd25eac9 8e40ad18
...@@ -146,8 +146,9 @@ class ActiveSession ...@@ -146,8 +146,9 @@ class ActiveSession
# remove sessions if there are more than ALLOWED_NUMBER_OF_ACTIVE_SESSIONS. # remove sessions if there are more than ALLOWED_NUMBER_OF_ACTIVE_SESSIONS.
sessions = active_session_entries(session_ids, user.id, redis) sessions = active_session_entries(session_ids, user.id, redis)
sessions.sort_by! {|session| session.updated_at }.reverse! sessions.sort_by! {|session| session.updated_at }.reverse!
sessions = sessions[ALLOWED_NUMBER_OF_ACTIVE_SESSIONS..-1].map { |session| session.session_id } sessions = sessions.drop(ALLOWED_NUMBER_OF_ACTIVE_SESSIONS)
destroy_sessions(redis, user, sessions) sessions = sessions.map { |session| session.session_id }
destroy_sessions(redis, user, sessions) if sessions.any?
end end
def self.cleaned_up_lookup_entries(redis, user) def self.cleaned_up_lookup_entries(redis, user)
......
...@@ -329,6 +329,35 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do ...@@ -329,6 +329,35 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do
) )
end end
end end
context 'when the number of active sessions is lower than the limit' do
before do
Gitlab::Redis::SharedState.with do |redis|
((max_number_of_sessions_plus_two - 4)..max_number_of_sessions_plus_two).each do |number|
redis.del("session:user:gitlab:#{user.id}:#{number}")
end
end
end
it 'does not remove active session entries, but removes lookup entries' do
lookup_entries_before_cleanup = Gitlab::Redis::SharedState.with do |redis|
redis.smembers("session:lookup:user:gitlab:#{user.id}")
end
sessions_before_cleanup = Gitlab::Redis::SharedState.with do |redis|
redis.scan_each(match: "session:user:gitlab:#{user.id}:*").to_a
end
ActiveSession.cleanup(user)
Gitlab::Redis::SharedState.with do |redis|
lookup_entries = redis.smembers("session:lookup:user:gitlab:#{user.id}")
sessions = redis.scan_each(match: "session:user:gitlab:#{user.id}:*").to_a
expect(sessions.count).to eq(sessions_before_cleanup.count)
expect(lookup_entries.count).to be < lookup_entries_before_cleanup.count
end
end
end
end end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment