Skip to content
This repository has been archived by the owner on Feb 4, 2022. It is now read-only.

Commit

Permalink
fix(wire-protocol): don't allow override of slaveOk
Browse files Browse the repository at this point in the history
The wire protocol refactor accidentally allowed the `slaveOk`
bit to be set in layers above the common `command` method. This
change reverts that, and always explicitly sets the value based
on the current read preference.
  • Loading branch information
mbroadst committed Jan 16, 2019
1 parent 94a6066 commit 8fcef69
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 2 deletions.
4 changes: 3 additions & 1 deletion lib/wireprotocol/2_6_support.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,16 @@ class WireProtocol {
const commandOptions = Object.assign(
{
command: true,
slaveOk: readPreference.slaveOk(),
numberToSkip: 0,
numberToReturn: -1,
checkKeys: false
},
options
);

// This value is not overridable
commandOptions.slaveOk = readPreference.slaveOk();

try {
const query = new Query(bson, `${databaseNamespace(ns)}.$cmd`, finalCmd, commandOptions);
pool.write(query, commandOptions, callback);
Expand Down
4 changes: 3 additions & 1 deletion lib/wireprotocol/3_2_support.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,14 +171,16 @@ class WireProtocol {
const commandOptions = Object.assign(
{
command: true,
slaveOk: readPreference.slaveOk(),
numberToSkip: 0,
numberToReturn: -1,
checkKeys: false
},
options
);

// This value is not overridable
commandOptions.slaveOk = readPreference.slaveOk();

try {
const query = new Query(bson, `${databaseNamespace(ns)}.$cmd`, finalCmd, commandOptions);
pool.write(query, commandOptions, callback);
Expand Down
30 changes: 30 additions & 0 deletions test/tests/unit/pool_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const expect = require('chai').expect;
const mock = require('mongodb-mock-server');
const Server = require('../../../lib/topologies/server');
const MongoWriteConcernError = require('../../../lib/error').MongoWriteConcernError;
const sinon = require('sinon');

const test = {};
describe('Pool (unit)', function() {
Expand Down Expand Up @@ -47,4 +48,33 @@ describe('Pool (unit)', function() {

client.connect();
});

it('should not allow overriding `slaveOk` when connected to a mongos', function(done) {
test.server.setMessageHandler(request => {
const doc = request.document;
if (doc.ismaster) {
request.reply(Object.assign({ msg: 'isdbgrid' }, mock.DEFAULT_ISMASTER));
} else if (doc.insert) {
request.reply({ ok: 1 });
}
});

const client = new Server(test.server.address());
client.on('error', done);
client.once('connect', () => {
const poolWrite = sinon.spy(client.s.pool, 'write');

client.insert('fake.ns', { a: 1 }, { slaveOk: true }, err => {
expect(err).to.not.exist;

const query = poolWrite.getCalls()[0].args[0];
expect(query.slaveOk).to.be.false;

client.s.pool.write.restore();
done();
});
});

client.connect();
});
});

0 comments on commit 8fcef69

Please sign in to comment.