From 949ee670c6a0cdb7b0ecaaaa751799148899c709 Mon Sep 17 00:00:00 2001
From: David Pursehouse <david.pursehouse@gmail.com>
Date: Thu, 4 Jun 2020 14:07:34 +0900
Subject: [PATCH] ObjectDirectory: Explicitly handle NoSuchFileException

On the first attempt to move the temp file, NoSuchFileException can
be raised if the destination folder does not exist. Instead of handling
this implicitly in the catch of IOException and then continuing to
create the destination folder and try again, explicitly catch it and
create the destination folder. If any other IOException occurs, treat
it as an unexpected error and return FAILURE.

Subsequently, on the second attempt to move the temp file, if ANY kind
of IOException occurs, also consider this an unexpected error and
return FAILURE.

In both catch blocks for IOException, add logging at ERROR level.

Change-Id: I9de9ee3d2b368be36e02ee1c0daf8e844f7e46c8
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
---
 .../storage/file/ObjectDirectory.java         | 32 ++++++++-----------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
index 651cf1911..33f7853ab 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
@@ -21,6 +21,7 @@ import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.nio.file.AtomicMoveNotSupportedException;
 import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.StandardCopyOption;
 import java.text.MessageFormat;
 import java.util.ArrayList;
@@ -695,15 +696,19 @@ public class ObjectDirectory extends FileObjectDatabase {
 			LOG.error(e.getMessage(), e);
 			FileUtils.delete(tmp, FileUtils.RETRY);
 			return InsertLooseObjectResult.FAILURE;
+		} catch (NoSuchFileException e) {
+			// It's possible the directory doesn't exist yet as the object
+			// directories are always lazily created. Note that we try the
+			// rename/move first as the directory likely does exist.
+
+			// Create the directory
+			FileUtils.mkdir(dst.getParentFile(), true);
 		} catch (IOException e) {
-			// ignore
+			LOG.error(e.getMessage(), e);
+			FileUtils.delete(tmp, FileUtils.RETRY);
+			return InsertLooseObjectResult.FAILURE;
 		}
 
-		// Maybe the directory doesn't exist yet as the object
-		// directories are always lazily created. Note that we
-		// try the rename first as the directory likely does exist.
-		//
-		FileUtils.mkdir(dst.getParentFile(), true);
 		try {
 			Files.move(FileUtils.toPath(tmp), FileUtils.toPath(dst),
 					StandardCopyOption.ATOMIC_MOVE);
@@ -711,21 +716,10 @@ public class ObjectDirectory extends FileObjectDatabase {
 			unpackedObjectCache.add(id);
 			return InsertLooseObjectResult.INSERTED;
 		} catch (IOException e) {
-			LOG.debug(e.getMessage(), e);
-		}
-
-		if (!createDuplicate && has(id)) {
+			LOG.error(e.getMessage(), e);
 			FileUtils.delete(tmp, FileUtils.RETRY);
-			return InsertLooseObjectResult.EXISTS_PACKED;
+			return InsertLooseObjectResult.FAILURE;
 		}
-
-		// The object failed to be renamed into its proper
-		// location and it doesn't exist in the repository
-		// either. We really don't know what went wrong, so
-		// fail.
-		//
-		FileUtils.delete(tmp, FileUtils.RETRY);
-		return InsertLooseObjectResult.FAILURE;
 	}
 
 	boolean searchPacksAgain(PackList old) {