[base] CHECK_LE(pos, size()) in BasicStringPiece::substr

This change adds a CHECK_LE(pos, size()) in BasicStringPiece::substr.
This attempts to model standard library behavior, since
std::basic_string_view::substr throws an out_of_range exception in this
case.

Reference: https://wg21.link/string.view.ops#itemdecl:2

TBR=dcheng

Bug: 1049498
Change-Id: Ie5c946b8cd1b2cb9d7022bfcd5fa2a89372ec31c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2372445
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802440}

base/strings/string_piece.h

@@ -325,8 +325,7 @@ template <typename STRING_TYPE> class BasicStringPiece {
   constexpr BasicStringPiece substr(
       size_type pos,
       size_type n = BasicStringPiece::npos) const {
-    // TODO(crbug.com/1049498): Be less lenient here and CHECK(pos <= size()).
-    pos = std::min(pos, size());
+    CHECK_LE(pos, size());
     return {data() + pos, std::min(n, size() - pos)};
   }
 

base/strings/string_piece_unittest.cc

@@ -474,11 +474,7 @@ TYPED_TEST(CommonStringPieceTest, CheckFind) {
   ASSERT_EQ(a.substr(23, 99), c);
   ASSERT_EQ(a.substr(0), a);
   ASSERT_EQ(a.substr(3, 2), TestFixture::as_string("de"));
-  // empty string nonsense
-  ASSERT_EQ(a.substr(99, 2), e);
-  ASSERT_EQ(d.substr(99), e);
   ASSERT_EQ(d.substr(0, 99), e);
-  ASSERT_EQ(d.substr(99, 99), e);
 }
 
 TYPED_TEST(CommonStringPieceTest, CheckCustom) {
@@ -678,6 +674,11 @@ TEST(StringPieceTest, OutOfBoundsDeath) {
     StringPiece piece;
     ASSERT_DEATH_IF_SUPPORTED(piece.remove_prefix(1), "");
   }
+
+  {
+    StringPiece piece;
+    ASSERT_DEATH_IF_SUPPORTED(piece.substr(1), "");
+  }
 }
 
 TEST(StringPieceTest, ConstexprData) {

base/strings/string_util_internal.h

@@ -5,6 +5,8 @@
 #ifndef BASE_STRINGS_STRING_UTIL_INTERNAL_H_
 #define BASE_STRINGS_STRING_UTIL_INTERNAL_H_
 
+#include <algorithm>
+
 #include "base/logging.h"
 #include "base/notreached.h"
 #include "base/strings/string_piece.h"
@@ -130,7 +132,7 @@ BasicStringPiece<Str> TrimStringPieceT(BasicStringPiece<Str> input,
   size_t end = (positions & TRIM_TRAILING)
                    ? input.find_last_not_of(trim_chars) + 1
                    : input.size();
-  return input.substr(begin, end - begin);
+  return input.substr(std::min(begin, input.size()), end - begin);
 }
 
 template <typename STR>

chromeos/network/onc/variable_expander.cc

@@ -4,6 +4,8 @@
 
 #include "chromeos/network/onc/variable_expander.h"
 
+#include <algorithm>
+
 #include "base/strings/strcat.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
@@ -86,8 +88,8 @@ bool Expand(base::StringPiece variable_name,
       }
     }
 
-    const base::StringPiece replacement_part =
-        replacement.substr(replacement_start, replacement_count);
+    const base::StringPiece replacement_part = replacement.substr(
+        std::min(replacement_start, replacement.size()), replacement_count);
     // Don't use ReplaceSubstringsAfterOffset here, it can lead to a doubling
     // of tokens, see VariableExpanderTest.DoesNotRecurse test.
     base::ReplaceFirstSubstringAfterOffset(str, token_start, full_token,

google_apis/gaia/oauth_multilogin_result.cc

@@ -4,6 +4,8 @@
 
 #include "google_apis/gaia/oauth_multilogin_result.h"
 
+#include <algorithm>
+
 #include "base/compiler_specific.h"
 #include "base/json/json_reader.h"
 #include "base/logging.h"
@@ -59,7 +61,7 @@ OAuthMultiloginResult::OAuthMultiloginResult(
 base::StringPiece OAuthMultiloginResult::StripXSSICharacters(
     const std::string& raw_data) {
   base::StringPiece body(raw_data);
-  return body.substr(body.find('\n'));
+  return body.substr(std::min(body.find('\n'), body.size()));
 }
 
 void OAuthMultiloginResult::TryParseFailedAccountsFromValue(