From 148e9c9bf1d3d439912b451158ff3a8d96dd5b75 Mon Sep 17 00:00:00 2001 From: "leandrogracia@chromium.org" Date: Thu, 5 Jul 2012 20:03:40 +0000 Subject: [PATCH] Character iterators should not advance if they are at end https://bugs.webkit.org/show_bug.cgi?id=90560 Reviewed by Ryosuke Niwa. Source/WebCore: CharacterIterator and BackwardsCharacterIterator try to advance their internal TextIterator without checking if they already are at end. This can cause crashes in TextIterator::advance. Test: platform/chromium/editing/surrounding-text/surrounding-text.html * editing/SurroundingText.cpp: (WebCore::SurroundingText::SurroundingText): * editing/TextIterator.cpp: (WebCore::CharacterIterator::advance): (WebCore::BackwardsCharacterIterator::advance): LayoutTests: Add a new test case where character iterators are already at end when trying to advance. This was caught by Chromium's address sanitizer here: http://code.google.com/p/chromium/issues/detail?id=135705 * platform/chromium/editing/surrounding-text/surrounding-text-expected.txt: * platform/chromium/editing/surrounding-text/surrounding-text.html: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@121921 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- LayoutTests/ChangeLog | 14 ++++++++++++++ .../surrounding-text/surrounding-text-expected.txt | 1 + .../editing/surrounding-text/surrounding-text.html | 1 + Source/WebCore/ChangeLog | 19 +++++++++++++++++++ Source/WebCore/editing/SurroundingText.cpp | 6 ++++-- Source/WebCore/editing/TextIterator.cpp | 4 ++++ 6 files changed, 43 insertions(+), 2 deletions(-) diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 41e569f..275f45e 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,17 @@ +2012-07-05 Leandro Gracia Gil + + Character iterators should not advance if they are at end + https://bugs.webkit.org/show_bug.cgi?id=90560 + + Reviewed by Ryosuke Niwa. + + Add a new test case where character iterators are already at end when + trying to advance. This was caught by Chromium's address sanitizer + here: http://code.google.com/p/chromium/issues/detail?id=135705 + + * platform/chromium/editing/surrounding-text/surrounding-text-expected.txt: + * platform/chromium/editing/surrounding-text/surrounding-text.html: + 2012-07-05 Alexey Proskuryakov [Mac][WK2] Enable HTTPS tests diff --git a/LayoutTests/platform/chromium/editing/surrounding-text/surrounding-text-expected.txt b/LayoutTests/platform/chromium/editing/surrounding-text/surrounding-text-expected.txt index 912c1c5..a0842d0 100644 --- a/LayoutTests/platform/chromium/editing/surrounding-text/surrounding-text-expected.txt +++ b/LayoutTests/platform/chromium/editing/surrounding-text/surrounding-text-expected.txt @@ -15,6 +15,7 @@ PASS surroundingText('
This is PASS surroundingText('
012345678901234567890123456789
', 15, 12) is "901234567890" PASS surroundingText('12345', 0, 100) is "" PASS surroundingText('12345', 0, 100) is "" +PASS surroundingText('

.', 0, 2) is "." PASS successfullyParsed is true TEST COMPLETE diff --git a/LayoutTests/platform/chromium/editing/surrounding-text/surrounding-text.html b/LayoutTests/platform/chromium/editing/surrounding-text/surrounding-text.html index 6b7f478..7c4b2ae 100644 --- a/LayoutTests/platform/chromium/editing/surrounding-text/surrounding-text.html +++ b/LayoutTests/platform/chromium/editing/surrounding-text/surrounding-text.html @@ -40,6 +40,7 @@ function run() { shouldBeEqualToString('surroundingText(\'

012345678901234567890123456789
\', 15, 12)', '901234567890'); shouldBeEqualToString('surroundingText(\'12345\', 0, 100)', ''); shouldBeEqualToString('surroundingText(\'12345\', 0, 100)', ''); + shouldBeEqualToString('surroundingText(\'

.\', 0, 2)', '.'); document.body.removeChild(document.getElementById('test')); finishJSTest(); diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index d883e04..3e2ecfa 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,22 @@ +2012-07-05 Leandro Gracia Gil + + Character iterators should not advance if they are at end + https://bugs.webkit.org/show_bug.cgi?id=90560 + + Reviewed by Ryosuke Niwa. + + CharacterIterator and BackwardsCharacterIterator try to advance their + internal TextIterator without checking if they already are at end. + This can cause crashes in TextIterator::advance. + + Test: platform/chromium/editing/surrounding-text/surrounding-text.html + + * editing/SurroundingText.cpp: + (WebCore::SurroundingText::SurroundingText): + * editing/TextIterator.cpp: + (WebCore::CharacterIterator::advance): + (WebCore::BackwardsCharacterIterator::advance): + 2012-07-05 John Mellor Text Autosizing: Add basic framework diff --git a/Source/WebCore/editing/SurroundingText.cpp b/Source/WebCore/editing/SurroundingText.cpp index df9aa7e..cfaedda 100644 --- a/Source/WebCore/editing/SurroundingText.cpp +++ b/Source/WebCore/editing/SurroundingText.cpp @@ -45,7 +45,8 @@ SurroundingText::SurroundingText(const VisiblePosition& visiblePosition, unsigne { const unsigned halfMaxLength = maxLength / 2; CharacterIterator forwardIterator(makeRange(visiblePosition, endOfDocument(visiblePosition)).get(), TextIteratorStopsOnFormControls); - forwardIterator.advance(maxLength - halfMaxLength); + if (!forwardIterator.atEnd()) + forwardIterator.advance(maxLength - halfMaxLength); Position position = visiblePosition.deepEquivalent().parentAnchoredEquivalent(); Document* document = position.document(); @@ -53,7 +54,8 @@ SurroundingText::SurroundingText(const VisiblePosition& visiblePosition, unsigne return; BackwardsCharacterIterator backwardsIterator(makeRange(startOfDocument(visiblePosition), visiblePosition).get(), TextIteratorStopsOnFormControls); - backwardsIterator.advance(halfMaxLength); + if (!backwardsIterator.atEnd()) + backwardsIterator.advance(halfMaxLength); m_positionOffsetInContent = Range::create(document, backwardsIterator.range()->endPosition(), position)->text().length(); m_contentRange = Range::create(document, backwardsIterator.range()->endPosition(), forwardIterator.range()->startPosition()); diff --git a/Source/WebCore/editing/TextIterator.cpp b/Source/WebCore/editing/TextIterator.cpp index 7ca3dbe..d7020a7 100644 --- a/Source/WebCore/editing/TextIterator.cpp +++ b/Source/WebCore/editing/TextIterator.cpp @@ -1406,6 +1406,8 @@ PassRefPtr CharacterIterator::range() const void CharacterIterator::advance(int count) { + ASSERT(!atEnd()); + if (count <= 0) { ASSERT(count == 0); return; @@ -1514,6 +1516,8 @@ PassRefPtr BackwardsCharacterIterator::range() const void BackwardsCharacterIterator::advance(int count) { + ASSERT(!atEnd()); + if (count <= 0) { ASSERT(!count); return; -- 2.7.4