Issue with find and npos

Summary:
I dont know whats the expected behavior but for std::string it seems
to work.

@override-unit-failures

Test Plan:
unit-tests

[ RUN      ] FBString.findWithNpos
folly/test/FBStringTest.cpp:1147: Failure
Value of: fbstr.find(":", fbstring::npos)
Actual: 9
Expected: fbstring::npos
Which is: 18446744073709551615

Reviewed By: andrei.alexandrescu@fb.com

FB internal diff: D1012870

diff --git a/folly/FBString.h b/folly/FBString.h
index 541baa0ff9949f58b6b938420d80f05c0b442fe0..6f5ea13194978aad661e2227f6fcc34e90ba8d26 100644 (file)
--- a/folly/FBString.h
+++ b/folly/FBString.h
@@ -1794,7 +1794,9 @@ public:
                  const size_type nsize) const {
     if (!nsize) return pos;
     auto const size = this->size();
-    if (nsize + pos > size) return npos;
+    // nsize + pos can overflow (eg pos == npos), guard against that by checking
+    // that nsize + pos does not wrap around.
+    if (nsize + pos > size || nsize + pos < pos) return npos;
     // Don't use std::search, use a Boyer-Moore-like trick by comparing
     // the last characters first
     auto const haystack = data();

diff --git a/folly/test/FBStringTest.cpp b/folly/test/FBStringTest.cpp
index 3e294282449be887f643b9f25ed763bc6f1db6ad..3efd652e4a9647ec08009ddf1318ada672777070 100644 (file)
--- a/folly/test/FBStringTest.cpp
+++ b/folly/test/FBStringTest.cpp
@@ -1139,6 +1139,11 @@ TEST(FBString, testFixedBugs) {
   }
 }
 
+TEST(FBString, findWithNpos) {
+  fbstring fbstr("localhost:80");
+  EXPECT_EQ(fbstring::npos, fbstr.find(":", fbstring::npos));
+}
+
 TEST(FBString, testHash) {
   fbstring a;
   fbstring b;