SERVER-14268: Improve server message parsing

12 files changed, 362 insertions, 148 deletions

diff --git a/src/mongo/db/SConscript b/src/mongo/db/SConscript
index 91e36615208..04a9febc820 100644
--- a/src/mongo/db/SConscript
+++ b/src/mongo/db/SConscript
@@ -56,3 +56,14 @@ env.CppUnitTest(
         '$BUILD_DIR/mongo/mongohasher',
     ],
 )
+
+env.CppUnitTest(
+    target="dbmessage_test",
+    source=[
+        "dbmessage_test.cpp"
+    ],
+    LIBDEPS=[
+        "common",
+        "$BUILD_DIR/mongo/clientdriver",
+    ],
+)
\ No newline at end of file
diff --git a/src/mongo/db/catalog/collection_cursor_cache.cpp b/src/mongo/db/catalog/collection_cursor_cache.cpp
index d358e7a4a52..2822117c9c7 100644
--- a/src/mongo/db/catalog/collection_cursor_cache.cpp
+++ b/src/mongo/db/catalog/collection_cursor_cache.cpp
@@ -236,11 +236,13 @@ namespace mongo {
 
     // ---
 
-    std::size_t CollectionCursorCache::timeoutCursorsGlobal(OperationContext* txn, int millisSinceLastCall) {;
+    std::size_t CollectionCursorCache::timeoutCursorsGlobal(OperationContext* txn, 
+        int millisSinceLastCall) {;
     return _globalCursorIdCache.timeoutCursors(txn, millisSinceLastCall);
     }
 
-    int CollectionCursorCache::eraseCursorGlobalIfAuthorized(OperationContext* txn, int n, long long* ids) {
+    int CollectionCursorCache::eraseCursorGlobalIfAuthorized(OperationContext* txn, int n, 
+        const long long* ids) {
         int numDeleted = 0;
         for ( int i = 0; i < n; i++ ) {
             if ( eraseCursorGlobalIfAuthorized(txn, ids[i] ) )
@@ -377,7 +379,8 @@ namespace mongo {
                 toDelete.push_back( cc );
         }
 
-        for ( vector<ClientCursor*>::const_iterator i = toDelete.begin(); i != toDelete.end(); ++i ) {
+        for ( vector<ClientCursor*>::const_iterator i = toDelete.begin();
+                i != toDelete.end(); ++i ) {
             ClientCursor* cc = *i;
             _deregisterCursor_inlock( cc );
             cc->kill();
diff --git a/src/mongo/db/catalog/collection_cursor_cache.h b/src/mongo/db/catalog/collection_cursor_cache.h
index 524f1e12e11..2015b7049d3 100644
--- a/src/mongo/db/catalog/collection_cursor_cache.h
+++ b/src/mongo/db/catalog/collection_cursor_cache.h
@@ -110,7 +110,8 @@ namespace mongo {
 
         // ----------------------
 
-        static int eraseCursorGlobalIfAuthorized(OperationContext* txn, int n, long long* ids);
+        static int eraseCursorGlobalIfAuthorized(OperationContext* txn, int n, 
+            const long long* ids);
         static bool eraseCursorGlobalIfAuthorized(OperationContext* txn, CursorId id);
 
         static bool eraseCursorGlobal(OperationContext* txn, CursorId id);
diff --git a/src/mongo/db/dbmessage.cpp b/src/mongo/db/dbmessage.cpp
index 4e951cc5e78..a3b06058547 100644
--- a/src/mongo/db/dbmessage.cpp
+++ b/src/mongo/db/dbmessage.cpp
@@ -65,6 +65,107 @@ namespace mongo {
         return ss.str();
     }
 
+    DbMessage::DbMessage(const Message& msg) : _msg(msg), _nsStart(NULL), _mark(NULL), _nsLen(0) {
+        // for received messages, Message has only one buffer
+        _theEnd = _msg.singleData()->_data + _msg.singleData()->dataLen();
+        _nextjsobj = _msg.singleData()->_data;
+
+        _reserved = readAndAdvance<int>();
+
+        // Read packet for NS
+        if (messageShouldHaveNs()) {
+
+            // Limit = buffer size of message -
+            //        (first int4 in message which is either flags or a zero constant)
+            size_t limit = _msg.singleData()->dataLen() - sizeof(int);
+
+            _nsStart = _nextjsobj;
+            _nsLen = strnlen(_nsStart, limit);
+
+            // Validate there is room for a null byte in the buffer
+            // Strings can be zero length
+            uassert(18633, "Failed to parse ns string", _nsLen <= (limit - 1));
+
+            _nextjsobj += _nsLen + 1; // skip namespace + null
+        }
+    }
+
+    const char * DbMessage::getns() const {
+        verify(messageShouldHaveNs());
+        return _nsStart;
+    }
+
+    int DbMessage::getQueryNToReturn() const {
+        verify(messageShouldHaveNs());
+        const char* p = _nsStart + _nsLen + 1;
+        checkRead<int>(p, 2);
+
+        return ((reinterpret_cast<const int*>(p)))[1];
+    }
+
+    int DbMessage::pullInt() {
+        return readAndAdvance<int>();
+    }
+
+    long long DbMessage::pullInt64() {
+        return readAndAdvance<long long>();
+    }
+
+    const long long* DbMessage::getArray(size_t count) const {
+        checkRead<long long>(_nextjsobj, count);
+        return reinterpret_cast<const long long*>(_nextjsobj);
+    }
+
+    BSONObj DbMessage::nextJsObj() {
+        massert(10304,
+            "Client Error: Remaining data too small for BSON object",
+            _nextjsobj != NULL && _theEnd - _nextjsobj >= 5);
+
+        if (serverGlobalParams.objcheck) {
+            Status status = validateBSON(_nextjsobj, _theEnd - _nextjsobj);
+            massert(10307,
+                str::stream() << "Client Error: bad object in message: " << status.reason(),
+                status.isOK());
+        }
+
+        BSONObj js(_nextjsobj);
+        verify(js.objsize() >= 5);
+        verify(js.objsize() <= (_theEnd - _nextjsobj));
+
+        _nextjsobj += js.objsize();
+        if (_nextjsobj >= _theEnd)
+            _nextjsobj = NULL;
+        return js;
+    }
+
+    void DbMessage::markReset(const char * toMark = NULL) {
+        if (toMark == NULL) {
+            toMark = _mark;
+        }
+
+        verify(toMark);
+        _nextjsobj = toMark;
+    }
+
+    template<typename T>
+    void DbMessage::checkRead(const char* start, size_t count) const {
+        if ((_theEnd - start) < static_cast<int>(sizeof(T) * count)) {
+            uassert(18634, "Not enough data to read", false);
+        }
+    }
+
+    template<typename T>
+    T DbMessage::read() const {
+        checkRead<T>(_nextjsobj, 1);
+
+        return *(reinterpret_cast<const T*>(_nextjsobj));
+    }
+
+    template<typename T> T DbMessage::readAndAdvance() {
+        T t = read<T>();
+        _nextjsobj += sizeof(T);
+        return t;
+    }
 
     void replyToQuery(int queryResultFlags,
                       AbstractMessagingPort* p, Message& requestMsg,
diff --git a/src/mongo/db/dbmessage.h b/src/mongo/db/dbmessage.h
index 621624dd9be..5131e2d30a2 100644
--- a/src/mongo/db/dbmessage.h
+++ b/src/mongo/db/dbmessage.h
@@ -122,135 +122,74 @@ namespace mongo {
        See http://dochub.mongodb.org/core/mongowireprotocol
     */
     class DbMessage {
+    // Assume sizeof(int) == 4 bytes
+    BOOST_STATIC_ASSERT(sizeof(int) == 4);
+
     public:
-        DbMessage(const Message& _m) : m(_m) , mark(0) {
-            // for received messages, Message has only one buffer
-            theEnd = _m.singleData()->_data + _m.header()->dataLen();
-            char *r = _m.singleData()->_data;
-            reserved = (int *) r;
-            data = r + 4;
-            nextjsobj = data;
+        // Note: DbMessage constructor reads the first 4 bytes and stores it in reserved
+        DbMessage(const Message& msg);
+
+        // Indicates whether this message is expected to have a ns
+        // or in the case of dbMsg, a string in the same place as ns
+        bool messageShouldHaveNs() const {
+            return (_msg.operation() >= dbMsg) & (_msg.operation() <= dbDelete);
         }
 
-        /** the 32 bit field before the ns 
+        /** the 32 bit field before the ns
          * track all bit usage here as its cross op
          * 0: InsertOption_ContinueOnError
          * 1: fromWriteback
          */
-        int& reservedField() { return *reserved; }
-
-        const char * getns() const {
-            return data;
-        }
+        int reservedField() const { return _reserved; }
 
-        const char * afterNS() const {
-            return data + strlen( data ) + 1;
-        }
-
-        int getInt( int num ) const {
-            const int * foo = (const int*)afterNS();
-            return foo[num];
-        }
+        const char * getns() const;
+        int getQueryNToReturn() const;
 
-        int getQueryNToReturn() const {
-            return getInt( 1 );
-        }
+        int pullInt();
+        long long pullInt64();
+        const long long* getArray(size_t count) const;
 
-        /**
-         * get an int64 at specified offsetBytes after ns
-         */
-        long long getInt64( int offsetBytes ) const {
-            const char * x = afterNS();
-            x += offsetBytes;
-            const long long * ll = (const long long*)x;
-            return ll[0];
+        /* for insert and update msgs */
+        bool moreJSObjs() const {
+            return _nextjsobj != 0;
         }
 
-        void resetPull() { nextjsobj = data; }
-        int pullInt() const { return pullInt(); }
-        int& pullInt() {
-            if ( nextjsobj == data )
-                nextjsobj += strlen(data) + 1; // skip namespace
-            int& i = *((int *)nextjsobj);
-            nextjsobj += 4;
-            return i;
-        }
-        long long pullInt64() const {
-            return pullInt64();
-        }
-        long long &pullInt64() {
-            if ( nextjsobj == data )
-                nextjsobj += strlen(data) + 1; // skip namespace
-            long long &i = *((long long *)nextjsobj);
-            nextjsobj += 8;
-            return i;
-        }
+        BSONObj nextJsObj();
 
-        OID* getOID() const {
-            return (OID *) (data + strlen(data) + 1); // skip namespace
-        }
+        const Message& msg() const { return _msg; }
 
-        void getQueryStuff(const char *&query, int& ntoreturn) {
-            int *i = (int *) (data + strlen(data) + 1);
-            ntoreturn = *i;
-            i++;
-            query = (const char *) i;
+        const char * markGet() const {
+            return _nextjsobj;
         }
 
-        /* for insert and update msgs */
-        bool moreJSObjs() const {
-            return nextjsobj != 0;
+        void markSet() {
+            _mark = _nextjsobj;
         }
-        BSONObj nextJsObj() {
-            if ( nextjsobj == data ) {
-                nextjsobj += strlen(data) + 1; // skip namespace
-                massert( 13066 ,  "Message contains no documents", theEnd > nextjsobj );
-            }
-            massert( 10304,
-                     "Client Error: Remaining data too small for BSON object",
-                     theEnd - nextjsobj >= 5 );
-
-            if (serverGlobalParams.objcheck) {
-                Status status = validateBSON( nextjsobj, theEnd - nextjsobj );
-                massert( 10307,
-                         str::stream() << "Client Error: bad object in message: " << status.reason(),
-                         status.isOK() );
-            }
 
-            BSONObj js(nextjsobj);
-            verify( js.objsize() >= 5 );
-            verify( js.objsize() < ( theEnd - data ) );
+        void markReset(const char * toMark);
 
-            nextjsobj += js.objsize();
-            if ( nextjsobj >= theEnd )
-                nextjsobj = 0;
-            return js;
-        }
+    private:
+        // Check if we have enough data to read
+        template<typename T>
+        void checkRead(const char* start, size_t count = 0) const;
 
-        const Message& msg() const { return m; }
+        // Read some type without advancing our position
+        template<typename T>
+        T read() const;
 
-        const char * markGet() {
-            return nextjsobj;
-        }
+        // Read some type, and advance our position
+        template<typename T> T readAndAdvance();
 
-        void markSet() {
-            mark = nextjsobj;
-        }
+        const Message& _msg;
+        int _reserved; // flags or zero depending on packet, starts the packet
 
-        void markReset( const char * toMark = 0) {
-            if( toMark == 0 ) toMark = mark;
-            verify( toMark );
-            nextjsobj = toMark;
-        }
+        const char* _nsStart; // start of namespace string, +4 from message start
+        const char* _nextjsobj; // current position reading packet
+        const char* _theEnd; // end of packet
 
-    private:
-        const Message& m;
-        int* reserved;
-        const char *data;
-        const char *nextjsobj;
-        const char *theEnd;
+        const char* _mark;
 
-        const char * mark;
+        unsigned int _nsLen;
     };
 
 
diff --git a/src/mongo/db/dbmessage_test.cpp b/src/mongo/db/dbmessage_test.cpp
new file mode 100644
index 00000000000..867a52d9885
--- /dev/null
+++ b/src/mongo/db/dbmessage_test.cpp
@@ -0,0 +1,143 @@
+/**
+ * Copyright (C) 2014 MongoDB 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * As a special exception, the copyright holders give permission to link the
+ * code of portions of this program with the OpenSSL library under certain
+ * conditions as described in each individual source file and distribute
+ * linked combinations including the program with the OpenSSL library. You
+ * must comply with the GNU Affero General Public License in all respects
+ * for all of the code used other than as permitted herein. If you modify
+ * file(s) with this exception, you may extend this exception to your
+ * version of the file(s), but you are not obligated to do so. If you do not
+ * wish to do so, delete this exception statement from your version. If you
+ * delete this exception statement from all source files in the program,
+ * then also delete it in the license file.
+ */
+
+#include <string>
+
+#include "mongo/bson/util/builder.h"
+#include "mongo/db/dbmessage.h"
+#include "mongo/unittest/unittest.h"
+
+namespace mongo {
+    using std::string;
+
+    // Test if the reserved field is short of 4 bytes
+    TEST(DBMessage1, ShortFlags) {
+        BufBuilder b;
+        string ns("test");
+
+        b.appendChar( 1 );
+
+        Message toSend;
+        toSend.setData( dbDelete , b.buf() , b.len() );
+
+        ASSERT_THROWS(DbMessage d1(toSend), UserException);
+    }
+
+    // Test a short NS missing a trailing null
+    TEST(DBMessage1, BadNS) {
+        BufBuilder b;
+
+        b.appendNum( static_cast<int>(1) );
+        b.appendChar( 'b' );
+        b.appendChar( 'a' );
+        b.appendChar( 'd' );
+        // Forget to append \0
+
+        Message toSend;
+        toSend.setData( dbDelete , b.buf() , b.len() );
+
+        ASSERT_THROWS(DbMessage d1(toSend), UserException);
+    }
+
+    // Test a valid kill message and try an extra pull
+    TEST(DBMessage1, GoodKill) {
+        BufBuilder b;
+
+        b.appendNum( static_cast<int>(1) );
+        b.appendNum( static_cast<int>(3) );
+
+        Message toSend;
+        toSend.setData( dbKillCursors , b.buf() , b.len() );
+
+        DbMessage d1(toSend);
+        ASSERT_EQUALS(3, d1.pullInt());
+
+        ASSERT_THROWS(d1.pullInt(), UserException);
+    }
+
+    // Try a bad read of a type too large
+    TEST(DBMessage1, GoodKill2) {
+        BufBuilder b;
+
+        b.appendNum( static_cast<int>(1) );
+        b.appendNum( static_cast<int>(3) );
+
+        Message toSend;
+        toSend.setData( dbKillCursors , b.buf() , b.len() );
+
+        DbMessage d1(toSend);
+        ASSERT_THROWS(d1.pullInt64(), UserException);
+    }
+
+    // Test a basic good insert, and an extra read
+    TEST(DBMessage1, GoodInsert) {
+        BufBuilder b;
+        string ns("test");
+
+        b.appendNum( static_cast<int>(1) );
+        b.appendStr(ns);
+        b.appendNum( static_cast<int>(3) );
+        b.appendNum( static_cast<int>(39) );
+
+        Message toSend;
+        toSend.setData( dbInsert , b.buf() , b.len() );
+
+        DbMessage d1(toSend);
+        ASSERT_EQUALS(3, d1.pullInt());
+        ASSERT_EQUALS(39, d1.pullInt());
+        ASSERT_THROWS(d1.pullInt(), UserException);
+    }
+
+    // Test a basic good insert, and an extra read
+    TEST(DBMessage1, GoodInsert2) {
+        BufBuilder b;
+        string ns("test");
+
+        b.appendNum( static_cast<int>(1) );
+        b.appendStr(ns);
+        b.appendNum( static_cast<int>(3) );
+        b.appendNum( static_cast<int>(39) );
+
+        BSONObj bo = BSON( "ts" << 0 );
+        bo.appendSelfToBufBuilder( b );
+
+        Message toSend;
+        toSend.setData( dbInsert , b.buf() , b.len() );
+
+        DbMessage d1(toSend);
+        ASSERT_EQUALS(3, d1.pullInt());
+
+
+        ASSERT_EQUALS(39, d1.pullInt());
+        BSONObj bo2 = d1.nextJsObj();
+        ASSERT_THROWS(d1.nextJsObj(), MsgAssertionException);
+    }
+
+
+
+} // mongo namespace
diff --git a/src/mongo/db/instance.cpp b/src/mongo/db/instance.cpp
index 7534a1a8f82..695f081880f 100644
--- a/src/mongo/db/instance.cpp
+++ b/src/mongo/db/instance.cpp
@@ -330,7 +330,8 @@ namespace mongo {
         // before we lock...
         int op = m.operation();
         bool isCommand = false;
-        const char *ns = m.singleData()->_data + 4;
+
+        DbMessage dbmsg(m);
 
         Client& c = cc();
         if (!c.isGod()) {
@@ -341,7 +342,9 @@ namespace mongo {
         }
 
         if ( op == dbQuery ) {
-            if( strstr(ns, ".$cmd") ) {
+            const char *ns = dbmsg.getns();
+
+            if (strstr(ns, ".$cmd")) {
                 isCommand = true;
                 opwrite(m);
                 if( strstr(ns, ".$cmd.sys.") ) {
@@ -426,7 +429,8 @@ namespace mongo {
         }
         else if ( op == dbMsg ) {
             // deprecated - replaced by commands
-            char *p = m.singleData()->_data;
+            const char *p = dbmsg.getns();
+
             int len = strlen(p);
             if ( len > 400 )
                 log() << curTimeMillis64() % 10000 <<
@@ -443,8 +447,6 @@ namespace mongo {
         }
         else {
             try {
-                const NamespaceString nsString( ns );
-
                 // The following operations all require authorization.
                 // dbInsert, dbUpdate and dbDelete can be easily pre-authorized,
                 // here, but dbKillCursors cannot.
@@ -453,25 +455,32 @@ namespace mongo {
                     logThreshold = 10;
                     receivedKillCursors(txn, m);
                 }
-                else if ( !nsString.isValid() ) {
-                    // Only killCursors doesn't care about namespaces
-                    uassert( 16257, str::stream() << "Invalid ns [" << ns << "]", false );
-                }
-                else if ( op == dbInsert ) {
-                    receivedInsert(txn, m, currentOp);
-                }
-                else if ( op == dbUpdate ) {
-                    receivedUpdate(txn, m, currentOp);
-                }
-                else if ( op == dbDelete ) {
-                    receivedDelete(txn, m, currentOp);
-                }
-                else {
+                else if (op != dbInsert && op != dbUpdate && op != dbDelete) {
                     mongo::log() << "    operation isn't supported: " << op << endl;
                     currentOp.done();
                     shouldLog = true;
                 }
-            }
+                else {
+                    const char* ns = dbmsg.getns();
+                    const NamespaceString nsString(ns);
+
+                    if (!nsString.isValid()) {
+                        uassert(16257, str::stream() << "Invalid ns [" << ns << "]", false);
+                    }
+                    else if (op == dbInsert) {
+                        receivedInsert(txn, m, currentOp);
+                    }
+                    else if (op == dbUpdate) {
+                        receivedUpdate(txn, m, currentOp);
+                    }
+                    else if (op == dbDelete) {
+                        receivedDelete(txn, m, currentOp);
+                    }
+                    else {
+                        invariant(false);
+                    }
+                }
+             }
             catch (const UserException& ue) {
                 setLastError(ue.getCode(), ue.getInfo().msg.c_str());
                 LOG(3) << " Caught Assertion in " << opToString(op) << ", continuing "
@@ -514,9 +523,8 @@ namespace mongo {
     } /* assembleResponse() */
 
     void receivedKillCursors(OperationContext* txn, Message& m) {
-        int *x = (int *) m.singleData()->_data;
-        x++; // reserved
-        int n = *x++;
+        DbMessage dbmessage(m);
+        int n = dbmessage.pullInt();
 
         uassert( 13659 , "sent 0 cursors to kill" , n != 0 );
         massert( 13658 , str::stream() << "bad kill cursors size: " << m.dataSize() , m.dataSize() == 8 + ( 8 * n ) );
@@ -527,7 +535,9 @@ namespace mongo {
             verify( n < 30000 );
         }
 
-        int found = CollectionCursorCache::eraseCursorGlobalIfAuthorized(txn, n, (long long *) x);
+        const long long* cursorArray = dbmessage.getArray(n);
+
+        int found = CollectionCursorCache::eraseCursorGlobalIfAuthorized(txn, n, cursorArray);
 
         if ( logger::globalLogDomain()->shouldLog(logger::LogSeverity::Debug(1)) || found != n ) {
             LOG( found == n ? 1 : 0 ) << "killcursors: found " << found << " of " << n << endl;
diff --git a/src/mongo/s/SConscript b/src/mongo/s/SConscript
index 52907e1951a..1514261b5f2 100644
--- a/src/mongo/s/SConscript
+++ b/src/mongo/s/SConscript
@@ -315,6 +315,7 @@ env.CppUnitTest(
         'cluster_write_op_conversion',
         '$BUILD_DIR/mongo/db/common',
         '$BUILD_DIR/mongo/server_options', # DbMessage needs server options
+        "$BUILD_DIR/mongo/clientdriver",
     ]
 )
 
@@ -328,6 +329,7 @@ env.CppUnitTest(
         'cluster_write_op_conversion',
         '$BUILD_DIR/mongo/db/common',
         '$BUILD_DIR/mongo/server_options', # DbMessage needs server options
+        "$BUILD_DIR/mongo/clientdriver",
     ]
 )
 
diff --git a/src/mongo/s/cursors.cpp b/src/mongo/s/cursors.cpp
index b33f3ad6fc2..6f2b588e3fd 100644
--- a/src/mongo/s/cursors.cpp
+++ b/src/mongo/s/cursors.cpp
@@ -333,19 +333,20 @@ namespace mongo {
     }
 
     void CursorCache::gotKillCursors(Message& m ) {
-        int *x = (int *) m.singleData()->_data;
-        x++; // reserved
-        int n = *x++;
+        DbMessage dbmessage(m);
+        int n = dbmessage.pullInt();
 
         if ( n > 2000 ) {
             ( n < 30000 ? warning() : error() ) << "receivedKillCursors, n=" << n << endl;
         }
 
-
         uassert( 13286 , "sent 0 cursors to kill" , n >= 1 );
         uassert( 13287 , "too many cursors to kill" , n < 30000 );
+        massert( 18632 , str::stream() << "bad kill cursors size: " << m.dataSize(), 
+                    m.dataSize() == 8 + ( 8 * n ) );
+
 
-        long long * cursors = (long long *)x;
+        const long long* cursors = dbmessage.getArray(n);
         ClientBasic* client = ClientBasic::getCurrent();
         AuthorizationSession* authSession = client->getAuthorizationSession();
         for ( int i=0; i<n; i++ ) {
diff --git a/src/mongo/s/request.cpp b/src/mongo/s/request.cpp
index 984bfdfabe9..f3d09e191f9 100644
--- a/src/mongo/s/request.cpp
+++ b/src/mongo/s/request.cpp
@@ -53,7 +53,6 @@ namespace mongo {
     Request::Request( Message& m, AbstractMessagingPort* p ) :
         _m(m) , _d( m ) , _p(p) , _didInit(false) {
 
-        verify( _d.getns() );
         _id = _m.header()->id;
 
         _txn.reset(new OperationContextNoop());
@@ -77,16 +76,16 @@ namespace mongo {
 
     // Deprecated, will move to the strategy itself
     void Request::reset() {
-        if ( _m.operation() == dbKillCursors ) {
+        _m.header()->id = _id;
+        _clientInfo->clearRequestInfo();
+
+        if ( !_d.messageShouldHaveNs()) {
             return;
         }
 
         uassert( 13644 , "can't use 'local' database through mongos" , ! str::startsWith( getns() , "local." ) );
 
         grid.getDBConfig( getns() );
-
-        _m.header()->id = _id;
-        _clientInfo->clearRequestInfo();
     }
 
     void Request::process( int attempt ) {
diff --git a/src/mongo/tools/sniffer.cpp b/src/mongo/tools/sniffer.cpp
index 75fa6f5eee1..9e39a3b1d15 100644
--- a/src/mongo/tools/sniffer.cpp
+++ b/src/mongo/tools/sniffer.cpp
@@ -338,9 +338,7 @@ void processMessage( Connection& c , Message& m ) {
             break;
         }
         case mongo::dbKillCursors: {
-            int *x = (int *) m.singleData()->_data;
-            x++; // reserved
-            int n = *x;
+            int n = d.pullInt();
             out() << "\tkillCursors n: " << n << endl;
             break;
         }
@@ -366,7 +364,7 @@ void processMessage( Connection& c , Message& m ) {
                 if ( m.operation() == mongo::dbGetMore ) {
                     DbMessage d( m );
                     d.pullInt();
-                    long long &cId = d.pullInt64();
+                    long long cId = d.pullInt64();
                     cId = mapCursor[ c ][ cId ];
                 }
                 Message response;
diff --git a/src/mongo/util/net/message.h b/src/mongo/util/net/message.h
index 154242f4fd5..80205e1d1f5 100644
--- a/src/mongo/util/net/message.h
+++ b/src/mongo/util/net/message.h
@@ -120,13 +120,18 @@ namespace mongo {
 
 #pragma pack(1)
     /* todo merge this with MSGHEADER (or inherit from it). */
-    struct MsgData {
+    class MsgData {
+        friend class Message;
+        friend class DbMessage;
+        friend class MessagingPort;
+    public:
         int len; /* len of the msg, including this field */
         MSGID id; /* request/reply id's match... */
         MSGID responseTo; /* id of the message we are responding to */
         short _operation;
         char _flags;
         char _version;
+
         int operation() const {
             return _operation;
         }
@@ -135,7 +140,6 @@ namespace mongo {
             _version = 0;
             _operation = o;
         }
-        char _data[4];
 
         int& dataAsInt() {
             return *((int *) _data);
@@ -157,6 +161,8 @@ namespace mongo {
         }
 
         int dataLen(); // len without header
+    private:
+        char _data[4]; //must be last member
     };
     const int MsgDataHeaderSize = sizeof(MsgData) - 4;
     inline int MsgData::dataLen() {