summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorShawn Pearce <sop@google.com>2014-04-23 17:04:26 -0700
committerDavid Pursehouse <david.pursehouse@sonymobile.com>2014-04-24 00:22:45 +0000
commit52c73b831dd60f4065ffdd3dee80facb80bdbe05 (patch)
treee1073d81dd34e76a04afd0d1e35ab87ccd1d39a5
parent22daa1e5e2a678b0e9d2b4899dca1397206dd025 (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.java51
-rw-r--r--gerrit-gwtui/src/test/java/com/google/gerrit/client/diff/EditIteratorTest.java7
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));