diff options
author | Shawn Pearce <sop@google.com> | 2014-04-23 17:04:26 -0700 |
---|---|---|
committer | David Pursehouse <david.pursehouse@sonymobile.com> | 2014-04-24 00:22:45 +0000 |
commit | 52c73b831dd60f4065ffdd3dee80facb80bdbe05 (patch) | |
tree | e1073d81dd34e76a04afd0d1e35ab87ccd1d39a5 | |
parent | 22daa1e5e2a678b0e9d2b4899dca1397206dd025 (diff) |
SideBySide2: Fix failure to load from "ISE EditIterator out of bounds"
Intraline edit information can include a negative delta on the B
side of an edit, for example:
{
a: [...],
b: [...],
edit_a: [[4, 11], [14, 12]]
edit_b: [[4, 66], [37, 41], [-2, 1]]
}
In the edit_a or edit_b block the list pairs are the number of
characters to advance since the last position. -2 encodes the edit
iterator has to go back 2 positions to return the correct value.
This situation arose from an intraline edit list of:
REPLACE( 4-15, 4- 70),
REPLACE(29-41,107-148),
INSERT (62-62,146-147)
The EditList is sorted by the beginA values, where 29 < 62. This
forced the final INSERT to sort in the edit_b list after the REPLACE
at 107. Since 146 < 148 the final edit_b entry was encoded with a
negative delta (148 - 146 = -2).
Update the UI code to accept a negative delta in the iterator. There
are already many /diff responses with negative delta that will be
cached by browsers for 7 days.
There may be other issues with the intraline diff algorithm that
allows it to create this final INSERT span within the REPLACE
span, but the code is essentially unchanged for many years in
Gerrit Code Review. Handle the corner case for now in the UI while
the server code is investigated further.
Change-Id: Ibec0fc845356af7c02f25acac23c4e709f7c3a34
-rw-r--r-- | gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/EditIterator.java | 51 | ||||
-rw-r--r-- | gerrit-gwtui/src/test/java/com/google/gerrit/client/diff/EditIteratorTest.java | 7 |
2 files changed, 41 insertions, 17 deletions
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/EditIterator.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/EditIterator.java index 246c101cd1..4c8d80e00c 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/EditIterator.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/EditIterator.java @@ -22,8 +22,8 @@ import net.codemirror.lib.LineCharacter; class EditIterator { private final JsArrayString lines; private final int startLine; - private int currLineIndex; - private int currLineOffset; + private int line; + private int pos; EditIterator(JsArrayString lineArray, int start) { lines = lineArray; @@ -31,27 +31,44 @@ class EditIterator { } LineCharacter advance(int numOfChar) { - while (currLineIndex < lines.length()) { - int lengthWithNewline = - lines.get(currLineIndex).length() - currLineOffset + 1; - if (numOfChar < lengthWithNewline) { + numOfChar = adjustForNegativeDelta(numOfChar); + + while (line < lines.length()) { + int len = lines.get(line).length() - pos + 1; // + 1 for LF + if (numOfChar < len) { LineCharacter at = LineCharacter.create( - startLine + currLineIndex, - numOfChar + currLineOffset); - currLineOffset += numOfChar; + startLine + line, + numOfChar + pos); + pos += numOfChar; return at; } - numOfChar -= lengthWithNewline; - advanceLine(); + + numOfChar -= len; + line++; + pos = 0; + if (numOfChar == 0) { - return LineCharacter.create(startLine + currLineIndex, 0); + return LineCharacter.create(startLine + line, 0); } } - throw new IllegalStateException("EditIterator index out of bound"); + + throw new IllegalStateException("EditIterator index out of bounds"); } - private void advanceLine() { - currLineIndex++; - currLineOffset = 0; + private int adjustForNegativeDelta(int n) { + while (n < 0) { + if (-n <= pos) { + pos += n; + return 0; + } + + n += pos; + line--; + if (line < 0) { + throw new IllegalStateException("EditIterator index out of bounds"); + } + pos = lines.get(line).length() + 1; + } + return n; } -}
\ No newline at end of file +} diff --git a/gerrit-gwtui/src/test/java/com/google/gerrit/client/diff/EditIteratorTest.java b/gerrit-gwtui/src/test/java/com/google/gerrit/client/diff/EditIteratorTest.java index 12a616a6ad..1ebf6bbde7 100644 --- a/gerrit-gwtui/src/test/java/com/google/gerrit/client/diff/EditIteratorTest.java +++ b/gerrit-gwtui/src/test/java/com/google/gerrit/client/diff/EditIteratorTest.java @@ -45,6 +45,13 @@ public class EditIteratorTest extends GwtTest { } @Test + public void testNegativeAdvance() { + EditIterator i = new EditIterator(lines, 0); + assertLineChsEqual(LineCharacter.create(1, 1), i.advance(5)); + assertLineChsEqual(LineCharacter.create(0, 3), i.advance(-2)); + } + + @Test public void testNoAdvance() { EditIterator iter = new EditIterator(lines, 0); assertLineChsEqual(LineCharacter.create(0), iter.advance(0)); |