Merge "PatchApplier: Check for existence of src/dest files before any operation"

This commit is contained in:
Han-Wen NIenhuys 2023-03-31 06:24:32 -04:00 committed by Gerrit Code Review @ Eclipse.org
commit d174684273
4 changed files with 131 additions and 23 deletions

View File

@ -93,12 +93,16 @@ protected void init(String aName, boolean preExists, boolean postExists)
}
protected void initPreImage(String aName) throws Exception {
File f = new File(db.getWorkTree(), aName);
preImage = IO
.readWholeStream(getTestResource(aName + "_PreImage"), 0)
.array();
addFile(aName, preImage);
}
protected void addFile(String aName, byte[] b) throws Exception {
File f = new File(db.getWorkTree(), aName);
Files.write(f.toPath(), b);
try (Git git = new Git(db)) {
Files.write(f.toPath(), preImage);
git.add().addFilepattern(aName).call();
}
}
@ -372,6 +376,68 @@ public void testShiftDown2() throws Exception {
verifyChange(result, "ShiftDown2");
}
@Test
public void testAddAlreadyExistingFile() throws Exception {
addFile("M1", "existing content".getBytes());
init("M1", false, false);
Result result = applyPatch();
assertEquals(1, result.getErrors().size());
assertEquals(0, result.getPaths().size());
}
@Test
public void testDeleteNonexistentFile() throws Exception {
init("NonASCIIDel", false, false);
Result result = applyPatch();
assertEquals(1, result.getErrors().size());
assertEquals(0, result.getPaths().size());
}
@Test
public void testModifyNonexistentFile() throws Exception {
init("ShiftDown", false, true);
Result result = applyPatch();
assertEquals(1, result.getErrors().size());
assertEquals(0, result.getPaths().size());
}
@Test
public void testRenameNonexistentFile() throws Exception {
init("RenameNoHunks", false, true);
Result result = applyPatch();
assertEquals(1, result.getErrors().size());
assertEquals(0, result.getPaths().size());
}
@Test
public void testCopyNonexistentFile() throws Exception {
init("CopyWithHunks", false, true);
Result result = applyPatch();
assertEquals(1, result.getErrors().size());
assertEquals(0, result.getPaths().size());
}
@Test
public void testCopyOnTopAlreadyExistingFile() throws Exception {
addFile("CopyResult", "existing content".getBytes());
init("CopyWithHunks", true, false);
Result result = applyPatch();
assertEquals(1, result.getErrors().size());
assertEquals(0, result.getPaths().size());
}
@Test
public void testDoesNotAffectUnrelatedFiles() throws Exception {
initPreImage("Unaffected");

View File

@ -20,6 +20,9 @@ applyBinaryPatchTypeNotSupported=Couldn't apply binary patch of type {0}
applyTextPatchCannotApplyHunk=Hunk cannot be applied
applyTextPatchSingleClearingHunk=Expected a single hunk for clearing all content
applyBinaryResultOidWrong=Result of binary patch for file {0} has wrong OID
applyPatchWithoutSourceOnAlreadyExistingSource=Cannot perform {0} action on an existing file
applyPatchWithCreationOverAlreadyExistingDestination=Cannot perform {0} action which overrides an existing file
applyPatchWithSourceOnNonExistentSource=Cannot perform {0} action on a non-existent file
applyTextPatchUnorderedHunkApplications=Current hunk must be applied after the last hunk
applyTextPatchUnorderedHunks=Got unordered hunks
applyingCommit=Applying {0}

View File

@ -46,6 +46,9 @@ public static JGitText get() {
/***/ public String applyBinaryOidTooShort;
/***/ public String applyBinaryPatchTypeNotSupported;
/***/ public String applyBinaryResultOidWrong;
/***/ public String applyPatchWithoutSourceOnAlreadyExistingSource;
/***/ public String applyPatchWithCreationOverAlreadyExistingDestination;
/***/ public String applyPatchWithSourceOnNonExistentSource;
/***/ public String applyTextPatchCannotApplyHunk;
/***/ public String applyTextPatchSingleClearingHunk;
/***/ public String applyTextPatchUnorderedHunkApplications;

View File

@ -9,6 +9,11 @@
*/
package org.eclipse.jgit.patch;
import static org.eclipse.jgit.diff.DiffEntry.ChangeType.ADD;
import static org.eclipse.jgit.diff.DiffEntry.ChangeType.COPY;
import static org.eclipse.jgit.diff.DiffEntry.ChangeType.DELETE;
import static org.eclipse.jgit.diff.DiffEntry.ChangeType.MODIFY;
import static org.eclipse.jgit.diff.DiffEntry.ChangeType.RENAME;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
import java.io.ByteArrayInputStream;
@ -257,32 +262,33 @@ public Result applyPatch(Patch p) throws IOException {
Set<String> modifiedPaths = new HashSet<>();
for (FileHeader fh : p.getFiles()) {
ChangeType type = fh.getChangeType();
File src = getFile(fh.getOldPath());
File dest = getFile(fh.getNewPath());
if (!verifyExistence(fh, src, dest, result)) {
continue;
}
switch (type) {
case ADD: {
File f = getFile(fh.getNewPath());
if (f != null) {
FileUtils.mkdirs(f.getParentFile(), true);
FileUtils.createNewFile(f);
if (dest != null) {
FileUtils.mkdirs(dest.getParentFile(), true);
FileUtils.createNewFile(dest);
}
apply(fh.getNewPath(), dirCache, dirCacheBuilder, f, fh, result);
apply(fh.getNewPath(), dirCache, dirCacheBuilder, dest, fh, result);
}
break;
case MODIFY:
apply(fh.getOldPath(), dirCache, dirCacheBuilder,
getFile(fh.getOldPath()), fh, result);
case MODIFY: {
apply(fh.getOldPath(), dirCache, dirCacheBuilder, src, fh, result);
break;
case DELETE:
}
case DELETE: {
if (!inCore()) {
File old = getFile(fh.getOldPath());
if (!old.delete())
if (!src.delete())
throw new IOException(MessageFormat.format(
JGitText.get().cannotDeleteFile, old));
JGitText.get().cannotDeleteFile, src));
}
break;
}
case RENAME: {
File src = getFile(fh.getOldPath());
File dest = getFile(fh.getNewPath());
if (!inCore()) {
/*
* this is odd: we rename the file on the FS, but
@ -299,9 +305,7 @@ public Result applyPatch(Patch p) throws IOException {
break;
}
case COPY: {
File dest = getFile(fh.getNewPath());
if (!inCore()) {
File src = getFile(fh.getOldPath());
FileUtils.mkdirs(dest.getParentFile(), true);
Files.copy(src.toPath(), dest.toPath());
}
@ -309,10 +313,10 @@ public Result applyPatch(Patch p) throws IOException {
break;
}
}
if (fh.getChangeType() != ChangeType.DELETE)
if (fh.getChangeType() != DELETE)
modifiedPaths.add(fh.getNewPath());
if (fh.getChangeType() != ChangeType.COPY
&& fh.getChangeType() != ChangeType.ADD)
if (fh.getChangeType() != COPY
&& fh.getChangeType() != ADD)
modifiedPaths.add(fh.getOldPath());
}
@ -368,6 +372,38 @@ private TreeWalk getTreeWalkForFile(String path, DirCache cache)
return walk;
}
private boolean fileExists(String path, @Nullable File f)
throws IOException {
if (f != null) {
return f.exists();
}
return inCore() && TreeWalk.forPath(repo, path, beforeTree) != null;
}
private boolean verifyExistence(FileHeader fh, File src, File dest,
Result result) throws IOException {
boolean isValid = true;
boolean srcShouldExist = List.of(MODIFY, DELETE, RENAME, COPY)
.contains(fh.getChangeType());
boolean destShouldNotExist = List.of(ADD, RENAME, COPY)
.contains(fh.getChangeType());
if (srcShouldExist != fileExists(fh.getOldPath(), src)) {
result.addError(MessageFormat.format(srcShouldExist
? JGitText.get().applyPatchWithSourceOnNonExistentSource
: JGitText
.get().applyPatchWithoutSourceOnAlreadyExistingSource,
fh.getPatchType()), fh.getOldPath(), null);
isValid = false;
}
if (destShouldNotExist && fileExists(fh.getNewPath(), dest)) {
result.addError(MessageFormat.format(JGitText
.get().applyPatchWithCreationOverAlreadyExistingDestination,
fh.getPatchType()), fh.getNewPath(), null);
isValid = false;
}
return isValid;
}
private static final int FILE_TREE_INDEX = 1;
/**
@ -681,7 +717,7 @@ private boolean checkOid(ObjectId baseId, ObjectId id, ChangeType type, File f,
boolean hashOk = false;
if (id != null) {
hashOk = baseId.equals(id);
if (!hashOk && ChangeType.ADD.equals(type)
if (!hashOk && ADD.equals(type)
&& ObjectId.zeroId().equals(baseId)) {
// We create a new file. The OID of an empty file is not the
// zero id!