Fix an exception in the diff code for a corner case

This regression was introduced with I1e15767fc1. It resulted in an
internal server error when the diff was requested for the old file
name of a renamed file. It's actually not reasonable to request that
diff but due to an issue in PolyGerrit, users now easily run into that
case.

The fix reverts the behavior for this case back to the previous one:
The backend returns an empty diff result with only skipped lines,
oblivious of the passed context and the presence of comments.

Whether that behavior is *correct* is difficult to decide as it's hard
to define what should be returned in that case. Actually, requesting
the diff for the old file name (given that Gerrit identified a rename)
is not a use case which makes sense.

This change additionally adds two further tests which weren't covered
previously.

Users also reported a similar issue with PDF files. When trying to
reproduce it locally, it didn't occur. However, the same PDF files were
locally marked as binary whereas on a Gerrit server, on which the issue
occurred, their content was shown as text on PolyGerrit. Given this
and the similar exception, it's very likely that the fix of this change
will also help for the PDF issue.

Bug: Issue 9498
Bug: Issue 9557
Change-Id: I101d262c1566aa89a91a82394f51dab7c6e6311e
This commit is contained in:
Alice Kober-Sotzek
2018-08-16 17:38:35 +02:00
parent 944cd404ac
commit 3be9dc9c6f
3 changed files with 137 additions and 0 deletions

View File

@@ -285,6 +285,13 @@ class PatchScriptBuilder {
int aSize = a.src.size();
int bSize = b.src.size();
if (edits.isEmpty() && (aSize == 0 || bSize == 0)) {
// The diff was requested for a file which was either added or deleted but which JGit doesn't
// consider a file addition/deletion (e.g. requesting a diff for the old file name of a
// renamed file looks like a deletion).
return;
}
Optional<Edit> lastEdit = getLast(edits);
if (isNewlineAtEndDeleted()) {
Optional<Edit> lastLineEdit = lastEdit.filter(edit -> edit.getEndA() == aSize);