From 0b2d41b8584e16d6f7abeca92eaae326033b4489 Mon Sep 17 00:00:00 2001
From: Thomas Wolf <thomas.wolf@paranor.ch>
Date: Fri, 29 May 2020 21:57:37 +0200
Subject: [PATCH] Verify that the user home directory is valid

If the determination of the user home directory produces a Java File
object with an invalid path, spurious exceptions may occur at the
most inopportune moments anytime later. In the case in the linked bug
report, start-up of EGit failed, leading to numerous user-visible
problems in Eclipse.

So validate the return value of FS.userHomeImpl(). If converting that
File to a Path throws an exception, log the problem and fall back to
Java system property user.home. If that also is not valid, use null.

(A null user home directory is allowed by FS, and calling in Java
new File(null, "some_string") is fine and produces a File relative
to the current working directory.)

Bug: 563739
Change-Id: If9eec0f9a31a45bd815231706285c71b09f8cf56
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
---
 .../eclipse/jgit/internal/JGitText.properties |  2 ++
 .../org/eclipse/jgit/internal/JGitText.java   |  2 ++
 .../src/org/eclipse/jgit/util/FS.java         | 30 ++++++++++++++++++-
 3 files changed, 33 insertions(+), 1 deletion(-)

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 c9ca11b54..9dd632093 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -269,6 +269,7 @@ exceptionCaughtDuringExecutionOfTagCommand=Exception caught during execution of
 exceptionHookExecutionInterrupted=Execution of "{0}" hook interrupted.
 exceptionOccurredDuringAddingOfOptionToALogCommand=Exception occurred during adding of {0} as option to a Log command
 exceptionOccurredDuringReadingOfGIT_DIR=Exception occurred during reading of $GIT_DIR/{0}. {1}
+exceptionWhileFindingUserHome=Problem determining the user home directory, trying Java user.home
 exceptionWhileReadingPack=Exception caught while accessing pack file {0}, the pack file might be corrupt. Caught {1} consecutive errors while trying to read this pack.
 expectedACKNAKFoundEOF=Expected ACK/NAK, found EOF
 expectedACKNAKGot=Expected ACK/NAK, got: {0}
@@ -356,6 +357,7 @@ invalidGitdirRef = Invalid .git reference in file ''{0}''
 invalidGitModules=Invalid .gitmodules file
 invalidGitType=invalid git type: {0}
 invalidHexString=Invalid hex string: {0}
+invalidHomeDirectory=Invalid home directory: {0}
 invalidHooksPath=Invalid git config core.hooksPath = {0}
 invalidId=Invalid id: {0}
 invalidId0=Invalid id
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 ec2414d41..f18ecb921 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -297,6 +297,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String exceptionHookExecutionInterrupted;
 	/***/ public String exceptionOccurredDuringAddingOfOptionToALogCommand;
 	/***/ public String exceptionOccurredDuringReadingOfGIT_DIR;
+	/***/ public String exceptionWhileFindingUserHome;
 	/***/ public String exceptionWhileReadingPack;
 	/***/ public String expectedACKNAKFoundEOF;
 	/***/ public String expectedACKNAKGot;
@@ -384,6 +385,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String invalidGitModules;
 	/***/ public String invalidGitType;
 	/***/ public String invalidHexString;
+	/***/ public String invalidHomeDirectory;
 	/***/ public String invalidHooksPath;
 	/***/ public String invalidId;
 	/***/ public String invalidId0;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
index 988953b00..91574efec 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
@@ -1036,12 +1036,36 @@ public abstract class FS {
 	public File userHome() {
 		Holder<File> p = userHome;
 		if (p == null) {
-			p = new Holder<>(userHomeImpl());
+			p = new Holder<>(safeUserHomeImpl());
 			userHome = p;
 		}
 		return p.value;
 	}
 
+	private File safeUserHomeImpl() {
+		File home;
+		try {
+			home = userHomeImpl();
+			if (home != null) {
+				home.toPath();
+				return home;
+			}
+		} catch (RuntimeException e) {
+			LOG.error(JGitText.get().exceptionWhileFindingUserHome, e);
+		}
+		home = defaultUserHomeImpl();
+		if (home != null) {
+			try {
+				home.toPath();
+				return home;
+			} catch (InvalidPathException e) {
+				LOG.error(MessageFormat
+						.format(JGitText.get().invalidHomeDirectory, home), e);
+			}
+		}
+		return null;
+	}
+
 	/**
 	 * Set the user's home directory location.
 	 *
@@ -1081,6 +1105,10 @@ public abstract class FS {
 	 * @return the user's home directory; null if the user does not have one.
 	 */
 	protected File userHomeImpl() {
+		return defaultUserHomeImpl();
+	}
+
+	private File defaultUserHomeImpl() {
 		final String home = AccessController.doPrivileged(
 				(PrivilegedAction<String>) () -> System.getProperty("user.home") //$NON-NLS-1$
 		);