Merge "PatchApplier: Check for existence of src/dest files before any operation"
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/patch/PatchApplierTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/patch/PatchApplierTest.java
index 2a7ad2a..eeec149 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/patch/PatchApplierTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/patch/PatchApplierTest.java
@@ -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();
}
}
@@ -373,6 +377,68 @@ public void testShiftDown2() throws Exception {
}
@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");
String expectedUnaffectedText = initPostImage("Unaffected");
diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index 1655cce..78dd9bd 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -20,6 +20,9 @@
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}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index 77377a3..28634cb 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -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;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/patch/PatchApplier.java b/org.eclipse.jgit/src/org/eclipse/jgit/patch/PatchApplier.java
index 36420fc..da698d6 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/patch/PatchApplier.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/patch/PatchApplier.java
@@ -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!
