[SERVER-11903] Remove BSONElement::validate() Created: 29/Nov/13  Updated: 11/Jul/16  Resolved: 03/Jan/14

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: 2.4.8
Fix Version/s: 2.5.5

Type: Bug Priority: Major - P3
Reporter: James Wahlin Assignee: Mathias Stearn
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
is related to SERVER-12260 batch_upconvert_test fails under addr... Closed
Backwards Compatibility: Minor Change
Operating System: ALL
Participants:

 Description   

Original title: BSONElement.validate() success on invalid BSON

There have been cases where BSONElement.validate() fails to recognize that an element is invalid. In the following code in tool.cpp we see the following:

            BSONObj o( buf );
 
>>>  BSONObj.valid() returns false signifying a corrupt BSON object.
            if ( _objcheck && ! o.valid() ) {
                cerr << "INVALID OBJECT - going try and pring out " << endl;
                cerr << "size: " << size << endl;
                BSONObjIterator i(o);
                while ( i.more() ) {
                    BSONElement e = i.next();
                    try {
>>>  e.validate() does not throw an exception and the "NEXT ONE" message is not printed.
                        e.validate();
                    }
                    catch ( ... ) {
                        cerr << "\t\t NEXT ONE IS INVALID" << endl;
                    }
                    cerr << "\t name : " << e.fieldName() << " " << e.type() << endl;
 
>>>  An assertion is triggered on printing "e" (a BSONElement) to standard error. 
                    cerr << "\t " << e << endl;
                }
            }

When we have a corrupt BSONElement, BSONElement.validate() should recognize and throw an exception.



 Comments   
Comment by Githook User [ 06/Jan/14 ]

Author:

{u'username': u'RedBeard0531', u'name': u'Mathias Stearn', u'email': u'mathias@10gen.com'}

Message: SERVER-11903 Fix bug in bson_validate_test support code

Not using the passed-in fieldname invalidated the ErrorIsInId test case.
Luckily it still passes with this fixed.
Branch: master
https://github.com/mongodb/mongo/commit/f2c66a95752628a1c51fd7b35148ff76e0718722

Comment by Githook User [ 06/Jan/14 ]

Author:

{u'username': u'RedBeard0531', u'name': u'Mathias Stearn', u'email': u'mathias@10gen.com'}

Message: SERVER-12260 Don't look for _id field name after EOO

Fixes a bug in implementation of SERVER-11903
Branch: master
https://github.com/mongodb/mongo/commit/8bfce9bcce36c2250f17fb51cadbdabb74754e10

Comment by Mathias Stearn [ 03/Jan/14 ]

Only backwards breaking for C++ driver users calling this function.

Comment by Githook User [ 03/Jan/14 ]

Author:

{u'username': u'RedBeard0531', u'name': u'Mathias Stearn', u'email': u'mathias@10gen.com'}

Message: SERVER-11903 Remove BSONElement::validate()

It was mostly used for improving the error messages about BSON that already
failed validation, but it did not cover all validation cases. To replace this
use case validateBSON was enhanced to include the _id of the invalid object in
the error message.
Branch: master
https://github.com/mongodb/mongo/commit/ab48b396e5bc48cb27ce3bc2dbd28bf3b6a96851

Comment by Mathias Stearn [ 23/Dec/13 ]

Decision: kill BSONElement::validate() as described in my last comment. Will also look over validateBSON error messaging and make sure it provides enough information in the cases where we are using BSONElement::validate() today.

Comment by Mathias Stearn [ 19/Dec/13 ]

There are a total of 5 places in the current codebase that call BSONElement::validate(). Other than in BSONObj::toString() they all follow the pattern of checking BSONObj::valid (which is just a wrapper around validateBSON) then on failure, checking BSONElement::validate() to provide more information in the error message. Those uses should be easy to change, assuming validateBSON can provide the needed information. I think the call in BSONObj::toString() should just be deleted since it doesn't seem necessary.

Generated at Thu Feb 08 03:27:03 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.