Skip to content

Commit 210e12a

Browse files
committed
SERVER-3760 fix memcmp to now potentially go to far on one of the input arrays.
in addition fix a case where compare order of bindata types, when subtype and len, were in consistent with BSONObj's compare. this ordering is somewhat arbitrary but must be consistent.
1 parent b7d7b75 commit 210e12a

File tree

2 files changed

+48
-5
lines changed

2 files changed

+48
-5
lines changed

db/key.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -458,16 +458,21 @@ namespace mongo {
458458
{
459459
int L = *l;
460460
int R = *r;
461+
int llen = binDataCodeToLength(L);
461462
int diff = L-R; // checks length and subtype simultaneously
462-
if( diff )
463+
if( diff ) {
464+
// unfortunately nibbles are backwards to do subtype and len in one check (could bit swap...)
465+
int rlen = binDataCodeToLength(R);
466+
if( llen != rlen )
467+
return llen - rlen;
463468
return diff;
469+
}
464470
// same length, same type
465471
l++; r++;
466-
int len = binDataCodeToLength(L);
467-
int res = memcmp(l, r, len);
472+
int res = memcmp(l, r, llen);
468473
if( res )
469474
return res;
470-
l += len; r += len;
475+
l += llen; r += llen;
471476
break;
472477
}
473478
case cdate:
@@ -618,14 +623,18 @@ namespace mongo {
618623
break;
619624
case cstring:
620625
{
626+
if( *l != *r )
627+
return false; // not same length
621628
unsigned sz = ((unsigned) *l) + 1;
622-
if( memcmp(l, r, sz) ) // first byte checked is the length byte
629+
if( memcmp(l, r, sz) )
623630
return false;
624631
l += sz; r += sz;
625632
break;
626633
}
627634
case cbindata:
628635
{
636+
if( *l != *r )
637+
return false; // len or subtype mismatch
629638
int len = binDataCodeToLength(*l) + 1;
630639
if( memcmp(l, r, len) )
631640
return false;

dbtests/jsobjtests.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,40 @@ namespace JsobjTests {
568568
keyTest( o, true );
569569
}
570570

571+
{
572+
// check (non)equality
573+
BSONObj a = BSONObjBuilder().appendBinData("", 8, (BinDataType) 1, "abcdefgh").obj();
574+
BSONObj b = BSONObjBuilder().appendBinData("", 8, (BinDataType) 1, "abcdefgj").obj();
575+
ASSERT( !a.equal(b) );
576+
int res_ab = a.woCompare(b);
577+
ASSERT( res_ab != 0 );
578+
keyTest( a, true );
579+
keyTest( b, true );
580+
581+
// check subtypes do not equal
582+
BSONObj c = BSONObjBuilder().appendBinData("", 8, (BinDataType) 4, "abcdefgh").obj();
583+
BSONObj d = BSONObjBuilder().appendBinData("", 8, (BinDataType) 0x81, "abcdefgh").obj();
584+
ASSERT( !a.equal(c) );
585+
int res_ac = a.woCompare(c);
586+
ASSERT( res_ac != 0 );
587+
keyTest( c, true );
588+
ASSERT( !a.equal(d) );
589+
int res_ad = a.woCompare(d);
590+
ASSERT( res_ad != 0 );
591+
keyTest( d, true );
592+
593+
KeyV1Owned A(a);
594+
KeyV1Owned B(b);
595+
KeyV1Owned C(c);
596+
KeyV1Owned D(d);
597+
ASSERT( !A.woEqual(B) );
598+
ASSERT( A.woCompare(B, Ordering::make(BSONObj())) == res_ab );
599+
ASSERT( !A.woEqual(C) );
600+
ASSERT( A.woCompare(C, Ordering::make(BSONObj())) < 0 && res_ac < 0 );
601+
ASSERT( !A.woEqual(D) );
602+
ASSERT( A.woCompare(D, Ordering::make(BSONObj())) < 0 && res_ad < 0 );
603+
}
604+
571605
{
572606
BSONObjBuilder b;
573607
b.appendBinData("f", 33, (BinDataType) 1, "123456789012345678901234567890123");

0 commit comments

Comments
 (0)