SERVER-29762 Look up arrays by index during update.

This patch moves getPositionalPathSpecification() from its location in
document_path_support.h to stringutils.h, where it is renamed to
parseUnsignedBase10Integer(). This function should now be what all
code uses that needs to use a path component to look up an element in
an array.

There are two problems with trying to look up an array element by
string, which this patch addresses (for one particular case). The
first is that it will fail if the path component has leading zeros
(e.g. "a.01.b").

The second problem is that mutable BSON does not maintain the
invariant that array elements have their index as their field name
throughout the entire lifetime of the mutable document. Array lookups
by string fail during the times that this invariant does not hold
(notably, after a mutable array has been extended in length).

3 files changed, 100 insertions, 6 deletions

diff --git a/src/mongo/util/stringutils.cpp b/src/mongo/util/stringutils.cpp
index 3f695fc9e4b..f9bf74eb778 100644
--- a/src/mongo/util/stringutils.cpp
+++ b/src/mongo/util/stringutils.cpp
@@ -29,8 +29,11 @@
 
 #include "mongo/platform/basic.h"
 
+#include <cctype>
+
 #include "mongo/util/stringutils.h"
 
+#include "mongo/base/parse_number.h"
 #include "mongo/util/hex.h"
 
 namespace mongo {
@@ -224,4 +227,17 @@ std::string escape(StringData sd, bool escape_slash) {
     return ret.str();
 }
 
+boost::optional<size_t> parseUnsignedBase10Integer(StringData fieldName) {
+    // Do not accept positions like '-4' or '+4'
+    if (!std::isdigit(fieldName[0])) {
+        return boost::none;
+    }
+    unsigned int index;
+    auto status = parseNumberFromStringWithBase<unsigned int>(fieldName, 10, &index);
+    if (status.isOK()) {
+        return static_cast<size_t>(index);
+    }
+    return boost::none;
+}
+
 }  // namespace mongo
diff --git a/src/mongo/util/stringutils.h b/src/mongo/util/stringutils.h
index 5f4be4f9163..13e982e22e5 100644
--- a/src/mongo/util/stringutils.h
+++ b/src/mongo/util/stringutils.h
@@ -31,6 +31,7 @@
 
 #include <ctype.h>
 
+#include <boost/optional.hpp>
 #include <memory>
 #include <string>
 #include <vector>
@@ -105,4 +106,11 @@ int versionCmp(const StringData rhs, const StringData lhs);
  */
 std::string escape(StringData s, bool escape_slash = false);
 
+/**
+ * Converts 'integer' from a base-10 string to a size_t value or returns boost::none if 'integer'
+ * is not a valid base-10 string. A valid string is not allowed to have anything but decimal
+ * numerals, not even a +/- prefix or leading/trailing whitespace.
+ */
+boost::optional<size_t> parseUnsignedBase10Integer(StringData integer);
+
 }  // namespace mongo
diff --git a/src/mongo/util/stringutils_test.cpp b/src/mongo/util/stringutils_test.cpp
index 302fc5c3de3..80bbfc5db10 100644
--- a/src/mongo/util/stringutils_test.cpp
+++ b/src/mongo/util/stringutils_test.cpp
@@ -36,7 +36,7 @@ namespace mongo {
 
 using std::string;
 
-TEST(Comparison, Basic) {
+TEST(StringUtilsTest, Basic) {
     //
     // Basic version comparison tests with different version string types
     //
@@ -58,7 +58,7 @@ TEST(Comparison, Basic) {
     ASSERT(versionCmp("1.2.3-pre", "1.2.3") < 0);
 }
 
-TEST(LexNumCmp, Simple1) {
+TEST(StringUtilsTest, Simple1) {
     ASSERT_EQUALS(0, LexNumCmp::cmp("a.b.c", "a.b.c", false));
 }
 
@@ -69,7 +69,7 @@ void assertCmp(int expected, StringData s1, StringData s2, bool lexOnly = false)
     ASSERT_EQUALS(expected < 0, cmp(s1, s2));
 }
 
-TEST(LexNumCmp, Simple2) {
+TEST(StringUtilsTest, Simple2) {
     ASSERT(!isdigit((char)255));
 
     assertCmp(0, "a", "a");
@@ -163,14 +163,14 @@ TEST(LexNumCmp, Simple2) {
     assertCmp(0, "ac.t", "ac.t");
 }
 
-TEST(LexNumCmp, LexOnly) {
+TEST(StringUtilsTest, LexOnly) {
     assertCmp(-1, "0", "00", true);
     assertCmp(1, "1", "01", true);
     assertCmp(-1, "1", "11", true);
     assertCmp(1, "2", "11", true);
 }
 
-TEST(LexNumCmp, Substring1) {
+TEST(StringUtilsTest, Substring1) {
     assertCmp(0, "1234", "1234", false);
     assertCmp(0, StringData("1234"), StringData("1234"), false);
     assertCmp(0, StringData("1234", 4), StringData("1234", 4), false);
@@ -180,7 +180,7 @@ TEST(LexNumCmp, Substring1) {
     assertCmp(0, StringData("0001", 3), StringData("0000", 3), false);
 }
 
-TEST(IntegerToHex, VariousConversions) {
+TEST(StringUtilsTest, VariousConversions) {
     ASSERT_EQUALS(std::string("0"), integerToHex(0));
     ASSERT_EQUALS(std::string("1"), integerToHex(1));
     ASSERT_EQUALS(std::string("1337"), integerToHex(0x1337));
@@ -193,4 +193,74 @@ TEST(IntegerToHex, VariousConversions) {
     ASSERT_EQUALS(std::string("8000000000000000"),
                   integerToHex(std::numeric_limits<long long>::min()));
 }
+
+TEST(StringUtilsTest, CanParseZero) {
+    boost::optional<size_t> result = parseUnsignedBase10Integer("0");
+    ASSERT(result && *result == 0);
+}
+
+TEST(StringUtilsTest, CanParseDoubleZero) {
+    boost::optional<size_t> result = parseUnsignedBase10Integer("00");
+    ASSERT(result && *result == 0);
+}
+
+TEST(StringUtilsTest, PositivePrefixFailsToParse) {
+    boost::optional<size_t> result = parseUnsignedBase10Integer("+0");
+    ASSERT(!result);
+}
+
+TEST(StringUtilsTest, NegativePrefixFailsToParse) {
+    boost::optional<size_t> result = parseUnsignedBase10Integer("-0");
+    ASSERT(!result);
+}
+
+TEST(StringUtilsTest, CanParseIntValue) {
+    boost::optional<size_t> result = parseUnsignedBase10Integer("10");
+    ASSERT(result && *result == 10);
+}
+
+TEST(StringUtilsTest, CanParseIntValueWithLeadingZeros) {
+    boost::optional<size_t> result = parseUnsignedBase10Integer("0010");
+    ASSERT(result && *result == 10);
+}
+
+TEST(StringUtilsTest, TrailingLetterFailsToParse) {
+    boost::optional<size_t> result = parseUnsignedBase10Integer("5a");
+    ASSERT(!result);
+}
+
+TEST(StringUtilsTest, LeadingLetterFailsToParse) {
+    boost::optional<size_t> result = parseUnsignedBase10Integer("a5");
+    ASSERT(!result);
+}
+
+TEST(StringUtilsTest, LetterWithinNumberFailsToParse) {
+    boost::optional<size_t> result = parseUnsignedBase10Integer("5a5");
+    ASSERT(!result);
+}
+
+TEST(StringUtilsTest, HexStringFailsToParse) {
+    boost::optional<size_t> result = parseUnsignedBase10Integer("0xfeed");
+    ASSERT(!result);
+}
+
+TEST(StringUtilsTest, BinaryStringFailsToParse) {
+    boost::optional<size_t> result = parseUnsignedBase10Integer("0b11010010");
+    ASSERT(!result);
+}
+
+TEST(StringUtilsTest, LeadingWhitespaceFailsToParse) {
+    boost::optional<size_t> result = parseUnsignedBase10Integer(" 10");
+    ASSERT(!result);
+}
+
+TEST(StringUtilsTest, TrailingWhitespaceFailsToParse) {
+    boost::optional<size_t> result = parseUnsignedBase10Integer("10 ");
+    ASSERT(!result);
+}
+
+TEST(StringUtilsTest, WhitespaceWithinNumberFailsToParse) {
+    boost::optional<size_t> result = parseUnsignedBase10Integer(" 10");
+    ASSERT(!result);
 }
+}  // namespace mongo