Fix bug in BreakIterator class that could cause illegal access

after setting new content by calling SetText function.

Public function SetText() couldn't update internal string
because it was a const reference.
Used StringPiece16 to fix.

BUG=411213
TEST=base_unittests --gtest_filter="BreakIteratorTest.GetStringAfterSetText"

Review URL: https://codereview.chromium.org/545113002

Cr-Commit-Position: refs/heads/master@{#298207}

3 files changed, 48 insertions, 5 deletions

diff --git a/base/i18n/break_iterator.cc b/base/i18n/break_iterator.cc
index 89b1d0f..e3aaa2b 100644
--- a/base/i18n/break_iterator.cc
+++ b/base/i18n/break_iterator.cc
@@ -14,7 +14,7 @@ namespace i18n {
 
 const size_t npos = static_cast<size_t>(-1);
 
-BreakIterator::BreakIterator(const string16& str, BreakType break_type)
+BreakIterator::BreakIterator(const StringPiece16& str, BreakType break_type)
     : iter_(NULL),
       string_(str),
       break_type_(break_type),
@@ -22,7 +22,7 @@ BreakIterator::BreakIterator(const string16& str, BreakType break_type)
       pos_(0) {
 }
 
-BreakIterator::BreakIterator(const string16& str, const string16& rules)
+BreakIterator::BreakIterator(const StringPiece16& str, const string16& rules)
     : iter_(NULL),
       string_(str),
       rules_(rules),
@@ -132,6 +132,7 @@ bool BreakIterator::SetText(const base::char16* text, const size_t length) {
     NOTREACHED() << "ubrk_setText failed";
     return false;
   }
+  string_ = StringPiece16(text, length);
   return true;
 }
 
@@ -172,6 +173,10 @@ bool BreakIterator::IsGraphemeBoundary(size_t position) const {
 }
 
 string16 BreakIterator::GetString() const {
+  return GetStringPiece().as_string();
+}
+
+StringPiece16 BreakIterator::GetStringPiece() const {
   DCHECK(prev_ != npos && pos_ != npos);
   return string_.substr(prev_, pos_ - prev_);
 }
diff --git a/base/i18n/break_iterator.h b/base/i18n/break_iterator.h
index 8120d47..19fdbe0 100644
--- a/base/i18n/break_iterator.h
+++ b/base/i18n/break_iterator.h
@@ -8,6 +8,7 @@
 #include "base/basictypes.h"
 #include "base/i18n/base_i18n_export.h"
 #include "base/strings/string16.h"
+#include "base/strings/string_piece.h"
 
 // The BreakIterator class iterates through the words, word breaks, and
 // line breaks in a UTF-16 string.
@@ -71,12 +72,12 @@ class BASE_I18N_EXPORT BreakIterator {
   };
 
   // Requires |str| to live as long as the BreakIterator does.
-  BreakIterator(const string16& str, BreakType break_type);
+  BreakIterator(const StringPiece16& str, BreakType break_type);
   // Make a rule-based iterator. BreakType == RULE_BASED is implied.
   // TODO(andrewhayden): This signature could easily be misinterpreted as
   // "(const string16& str, const string16& locale)". We should do something
   // better.
-  BreakIterator(const string16& str, const string16& rules);
+  BreakIterator(const StringPiece16& str, const string16& rules);
   ~BreakIterator();
 
   // Init() must be called before any of the iterators are valid.
@@ -115,6 +116,8 @@ class BASE_I18N_EXPORT BreakIterator {
   // have advanced to somewhere useful.
   string16 GetString() const;
 
+  StringPiece16 GetStringPiece() const;
+
   // Returns the value of pos() returned before Advance() was last called.
   size_t prev() const { return prev_; }
 
@@ -130,7 +133,7 @@ class BASE_I18N_EXPORT BreakIterator {
   void* iter_;
 
   // The string we're iterating over. Can be changed with SetText(...)
-  const string16& string_;
+  StringPiece16 string_;
 
   // Rules for our iterator. Mutually exclusive with break_type_.
   const string16 rules_;
diff --git a/base/i18n/break_iterator_unittest.cc b/base/i18n/break_iterator_unittest.cc
index 6dcae18..8569135 100644
--- a/base/i18n/break_iterator_unittest.cc
+++ b/base/i18n/break_iterator_unittest.cc
@@ -334,5 +334,40 @@ TEST(BreakIteratorTest, BreakCharacter) {
   }
 }
 
+// Test for https://code.google.com/p/chromium/issues/detail?id=411213
+// We should be able to get valid substrings with GetString() function
+// after setting new content by calling SetText().
+TEST(BreakIteratorTest, GetStringAfterSetText) {
+  const string16 initial_string(ASCIIToUTF16("str"));
+  BreakIterator iter(initial_string, BreakIterator::BREAK_WORD);
+  ASSERT_TRUE(iter.Init());
+
+  const string16 long_string(ASCIIToUTF16("another,string"));
+  EXPECT_TRUE(iter.SetText(long_string.c_str(), long_string.size()));
+  EXPECT_TRUE(iter.Advance());
+  EXPECT_TRUE(iter.Advance()); // Advance to ',' in |long_string|
+
+  // Check that the current position is out of bounds of the |initial_string|.
+  EXPECT_LT(initial_string.size(), iter.pos());
+
+  // Check that we can get a valid substring of |long_string|.
+  EXPECT_EQ(ASCIIToUTF16(","), iter.GetString());
+}
+
+TEST(BreakIteratorTest, GetStringPiece) {
+  const string16 initial_string(ASCIIToUTF16("some string"));
+  BreakIterator iter(initial_string, BreakIterator::BREAK_WORD);
+  ASSERT_TRUE(iter.Init());
+
+  EXPECT_TRUE(iter.Advance());
+  EXPECT_EQ(iter.GetString(), iter.GetStringPiece().as_string());
+  EXPECT_EQ(StringPiece16(ASCIIToUTF16("some")), iter.GetStringPiece());
+
+  EXPECT_TRUE(iter.Advance());
+  EXPECT_TRUE(iter.Advance());
+  EXPECT_EQ(iter.GetString(), iter.GetStringPiece().as_string());
+  EXPECT_EQ(StringPiece16(ASCIIToUTF16("string")), iter.GetStringPiece());
+}
+
 }  // namespace i18n
 }  // namespace base