From 6b88cdc56ccccf207437d67bb65e8db5946aea69 Mon Sep 17 00:00:00 2001 From: Aaron Date: Thu, 5 Feb 2009 11:51:51 -0500 Subject: [PATCH] More validation for modifier updates --- db/jsobj.h | 5 +++ db/lasterror.h | 10 ++--- db/query.cpp | 78 ++++++++++++++++++++++++--------------- dbtests/querytests.cpp | 84 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 142 insertions(+), 35 deletions(-) diff --git a/db/jsobj.h b/db/jsobj.h index c133b8db203..cdde8a3ce52 100644 --- a/db/jsobj.h +++ b/db/jsobj.h @@ -247,6 +247,11 @@ namespace mongo { return *((OID*) value()); } + /** True if element is null. */ + bool isNull() const { + return type() == jstNULL; + } + /** Size (length) of a string element. You must assure of type String first. */ int valuestrsize() const { diff --git a/db/lasterror.h b/db/lasterror.h index 10a3f347446..b1ddcb3d162 100644 --- a/db/lasterror.h +++ b/db/lasterror.h @@ -1,8 +1,8 @@ -// lasterror.h - -/** -* Copyright (C) 2009 10gen Inc. -* +// lasterror.h + +/** +* Copyright (C) 2009 10gen Inc. +* * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License, version 3, * as published by the Free Software Foundation. diff --git a/db/query.cpp b/db/query.cpp index 6bda7204166..ad7c293a19f 100644 --- a/db/query.cpp +++ b/db/query.cpp @@ -287,43 +287,59 @@ namespace mongo { BSONObjIterator it(from); while ( it.more() ) { BSONElement e = it.next(); + if ( e.eoo() ) + break; const char *fn = e.fieldName(); - if ( *fn == '$' && e.type() == Object && - fn[4] == 0 ) { - BSONObj j = e.embeddedObject(); - BSONObjIterator jt(j); - Op op = Mod::SET; - if ( strcmp("$inc",fn) == 0 ) { - op = Mod::INC; - // we rename to $SET instead of $set so that on an op like - // { $set: {x:1}, $inc: {y:1} } - // we don't get two "$set" fields which isn't allowed - strcpy((char *) fn, "$SET"); + uassert( "Invalid modifier specified", + *fn == '$' && e.type() == Object && fn[4] == 0 ); + BSONObj j = e.embeddedObject(); + BSONObjIterator jt(j); + Op op = Mod::SET; + if ( strcmp("$inc",fn) == 0 ) { + op = Mod::INC; + // we rename to $SET instead of $set so that on an op like + // { $set: {x:1}, $inc: {y:1} } + // we don't get two "$set" fields which isn't allowed + strcpy((char *) fn, "$SET"); + } else { + uassert( "Invalid modifier specified", strcasecmp("$set",fn ) == 0 ); + } + while ( jt.more() ) { + BSONElement f = jt.next(); + if ( f.eoo() ) + break; + Mod m; + m.op = op; + m.fieldName = f.fieldName(); + uassert( "Mod on _id not allowed", strcmp( m.fieldName, "_id" ) != 0 ); + for ( vector::iterator i = mods.begin(); i != mods.end(); i++ ) { + uassert( "Field name duplication not allowed with modifiers", + strcmp( m.fieldName, i->fieldName ) != 0 ); } - while ( jt.more() ) { - BSONElement f = jt.next(); - if ( f.eoo() ) - break; - Mod m; - m.op = op; - m.fieldName = f.fieldName(); - uassert( "Mod on _id not allowed", strcmp( m.fieldName, "_id" ) != 0 ); - if ( f.isNumber() ) { - if ( f.type() == NumberDouble ) { - m.ndouble = (double *) f.value(); - m.nint = 0; - } - else { - m.ndouble = 0; - m.nint = (int *) f.value(); - } - mods.push_back( m ); - } + uassert( "Modifiers allowed for numbers only", f.isNumber() ); + if ( f.type() == NumberDouble ) { + m.ndouble = (double *) f.value(); + m.nint = 0; } + else { + m.ndouble = 0; + m.nint = (int *) f.value(); + } + mods.push_back( m ); } } } + void checkNoMods( BSONObj o ) { + BSONObjIterator i( o ); + while( i.more() ) { + BSONElement e = i.next(); + if ( e.eoo() ) + break; + massert( "Modifiers and non-modifiers cannot be mixed", e.fieldName()[ 0 ] != '$' ); + } + } + int __updateObjects(const char *ns, BSONObj updateobj, BSONObj &pattern, bool upsert, stringstream& ss, bool logop=false) { int profile = database->profile; @@ -398,6 +414,8 @@ namespace mongo { } } return 2; + } else { + checkNoMods( updateobj ); } theDataFileMgr.update(ns, r, c->currLoc(), updateobj.objdata(), updateobj.objsize(), ss); diff --git a/dbtests/querytests.cpp b/dbtests/querytests.cpp index e3ec60d4d5b..5e4bf417db9 100644 --- a/dbtests/querytests.cpp +++ b/dbtests/querytests.cpp @@ -20,7 +20,9 @@ #include "../db/query.h" #include "../db/db.h" +#include "../db/instance.h" #include "../db/json.h" +#include "../db/lasterror.h" #include "dbtests.h" @@ -170,6 +172,82 @@ namespace QueryTests { } }; + class ClientBase { + public: + // NOTE: Not bothering to backup the old error record. + ClientBase() { + mongo::lastError.reset( new LastError() ); + } + ~ClientBase() { + mongo::lastError.release(); + } + protected: + static void insert( const char *ns, BSONObj o ) { + client_.insert( ns, o ); + } + static void update( const char *ns, BSONObj q, BSONObj o, bool upsert = 0 ) { + client_.update( ns, Query( q ), o, upsert ); + } + static bool error() { + return !client_.getPrevError().getField( "err" ).isNull(); + } + private: + static DBDirectClient client_; + }; + DBDirectClient ClientBase::client_; + + class Fail : public ClientBase { + public: + void run() { + prep(); + ASSERT( !error() ); + doIt(); + ASSERT( error() ); + } + protected: + const char *ns() { return "QueryTests::Fail"; } + virtual void prep() { + insert( ns(), fromjson( "{a:1}" ) ); + } + virtual void doIt() = 0; + }; + + class ModId : public Fail { + void doIt() { + update( ns(), emptyObj, fromjson( "{$set:{'_id':4}}" ) ); + } + }; + + class ModNonmodMix : public Fail { + void doIt() { + update( ns(), emptyObj, fromjson( "{$set:{a:4},z:3}" ) ); + } + }; + + class InvalidMod : public Fail { + void doIt() { + update( ns(), emptyObj, fromjson( "{$awk:{a:4}}" ) ); + } + }; + + class ModNotFirst : public Fail { + void doIt() { + update( ns(), emptyObj, fromjson( "{z:3,$set:{a:4}}" ) ); + } + }; + + class ModDuplicateFieldSpec : public Fail { + void doIt() { + update( ns(), emptyObj, fromjson( "{$set:{a:4},$inc:{a:1}}" ) ); + } + }; + + class ModNonNumber : public Fail { + void doIt() { + update( ns(), emptyObj, fromjson( "{$set:{a:'d'}}" ) ); + } + }; + class All : public UnitTest::Suite { public: All() { @@ -182,6 +260,12 @@ namespace QueryTests { add< CountQuery >(); add< CountFields >(); add< CountQueryFields >(); + add< ModId >(); + add< ModNonmodMix >(); + add< InvalidMod >(); + add< ModNotFirst >(); + add< ModDuplicateFieldSpec >(); + add< ModNonNumber >(); } };