thodg/got

diff --git a/lib/worktree.c b/lib/worktree.c
index 8d06a93..e9d864e 100644
--- a/lib/worktree.c
+++ b/lib/worktree.c
@@ -1029,6 +1029,48 @@ install_blob(struct got_worktree *worktree, const char *ondisk_path,
     struct got_repository *repo,
     got_worktree_checkout_cb progress_cb, void *progress_arg);
 
+/*
+ * This function assumes that the provided symlink target points at a
+ * safe location in the work tree!
+ */
+static const struct got_error *
+replace_existing_symlink(const char *ondisk_path, const char *target_path,
+    size_t target_len)
+{
+	const struct got_error *err = NULL;
+	ssize_t elen;
+	char etarget[PATH_MAX];
+	int fd;
+
+	/*
+	 * "Bad" symlinks (those pointing outside the work tree or into the
+	 * .got directory) are installed in the work tree as a regular file
+	 * which contains the bad symlink target path.
+	 * The new symlink target has already been checked for safety by our
+	 * caller. If we can successfully open a regular file then we simply
+	 * replace this file with a symlink below.
+	 */
+	fd = open(ondisk_path, O_RDWR | O_EXCL | O_NOFOLLOW);
+	if (fd == -1) {
+		if (errno != ELOOP)
+			return got_error_from_errno2("open", ondisk_path);
+
+		/* We are updating an existing on-disk symlink. */
+		elen = readlink(ondisk_path, etarget, sizeof(etarget));
+		if (elen == -1)
+			return got_error_from_errno2("readlink", ondisk_path);
+
+		if (elen == target_len &&
+		    memcmp(etarget, target_path, target_len) == 0)
+			return NULL; /* nothing to do */
+	}
+
+	err = update_symlink(ondisk_path, target_path, target_len);
+	if (fd != -1 && close(fd) == -1 && err == NULL)
+		err = got_error_from_errno2("close", ondisk_path);
+	return err;
+}
+
 static const struct got_error *
 install_symlink(struct got_worktree *worktree, const char *ondisk_path,
     const char *path, mode_t te_mode, mode_t st_mode,
@@ -1131,40 +1173,15 @@ install_symlink(struct got_worktree *worktree, const char *ondisk_path,
 
 	if (symlink(target_path, ondisk_path) == -1) {
 		if (errno == EEXIST) {
-			struct stat sb;
-			ssize_t elen;
-			char etarget[PATH_MAX];
-			if (lstat(ondisk_path, &sb) == -1) {
-				err = got_error_from_errno2("lstat",
-				    ondisk_path);
-				goto done;
-			}
-			if (!S_ISLNK(sb.st_mode)) {
-				err = got_error_path(ondisk_path,
-				    GOT_ERR_FILE_OBSTRUCTED);
-				goto done;
-			}
-			elen = readlink(ondisk_path, etarget, sizeof(etarget));
-			if (elen == -1) {
-				err = got_error_from_errno2("readlink",
-				    ondisk_path);
-				goto done;
-			}
-			if (elen == target_len &&
-			    memcmp(etarget, target_path, target_len) == 0) {
-				err = NULL; /* nothing to do */
-				goto done;
-			} else {
-				err = update_symlink(ondisk_path, target_path,
-				    target_len);
-				if (err)
-					goto done;
-				if (progress_cb) {
-					err = (*progress_cb)(progress_arg,
-					    GOT_STATUS_UPDATE, path);
-				}
+			err = replace_existing_symlink(ondisk_path,
+			    target_path, target_len);
+			if (err)
 				goto done;
+			if (progress_cb) {
+				err = (*progress_cb)(progress_arg,
+				    GOT_STATUS_UPDATE, path);
 			}
+			goto done; /* Nothing else to do. */
 		}
 
 		if (errno == ENOENT) {
diff --git a/regress/cmdline/commit.sh b/regress/cmdline/commit.sh
index 55a121e..26fadaa 100755
--- a/regress/cmdline/commit.sh
+++ b/regress/cmdline/commit.sh
@@ -1034,6 +1034,97 @@ function test_commit_symlink {
 	test_done "$testroot" "0"
 }
 
+function test_commit_fix_bad_symlink {
+	local testroot=`test_init commit_fix_bad_symlink`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		echo "got checkout failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	(cd $testroot/wt && ln -s /etc/passwd passwd.link)
+	(cd $testroot/wt && got add passwd.link > /dev/null)
+
+	(cd $testroot/wt && got commit -m 'commit bad symlink' > $testroot/stdout)
+
+	if [ -h $testroot/wt/passwd.link ]; then
+		echo "passwd.link is a symlink but should be a regular file" >&2
+		test_done "$testroot" "1"
+		return 1
+	fi
+
+	# create another work tree which will contain the "bad" symlink
+	got checkout $testroot/repo $testroot/wt2 > /dev/null
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		echo "got checkout failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# change "bad" symlink back into a "good" symlink
+	(cd $testroot/wt && ln -sfh alpha passwd.link)
+
+	(cd $testroot/wt && got commit -m 'fix bad symlink' \
+		> $testroot/stdout)
+
+	local head_rev=`git_show_head $testroot/repo`
+	echo "M  passwd.link" > $testroot/stdout.expected
+	echo "Created commit $head_rev" >> $testroot/stdout.expected
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	if ! [ -h $testroot/wt/passwd.link ]; then
+		echo 'passwd.link is not a symlink' >&2
+		test_done "$testroot" 1
+		return 1
+	fi
+
+	readlink $testroot/wt/passwd.link > $testroot/stdout
+	echo "alpha" > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		return 1
+	fi
+
+	# Update the other work tree; the bad symlink should be fixed
+	(cd $testroot/wt2 && got update > /dev/null)
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		echo "got checkout failed unexpectedly" >&2
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	if ! [ -h $testroot/wt2/passwd.link ]; then
+		echo 'passwd.link is not a symlink' >&2
+		test_done "$testroot" 1
+		return 1
+	fi
+
+	readlink $testroot/wt2/passwd.link > $testroot/stdout
+	echo "alpha" > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		return 1
+	fi
+
+	test_done "$testroot" "0"
+}
+
 run_test test_commit_basic
 run_test test_commit_new_subdir
 run_test test_commit_subdir
@@ -1055,3 +1146,4 @@ run_test test_commit_xbit_change
 run_test test_commit_normalizes_filemodes
 run_test test_commit_with_unrelated_submodule
 run_test test_commit_symlink
+run_test test_commit_fix_bad_symlink