Skip to content

Commit

Permalink
SERVER-7557: fix okForStorage
Browse files Browse the repository at this point in the history
  • Loading branch information
@scotthernandez
scotthernandez committed Jul 23, 2013
1 parent b721568 commit 4211c6a
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 24 deletions.
23 changes: 20 additions & 3 deletions src/mongo/bson/bsonobj.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,25 @@ namespace mongo {
/** performs a cursory check on the object's size only. */
bool isValid() const;

/** @return if the user is a valid user doc
criter: isValid() no . or $ field names
/** @return ok if it can be stored as a valid embedded doc.
* Not valid if any field name:
* - contains a "."
* - starts with "$"
* -- unless it is a dbref ($ref/$id/[$db]/...)
*/
bool okForStorage() const;
inline bool okForStorage() const {
return _okForStorage(false);
}

/** Same as above with the following extra restrictions
* Not valid if:
* - "_id" field is a
* -- Regex
* -- Array
*/
inline bool okForStorageAsRoot() const {
return _okForStorage(true);
}

/** @return true if object is empty -- i.e., {} */
bool isEmpty() const { return objsize() <= 5; }
Expand Down Expand Up @@ -527,6 +542,8 @@ namespace mongo {
if ( !isValid() )
_assertInvalid();
}

bool _okForStorage(bool root) const;
};

std::ostream& operator<<( std::ostream &s, const BSONObj &o );
Expand Down
54 changes: 40 additions & 14 deletions src/mongo/db/jsobj.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -880,27 +880,53 @@ namespace mongo {
return b.obj();
}

bool BSONObj::okForStorage() const {
bool BSONObj::_okForStorage(bool root) const {
BSONObjIterator i( *this );
bool first = true;
while ( i.more() ) {
BSONElement e = i.next();
const char * name = e.fieldName();

if ( strchr( name , '.' ) ||
strchr( name , '$' ) ) {
return
strcmp( name , "$ref" ) == 0 ||
strcmp( name , "$id" ) == 0
;
}
const char* name = e.fieldName();

// Cannot start with "$", unless dbref which must start with ($ref, $id)
if (mongoutils::str::startsWith(name, '$')) {
if ( first &&
// $ref is a collection name and must be a String
mongoutils::str::equals(name, "$ref") && e.type() == String &&
mongoutils::str::equals(i.next().fieldName(), "$id") ) {

first = false;
// keep inspecting fields for optional "$db"
e = i.next();
name = e.fieldName(); // "" if eoo()

// optional $db field must be a String
if (mongoutils::str::equals(name, "$db") && e.type() == String) {
continue; //this element is fine, so continue on to siblings (if any more)
}

// check no regexp for _id (SERVER-9502)
if (mongoutils::str::equals(e.fieldName(), "_id")) {
if (e.type() == RegEx) {
// Can't start with a "$", all other checks are done below (outside if blocks)
if (mongoutils::str::startsWith(name, '$')) {
return false;
}
}
else {
// not an okay, $ prefixed field name.
return false;
}
}

// Do not allow "." in the field name
if (strchr(name, '.')) {
return false;
}

// (SERVER-9502) Do not allow storing an _id field with a RegEx type or
// Array type in a root document
if (root && (e.type() == RegEx || e.type() == Array)
&& mongoutils::str::equals(name,"_id")) {
return false;
}

if ( e.mayEncapsulate() ) {
switch ( e.type() ) {
case Object:
Expand All @@ -915,8 +941,8 @@ namespace mongo {
default:
uassert( 12579, "unhandled cases in BSONObj okForStorage" , 0 );
}

}
first = false;
}
return true;
}
Expand Down
71 changes: 64 additions & 7 deletions src/mongo/dbtests/jsobjtests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1857,25 +1857,82 @@ namespace JsobjTests {
public:

void good( string s ) {
BSONObj o = fromjson( s );
if ( o.okForStorage() )
good( fromjson( s ) );
}

void good( BSONObj o ) {
if ( o.okForStorageAsRoot() )
return;
throw UserException( 12528 , (string)"should be ok for storage:" + s );
throw UserException( 12528 , (string)"should be ok for storage:" + o.toString() );
}

void bad( string s ) {
BSONObj o = fromjson( s );
if ( ! o.okForStorage() )
bad( fromjson( s ) );
}

void bad( BSONObj o ) {
if ( ! o.okForStorageAsRoot() )
return;
throw UserException( 12529 , (string)"should NOT be ok for storage:" + s );
throw UserException( 12529 , (string)"should NOT be ok for storage:" + o.toString() );
}

void run() {
// basic docs are good
good( "{}" );
good( "{x:1}" );
good( "{x:{a:2}}" );

// no dots allowed
bad( "{'x.y':1}" );
bad( "{'x\\.y':1}" );

good( "{x:{a:2}}" );
// Check for $
bad( "{x:{'$a':2}}" );
good( "{'a$b':2}" );
good( "{'a$': {b: 2}}" );
good( "{'a$':2}" );
good( "{'a $ a': 'foo'}" );

// Queries are not ok
bad( "{num: {$gt: 1}}" );
bad( "{_id: {$regex:'test'}}" );
bad( "{$gt: 2}" );
bad( "{a : { oo: [ {$bad:1}, {good:1}] }}");
good( "{a : { oo: [ {'\\\\$good':1}, {good:1}] }}");

// DBRef stuff -- json parser can't handle this yet
good( BSON("a" << BSON("$ref" << "coll" << "$id" << 1)) );
good( BSON("a" << BSON("$ref" << "coll" << "$id" << 1 << "$db" << "a")) );
good( BSON("a" << BSON("$ref" << "coll" << "$id" << 1 << "stuff" << 1)) );
good( BSON("a" << BSON("$ref" << "coll" << "$id" << 1 << "$db" <<
"a" << "stuff" << 1)) );

bad( BSON("a" << BSON("$ref" << 1 << "$id" << 1)) );
bad( BSON("a" << BSON("$ref" << 1 << "$id" << 1 << "$db" << "a")) );
bad( BSON("a" << BSON("$ref" << "coll" << "$id" << 1 << "$db" << 1)) );
bad( BSON("a" << BSON("$ref" << "coll")) );
bad( BSON("a" << BSON("$ref" << "coll" << "$db" << "db")) );
bad( BSON("a" << BSON("$id" << 1)) );
bad( BSON("a" << BSON("$id" << 1 << "$ref" << "coll")) );
bad( BSON("a" << BSON("$ref" << "coll" << "$id" << 1 << "$hater" << 1)) );
bad( BSON("a" << BSON("$ref" << "coll" << "$id" << 1 << "dot.dot" << 1)) );

// _id isn't a RegEx, or Array
good( "{_id: 0}" );
good( "{_id: {a:1, b:1}}" );
good( "{_id: {rx: /a/}}" );
good( "{_id: {rx: {$regex: 'a'}}}" );
bad( "{_id: /a/ }" );
bad( "{_id: /a/, other:1}" );
bad( "{hi:1, _id: /a/ }" );
bad( "{_id: /a/i }" );
bad( "{first:/f/i, _id: /a/i }" );
//Not really a regex type
bad( "{_id: {$regex: 'a'} }" );
bad( "{_id: {$regex: 'a', $options:'i'} }" );
bad( "{_id: [1,2]}" );
bad( "{_id: [1]}" );

}
};

Expand Down

0 comments on commit 4211c6a

Please sign in to comment.