Skip to content

Commit

Permalink
Fix RUBY-1988 Ensure cursor IDs are always encoded as 64-bit integers…
Browse files Browse the repository at this point in the history
… in BSON when using killCursors (#1560)
  • Loading branch information
p-mongo authored and p committed Oct 31, 2019
1 parent ffc8ca8 commit ee6301c
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 27 deletions.
21 changes: 16 additions & 5 deletions lib/mongo/cursor/builder/kill_cursors_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ def specification
private

def kill_cursors_command
{ :killCursors => collection_name, :cursors => [ cursor.id ] }
{
killCursors: collection_name,
cursors: [ BSON::Int64.new(cursor.id) ],
}
end

class << self
Expand All @@ -65,23 +68,31 @@ class << self
# KillCursorsCommand.update_cursors(spec, ids)
#
# @return [ Hash ] The specification.
# @return [ Array ] The ids to update with.
# @return [ Array<Integer> ] The ids to update with.
#
# @since 2.3.0
def update_cursors(spec, ids)
spec[:selector].merge!(cursors: spec[:selector][:cursors] & ids)
# Ruby 2.5+ can & BSON::Int64 instances.
# Ruby 2.4 and earlier cannot.
# Convert stored ids to Ruby integers for compatibility with
# older Rubies.
ids = get_cursors_list(spec) & ids
ids = ids.map do |cursor_id|
BSON::Int64.new(cursor_id)
end
spec[:selector].merge!(cursors: ids)
end

# Get the list of cursor ids from a spec generated by this Builder.
#
# @example Get the list of cursor ids.
# KillCursorsCommand.cursors(spec)
#
# @return [ Hash ] The specification.
# @return [ Array<Integer> ] The cursor ids.
#
# @since 2.3.0
def get_cursors_list(spec)
spec[:selector][:cursors]
spec[:selector][:cursors].map(&:value)
end
end
end
Expand Down
22 changes: 17 additions & 5 deletions lib/mongo/cursor/builder/op_kill_cursors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ def initialize(cursor)
#
# @since 2.2.0
def specification
{ :coll_name => collection_name, :db_name => database.name, :cursor_ids => [ cursor.id ] }
{
coll_name: collection_name,
db_name: database.name,
cursor_ids: [ BSON::Int64.new(cursor.id) ],
}
end

class << self
Expand All @@ -59,23 +63,31 @@ class << self
# OpKillCursors.update_cursors(spec, ids)
#
# @return [ Hash ] The specification.
# @return [ Array ] The ids to update with.
# @return [ Array<Integer> ] The ids to update with.
#
# @since 2.3.0
def update_cursors(spec, ids)
spec.merge!(cursor_ids: spec[:cursor_ids] & ids)
# Ruby 2.5+ can & BSON::Int64 instances.
# Ruby 2.4 and earlier cannot.
# Convert stored ids to Ruby integers for compatibility with
# older Rubies.
ids = get_cursors_list(spec) & ids
ids = ids.map do |cursor_id|
BSON::Int64.new(cursor_id)
end
spec.merge!(cursor_ids: ids)
end

# Get the list of cursor ids from a spec generated by this Builder.
#
# @example Get the list of cursor ids.
# OpKillCursors.cursors(spec)
#
# @return [ Hash ] The specification.
# @return [ Array<Integer> ] The cursor ids.
#
# @since 2.3.0
def get_cursors_list(spec)
spec[:cursor_ids]
spec[:cursor_ids].map(&:value)
end
end
end
Expand Down
19 changes: 6 additions & 13 deletions lib/mongo/protocol/kill_cursors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def payload
command_name: 'killCursors',
database_name: @database,
command: upconverter.command,
request_id: request_id
request_id: request_id,
)
end

Expand Down Expand Up @@ -85,16 +85,6 @@ def payload
# @since 2.1.0
class Upconverter

# The kill cursors constant.
#
# @since 2.2.0
KILL_CURSORS = 'killCursors'.freeze

# The cursors constant.
#
# @since 2.2.0
CURSORS = 'cursors'.freeze

# @return [ String ] collection The name of the collection.
attr_reader :collection

Expand Down Expand Up @@ -125,8 +115,11 @@ def initialize(collection, cursor_ids)
# @since 2.1.0
def command
document = BSON::Document.new
document.store(KILL_CURSORS, collection)
document.store(CURSORS, cursor_ids)
document.store('killCursors', collection)
store_ids = cursor_ids.map do |cursor_id|
BSON::Int64.new(cursor_id)
end
document.store('cursors', store_ids)
document
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/integration/cursor_reaping_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
client.cluster.instance_variable_get('@periodic_executor').execute

started_event = EventSubscriber.started_events.detect do |event|
event.command['killCursors'] && event.command['cursors'].include?(cursor_id)
event.command['killCursors'] && event.command['cursors'].map(&:value).include?(cursor_id)
end

expect(started_event).not_to be_nil
Expand Down
6 changes: 3 additions & 3 deletions spec/mongo/cursor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@
Mongo::Collection::View.new(
authorized_collection,
{},
:batch_size => 2
:batch_size => 2,
)
end

Expand All @@ -329,9 +329,9 @@

it 'schedules a kill cursors op' do
cluster.instance_variable_get(:@periodic_executor).flush
expect {
expect do
cursor.to_a
}.to raise_exception(Mongo::Error::OperationFailure, /[cC]ursor.*not found/)
end.to raise_exception(Mongo::Error::OperationFailure, /[cC]ursor.*not found/)
end

context 'when the cursor is unregistered before the kill cursors operations are executed' do
Expand Down

0 comments on commit ee6301c

Please sign in to comment.