opendev/gerrit
Code Issues Proposed changes

Fix server crash when showing no difference

Browse Source 
When displaying no difference between files the server crashed
trying to guess the MIME type of the old and new image.  If we
have no difference then there is no FileHeader returned in the
DiffCacheContent, so we cannot ask it for the file name to give
to the MIME type guessing routines.  Instead we must rely upon
the DiffCacheKey, which also has the file names stored in it.

Bug: GERRIT-248
Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
Shawn O. Pearce
2009-07-18 11:18:12 -07:00
parent 9fb4ce89ef
commit aaa09300d8
2 changed files with 57 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/main/java/com/google/gerrit/server/patch/PatchScriptAction.java
View File

@@ -126,7 +126,7 @@ class PatchScriptAction implements Action<PatchScript> {
}
try {
return b.toPatchScript(contentWS, comments, contentActual);
return b.toPatchScript(key, contentWS, comments, contentActual);
} catch (CorruptEntityException e) {
log.error("File content for " + key + " unavailable", e);
throw new Failure(new NoSuchEntityException());

86
src/main/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
View File

@@ -96,9 +96,9 @@ class PatchScriptBuilder {
return settings.getContext();
}
PatchScript toPatchScript(final DiffCacheContent contentWS,
final CommentDetail comments, final DiffCacheContent contentAct)
throws CorruptEntityException {
PatchScript toPatchScript(final DiffCacheKey key,
final DiffCacheContent contentWS, final CommentDetail comments,
final DiffCacheContent contentAct) throws CorruptEntityException {
final FileHeader fh = contentAct.getFileHeader();
DisplayMethod displayA = DisplayMethod.DIFF;
DisplayMethod displayB = DisplayMethod.DIFF;
@@ -113,42 +113,51 @@ class PatchScriptBuilder {
displayB);
}
srcA = open(contentAct.getOldId());
MimeType typeA = registry.getMimeType(fh.getOldName(), srcA.getContent());
MimeType typeA;
MimeType typeB;
srcA = open(contentAct.getOldId());
typeA = registry.getMimeType(oldFileName(key, fh), srcA.getContent());
if (eq(contentAct.getOldId(), contentAct.getNewId())) {
srcB = srcA;
typeB = typeA;
} else {
srcB = open(contentAct.getNewId());
typeB = registry.getMimeType(fh.getNewName(), srcB.getContent());
typeB = registry.getMimeType(newFileName(key, fh), srcB.getContent());
}
switch (fh.getChangeType()) {
case DELETE:
if (isImage(typeA)) {
displayA = DisplayMethod.IMG;
}
displayB = DisplayMethod.NONE;
break;
case ADD:
displayA = DisplayMethod.NONE;
if (isImage(typeB)) {
displayB = DisplayMethod.IMG;
}
break;
case COPY:
case MODIFY:
case RENAME:
if (isImage(typeA)) {
displayA = DisplayMethod.IMG;
}
if (isImage(typeB)) {
displayB = DisplayMethod.IMG;
}
break;
if (fh != null) {
switch (fh.getChangeType()) {
case DELETE:
if (isImage(typeA)) {
displayA = DisplayMethod.IMG;
}
displayB = DisplayMethod.NONE;
break;
case ADD:
displayA = DisplayMethod.NONE;
if (isImage(typeB)) {
displayB = DisplayMethod.IMG;
}
break;
case COPY:
case MODIFY:
case RENAME:
if (isImage(typeA)) {
displayA = DisplayMethod.IMG;
}
if (isImage(typeB)) {
displayB = DisplayMethod.IMG;
}
break;
}
} else {
if (srcA.getContent().length > 0 && isImage(typeA)) {
displayA = DisplayMethod.IMG;
}
if (srcB.getContent().length > 0 && isImage(typeB)) {
displayB = DisplayMethod.IMG;
}
}
edits = new ArrayList<Edit>(contentAct.getEdits());
@@ -194,6 +203,23 @@ class PatchScriptBuilder {
displayB);
}
private static String oldFileName(final DiffCacheKey key, final FileHeader fh) {
if (fh != null) {
return fh.getOldName();
}
if (key.getSourceFileName() != null) {
return key.getSourceFileName();
}
return key.getFileName();
}
private static String newFileName(final DiffCacheKey key, final FileHeader fh) {
if (fh != null) {
return fh.getOldName();
}
return key.getFileName();
}
private boolean isImage(final MimeType type) {
return "image".equals(type.getMediaType()) && registry.isSafeInline(type);
}