SERVER-13007 added delimiters to plan cache key

2 files changed, 100 insertions, 20 deletions

diff --git a/src/mongo/db/query/canonical_query.cpp b/src/mongo/db/query/canonical_query.cpp
index 59273b79b2c..0f8531834e9 100644
--- a/src/mongo/db/query/canonical_query.cpp
+++ b/src/mongo/db/query/canonical_query.cpp
@@ -39,6 +39,35 @@ namespace {
     using std::string;
     using namespace mongo;
 
+    // Delimiters for cache key encoding.
+    const char kEncodeChildrenBegin = '[';
+    const char kEncodeChildrenEnd = ']';
+    const char kEncodeChildrenSeparator = ',';
+    const char kEncodeSortSection = '~';
+    const char kEncodeProjectionSection = '|';
+
+    /**
+     * Encode user-provided string. Cache key delimiters seen in the
+     * user string are escaped with a backslash.
+     */
+    void encodeUserString(const StringData& s, mongoutils::str::stream* os) {
+        for (size_t i = 0; i < s.size(); ++i) {
+            char c = s[i];
+            switch (c) {
+            case kEncodeChildrenBegin:
+            case kEncodeChildrenEnd:
+            case kEncodeChildrenSeparator:
+            case kEncodeSortSection:
+            case kEncodeProjectionSection:
+            case '\\':
+                  *os << '\\';
+                // Fall through to default case.
+            default:
+                *os << c;
+            }
+        }
+    }
+
     void encodePlanCacheKeyTree(const MatchExpression* tree, mongoutils::str::stream* os);
 
     /**
@@ -179,7 +208,9 @@ namespace {
      */
     void encodePlanCacheKeyTree(const MatchExpression* tree, mongoutils::str::stream* os) {
         // Encode match type and path.
-        *os << encodeMatchType(tree->matchType()) << tree->path();
+        *os << encodeMatchType(tree->matchType());
+
+        encodeUserString(tree->path(), os);
 
         // GEO and GEO_NEAR require additional encoding.
         if (MatchExpression::GEO == tree->matchType()) {
@@ -190,9 +221,20 @@ namespace {
         }
 
         // Traverse child nodes.
+        // Enclose children in [].
+        if (tree->numChildren() > 0) {
+            *os << kEncodeChildrenBegin;
+        }
+        // Use comma to separate children encoding.
         for (size_t i = 0; i < tree->numChildren(); ++i) {
+            if (i > 0) {
+                *os << kEncodeChildrenSeparator;
+            }
             encodePlanCacheKeyTree(tree->getChild(i), os);
         }
+        if (tree->numChildren() > 0) {
+            *os << kEncodeChildrenEnd;
+        }
     }
 
     /**
@@ -201,6 +243,12 @@ namespace {
      * LiteParsedQuery.
      */
     void encodePlanCacheKeySort(const BSONObj& sortObj, mongoutils::str::stream* os) {
+        if (sortObj.isEmpty()) {
+            return;
+        }
+
+        *os << kEncodeSortSection;
+
         BSONObjIterator it(sortObj);
         while (it.more()) {
             BSONElement elt = it.next();
@@ -216,7 +264,7 @@ namespace {
             else {
                 *os << "d";
             }
-            *os << elt.fieldName();
+            encodeUserString(elt.fieldName(), os);
         }
     }
 
@@ -232,7 +280,7 @@ namespace {
             return;
         }
 
-        *os << "p";
+        *os << kEncodeProjectionSection;
 
         // Sorts the BSON elements by field name using a map.
         std::map<StringData, BSONElement> elements;
@@ -251,8 +299,8 @@ namespace {
             // BSONElement::toString() arguments
             // includeFieldName - skip field name (appending after toString() result). false.
             // full: choose less verbose representation of child/data values. false.
-            *os << elt.toString(false, false);
-            *os << elt.fieldName();
+            encodeUserString(elt.toString(false, false), os);
+            encodeUserString(elt.fieldName(), os);
         }
     }
 
diff --git a/src/mongo/db/query/canonical_query_test.cpp b/src/mongo/db/query/canonical_query_test.cpp
index 2df7ee0a67b..7dc313411db 100644
--- a/src/mongo/db/query/canonical_query_test.cpp
+++ b/src/mongo/db/query/canonical_query_test.cpp
@@ -475,28 +475,60 @@ namespace {
 
     TEST(PlanCacheTest, GetPlanCacheKey) {
         // Generated cache keys should be treated as opaque to the user.
+
         // No sorts
         testGetPlanCacheKey("{}", "{}", "{}", "an");
-        testGetPlanCacheKey("{$or: [{a: 1}, {b: 2}]}", "{}", "{}", "oreqaeqb");
+        testGetPlanCacheKey("{$or: [{a: 1}, {b: 2}]}", "{}", "{}", "or[eqa,eqb]");
+        testGetPlanCacheKey("{$or: [{a: 1}, {b: 1}, {c: 1}], d: 1}", "{}", "{}",
+                            "an[or[eqa,eqb,eqc],eqd]");
+        testGetPlanCacheKey("{$or: [{a: 1}, {b: 1}], c: 1, d: 1}", "{}", "{}",
+                            "an[or[eqa,eqb],eqc,eqd]");
+        testGetPlanCacheKey("{a: 1, b: 1, c: 1}", "{}", "{}", "an[eqa,eqb,eqc]");
+        testGetPlanCacheKey("{a: 1, beqc: 1}", "{}", "{}", "an[eqa,eqbeqc]");
+        testGetPlanCacheKey("{ap1a: 1}", "{}", "{}", "eqap1a");
+        testGetPlanCacheKey("{aab: 1}", "{}", "{}", "eqaab");
+
         // With sort
-        testGetPlanCacheKey("{}", "{a: 1}", "{}", "anaa");
-        testGetPlanCacheKey("{}", "{a: -1}", "{}", "anda");
+        testGetPlanCacheKey("{}", "{a: 1}", "{}", "an~aa");
+        testGetPlanCacheKey("{}", "{a: -1}", "{}", "an~da");
         testGetPlanCacheKey("{}", "{a: {$meta: 'textScore'}}", "{a: {$meta: 'textScore'}}",
-                            "antap{ $meta: \"textScore\" }a");
+                            "an~ta|{ $meta: \"textScore\" }a");
+        testGetPlanCacheKey("{a: 1}", "{b: 1}", "{}", "eqa~ab");
+
         // With projection
-        testGetPlanCacheKey("{}", "{}", "{a: 1}", "anp1a");
-        testGetPlanCacheKey("{}", "{}", "{a: 0}", "anp0a");
-        testGetPlanCacheKey("{}", "{}", "{a: 99}", "anp99a");
-        testGetPlanCacheKey("{}", "{}", "{a: 'foo'}", "anp\"foo\"a");
-        testGetPlanCacheKey("{}", "{}", "{a: {$slice: [3, 5]}}", "anp{ $slice: [ 3, 5 ] }a");
-        testGetPlanCacheKey("{}", "{}", "{a: {$elemMatch: {x: 2}}}", "anp{ $elemMatch: { x: 2 } }a");
-        testGetPlanCacheKey("{a: 1}", "{}", "{'a.$': 1}", "eqap1a.$");
+        testGetPlanCacheKey("{}", "{}", "{a: 1}", "an|1a");
+        testGetPlanCacheKey("{}", "{}", "{a: 0}", "an|0a");
+        testGetPlanCacheKey("{}", "{}", "{a: 99}", "an|99a");
+        testGetPlanCacheKey("{}", "{}", "{a: 'foo'}", "an|\"foo\"a");
+        testGetPlanCacheKey("{}", "{}", "{a: {$slice: [3, 5]}}", "an|{ $slice: \\[ 3\\, 5 \\] }a");
+        testGetPlanCacheKey("{}", "{}", "{a: {$elemMatch: {x: 2}}}",
+                            "an|{ $elemMatch: { x: 2 } }a");
+        testGetPlanCacheKey("{a: 1}", "{}", "{'a.$': 1}", "eqa|1a.$");
+        testGetPlanCacheKey("{a: 1}", "{}", "{a: 1}", "eqa|1a");
+
         // Projection should be order-insensitive
-        testGetPlanCacheKey("{}", "{}", "{a: 1, b: 1}", "anp1a1b");
-        testGetPlanCacheKey("{}", "{}", "{b: 1, a: 1}", "anp1a1b");
+        testGetPlanCacheKey("{}", "{}", "{a: 1, b: 1}", "an|1a1b");
+        testGetPlanCacheKey("{}", "{}", "{b: 1, a: 1}", "an|1a1b");
+
         // With or-elimination and projection
-        testGetPlanCacheKey("{$or: [{a: 1}]}", "{}", "{_id: 0, a: 1}", "eqap0_id1a");
-        testGetPlanCacheKey("{$or: [{a: 1}]}", "{}", "{'a.$': 1}", "eqap1a.$");
+        testGetPlanCacheKey("{$or: [{a: 1}]}", "{}", "{_id: 0, a: 1}", "eqa|0_id1a");
+        testGetPlanCacheKey("{$or: [{a: 1}]}", "{}", "{'a.$': 1}", "eqa|1a.$");
+    }
+
+    // Delimiters found in user field names or non-standard projection field values
+    // must be escaped.
+    TEST(PlanCacheTest, GetPlanCacheKeyEscaped) {
+        // Field name in query.
+        testGetPlanCacheKey("{'a,[]~|': 1}", "{}", "{}", "eqa\\,\\[\\]\\~\\|");
+
+        // Field name in sort.
+        testGetPlanCacheKey("{}", "{'a,[]~|': 1}", "{}", "an~aa\\,\\[\\]\\~\\|");
+
+        // Field name in projection.
+        testGetPlanCacheKey("{}", "{}", "{'a,[]~|': 1}", "an|1a\\,\\[\\]\\~\\|");
+
+        // Value in projection.
+        testGetPlanCacheKey("{}", "{}", "{a: 'foo,[]~|'}", "an|\"foo\\,\\[\\]\\~\\|\"a");
     }
 
     // Cache keys for $geoWithin queries with legacy and GeoJSON coordinates should