[CXX-832] Friendlier error message when decoding Decimal128 BSONElements Created: 27/Jan/16  Updated: 21/Jun/16  Resolved: 17/Jun/16

Status: Closed
Project: C++ Driver
Component/s: None
Affects Version/s: None
Fix Version/s: legacy-1.1.2

Type: Improvement Priority: Major - P3
Reporter: Rathi Gnanasekaran Assignee: J Rassi
Resolution: Done Votes: 0
Labels: legacy-cxx
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
is depended on by DRIVERS-281 Rephrase unsupported/corrupt BSON mes... Closed

 Description   

I'm kicking this out to legacy-1.1.2. This is a large and complex change given the structure of the BSON library in the legacy driver.



 Comments   
Comment by Githook User [ 17/Jun/16 ]

Author:

{u'username': u'jrassi', u'name': u'J. Rassi', u'email': u'rassi@10gen.com'}

Message: CXX-832 Friendlier error message when decoding Decimal128 BSONElements
Branch: legacy
https://github.com/mongodb/mongo-cxx-driver/commit/1a17e3dd9feba6aa6a84aa61f88b2d24b5573509

Comment by J Rassi [ 16/Jun/16 ]

https://github.com/mongodb/mongo-cxx-driver/pull/499

Comment by Andrew Morrow (Inactive) [ 16/Jun/16 ]

That sounds reasonable. We can always update it later.

Comment by David Golden [ 16/Jun/16 ]

By the time Decimal 128 is available in 3.4, we are supposed to have support for it in the C++11 driver.

Still... how about "Please look for a newer version that supports it" as a way to put it. Doesn't promise, just says to look into it.

Comment by Andrew Morrow (Inactive) [ 16/Jun/16 ]

To what should we prompt them to upgrade? The C++11 driver doesn't yet have decimal...

Comment by David Golden [ 16/Jun/16 ]

The Drivers ticket specifically wants drivers to prompt users to upgrade. I think we should do that with a very targeted patch. So we don't do it again for future types, how about we make the message more like "...does not support BSON type %d. Please upgrade..."?

Comment by J Rassi [ 15/Jun/16 ]

The BSONObj constructors are happy to create objects where the raw data contains invalid elements, as these constructors do not inspect the data itself. BSONElement::size(), however, will throw a MsgAssertionException with exception string "BSONElement: bad type 19" when called on a BSONElement that has been initialized from a buffer containing a decimal element.

In practice, this means that current version of the legacy C++ driver will happily return BSONObjs to clients from methods like DBClientInterface::findOne() / DBClientCursor::next() / DBClientInterface::findAndModify() / etc. that contain decimal values. However, calling any accessor methods on these objects like getField(), operator[](), toString() will cause the above exception to be thrown. Ditto for using a BSONObjIterator to iterate over these objects.

For testing, I have created a modified build of the server where all documents returned by query subsystem are given an extra field with a decimal value. See the patch below, applied on top of server commit 9fd6bc9:

diff --git a/src/mongo/db/query/plan_executor.cpp b/src/mongo/db/query/plan_executor.cpp
index 395e84c..74e642e 100644
--- a/src/mongo/db/query/plan_executor.cpp
+++ b/src/mongo/db/query/plan_executor.cpp
@@ -343,7 +343,22 @@ PlanExecutor::ExecState PlanExecutor::getNext(BSONObj* objOut, RecordId* dlOut)
     ExecState state = getNextImpl(objOut ? &snapshotted : NULL, dlOut);
 
     if (objOut) {
-        *objOut = snapshotted.value();
+        BSONObj getNextObj = snapshotted.value();
+        if (!NamespaceString(_ns).isLocal()) {
+            BSONObjBuilder modifiedObjBuilder;
+            modifiedObjBuilder.append("decimal", Decimal128(1));
+            modifiedObjBuilder.appendElements(getNextObj);
+            BSONObj modifiedObj = modifiedObjBuilder.obj();
+            if (getNextObj.isOwned()) {
+                getNextObj = modifiedObj;
+            } else {
+                char* newObjData = new char[modifiedObj.objsize()];  // Leak.
+                std::memcpy(newObjData, modifiedObj.objdata(), modifiedObj.objsize());
+                getNextObj = BSONObj(newObjData);
+            }
+        }
+
+        *objOut = getNextObj;
     }
 
     return state;

I have run the legacy C++ integration test suite against this build, and observed many tests failing due to this exception being thrown, but no signs of memory corruption or other misbehavior.

As a result, I'd recommend either closing this ticket without any code changes, or creating a pull request for patch like the below to make some of the error messages more friendly. Thoughts?

diff --git a/src/mongo/bson/bsonelement.cpp b/src/mongo/bson/bsonelement.cpp
index e7bed6e..0de58cf 100644
--- a/src/mongo/bson/bsonelement.cpp
+++ b/src/mongo/bson/bsonelement.cpp
@@ -500,6 +500,10 @@ int BSONElement::size(int maxLen) const {
             x = (int)(len1 + 1 + len2 + 1);
         } break;
         default: {
+            massert(0,
+                    "This version of the MongoDB C++ driver does not support the Decimal128 BSON "
+                        "type.  Please upgrade your driver.",
+                    type() != 19);
             StringBuilder ss;
             ss << "BSONElement: bad type " << (int)type();
             std::string msg = ss.str();
@@ -563,6 +567,10 @@ int BSONElement::size() const {
             x = (int)(len1 + 1 + len2 + 1);
         } break;
         default: {
+            massert(0,
+                    "This version of the MongoDB C++ driver does not support the Decimal128 BSON "
+                        "type.  Please upgrade your driver.",
+                    type() != 19);
             StringBuilder ss;
             ss << "BSONElement: bad type " << (int)type();
             std::string msg = ss.str();

Generated at Wed Feb 07 22:00:29 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.