Permalink
Browse files

SERVER-6634 Handle negative and invalid skip values in count command

  • Loading branch information...
1 parent 635aed0 commit 9424ec487448c6b8ef79ac1eb63e515fa0ed09d2 @sverch sverch committed Aug 27, 2012
Showing with 37 additions and 4 deletions.
  1. +5 −0 jstests/count2.js
  2. +0 −2 jstests/sharding/count1.js
  3. +14 −0 src/mongo/db/dbcommands.cpp
  4. +5 −0 src/mongo/db/instance.cpp
  5. +13 −2 src/mongo/s/commands_public.cpp
View
@@ -21,3 +21,8 @@ assert.eq( 50 , t.find( { m : 5 } ).skip( 10 ).limit(20).count() , "J" )
assert.eq( 20 , t.find( { m : 5 } ).skip( 10 ).limit(20).countReturn() , "K" )
assert.eq( 5 , t.find( { m : 5 } ).skip( 45 ).limit(20).countReturn() , "L" )
+
+// Negative skip values should return error
+var negSkipResult = db.runCommand({ count: 't', skip : -2 });
+assert( ! negSkipResult.ok , "negative skip value shouldn't work, n = " + negSkipResult.n );
+assert( negSkipResult.errmsg.length > 0 , "no error msg for negative skip" );
@@ -163,12 +163,10 @@ var badCmdResult = db.runCommand({ count: 'foo', query: { $c: { $abc: 3 }}});
assert( ! badCmdResult.ok , "invalid query syntax didn't return error" );
assert( badCmdResult.errmsg.length > 0 , "no error msg for invalid query" );
-/* uncomment when SERVER-6634 is fixed:
// Negative skip values should return error
var negSkipResult = db.runCommand({ count: 'foo', skip : -2 });
assert( ! negSkipResult.ok , "negative skip value shouldn't work" );
assert( negSkipResult.errmsg.length > 0 , "no error msg for negative skip" );
-*/
// Negative skip values with positive limit should return error
var negSkipLimitResult = db.runCommand({ count: 'foo', skip : -2, limit : 1 });
@@ -798,6 +798,20 @@ namespace mongo {
virtual bool adminOnly() const { return false; }
virtual void help( stringstream& help ) const { help << "count objects in collection"; }
virtual bool run(const string& dbname, BSONObj& cmdObj, int, string& errmsg, BSONObjBuilder& result, bool) {
+
+ long long skip = 0;
+ if ( cmdObj["skip"].isNumber() ) {
+ skip = cmdObj["skip"].numberLong();
+ if ( skip < 0 ) {
+ errmsg = "skip value is negative in count query";
+ return false;
+ }
+ }
+ else if ( cmdObj["skip"].ok() ) {
+ errmsg = "skip value is not a valid number";
+ return false;
+ }
+
string ns = parseNs(dbname, cmdObj);
string err;
int errCode;
@@ -910,6 +910,11 @@ namespace mongo {
HostAndPort DBDirectClient::_clientHost = HostAndPort( "0.0.0.0" , 0 );
unsigned long long DBDirectClient::count(const string &ns, const BSONObj& query, int options, int limit, int skip ) {
+ if ( skip < 0 ) {
+ warning() << "setting negative skip value: " << skip
+ << " to zero in query: " << query << endl;
+ skip = 0;
+ }
Lock::DBRead lk( ns );
string errmsg;
int errCode;
@@ -494,6 +494,19 @@ namespace mongo {
BSONObjBuilder& result,
bool ){
+ long long skip = 0;
+ if( cmdObj["skip"].isNumber() ){
+ skip = cmdObj["skip"].numberLong();
+ if( skip < 0 ){
+ errmsg = "skip value is negative in count query";
+ return false;
+ }
+ }
+ else if( cmdObj["skip"].ok() ){
+ errmsg = "skip value is not a valid number";
+ return false;
+ }
+
const string collection = cmdObj.firstElement().valuestrsafe();
const string fullns = dbName + "." + collection;
@@ -514,8 +527,6 @@ namespace mongo {
* apply it only once we have collected all counts.
*/
if( limit != 0 && cmdObj["skip"].isNumber() ){
- long long skip = cmdObj["skip"].numberLong();
- uassert( 16260 , "skip has to be positive" , skip >= 0 );
if ( limit > 0 )
limit += skip;
else

0 comments on commit 9424ec4

Please sign in to comment.