Support LFS Server URL without .git suffix

According to Git LFS documentation, URLs with and without .git suffix
should be supported. By default, Git LFS will append .git/info/lfs to
the end of a Git remote URL. To build the LFS server URL it will use:

Git Remote: https://git-server.com/foo/bar
LFS Server: https://git-server.com/foo/bar.git/info/lfs

Git Remote: https://git-server.com/foo/bar.git
LFS Server: https://git-server.com/foo/bar.git/info/lfs

Fix the LfsConnectionFactory accordingly. Move a utility method to
add the ".git" suffix if not present yet from FileResolver to
StringUtils and use it.

Bug: 578621
Change-Id: I8d3645872d5f03bb8e82c9c73647adb3e81ce484
Signed-off-by: Nail Samatov <sanail@yandex.ru>
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

5 files changed, 181 insertions, 19 deletions

diff --git a/org.eclipse.jgit.lfs.test/META-INF/MANIFEST.MF b/org.eclipse.jgit.lfs.test/META-INF/MANIFEST.MF
index b70fca6188..7aafbe6d26 100644
--- a/org.eclipse.jgit.lfs.test/META-INF/MANIFEST.MF
+++ b/org.eclipse.jgit.lfs.test/META-INF/MANIFEST.MF
@@ -13,9 +13,11 @@ Import-Package: org.eclipse.jgit.api;version="[6.1.0,6.2.0)",
  org.eclipse.jgit.junit;version="[6.1.0,6.2.0)",
  org.eclipse.jgit.lfs;version="[6.1.0,6.2.0)",
  org.eclipse.jgit.lfs.errors;version="[6.1.0,6.2.0)",
+ org.eclipse.jgit.lfs.internal;version="[6.1.0,6.2.0)",
  org.eclipse.jgit.lfs.lib;version="[6.1.0,6.2.0)",
  org.eclipse.jgit.lib;version="[6.1.0,6.2.0)",
  org.eclipse.jgit.revwalk;version="[6.1.0,6.2.0)",
+ org.eclipse.jgit.transport;version="[6.1.0,6.2.0)",
  org.eclipse.jgit.treewalk;version="[6.1.0,6.2.0)",
  org.eclipse.jgit.treewalk.filter;version="[6.1.0,6.2.0)",
  org.eclipse.jgit.util;version="[6.1.0,6.2.0)",
diff --git a/org.eclipse.jgit.lfs.test/tst/org/eclipse/jgit/lfs/internal/LfsConnectionFactoryTest.java b/org.eclipse.jgit.lfs.test/tst/org/eclipse/jgit/lfs/internal/LfsConnectionFactoryTest.java
new file mode 100644
index 0000000000..c7bd48e12c
--- /dev/null
+++ b/org.eclipse.jgit.lfs.test/tst/org/eclipse/jgit/lfs/internal/LfsConnectionFactoryTest.java
@@ -0,0 +1,132 @@
+/*
+ * Copyright (C) 2022 Nail Samatov <sanail@yandex.ru> and others
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Distribution License v. 1.0 which is available at
+ * https://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+package org.eclipse.jgit.lfs.internal;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+
+import java.util.TreeMap;
+
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.RemoteAddCommand;
+import org.eclipse.jgit.attributes.FilterCommandRegistry;
+import org.eclipse.jgit.junit.RepositoryTestCase;
+import org.eclipse.jgit.lfs.CleanFilter;
+import org.eclipse.jgit.lfs.Protocol;
+import org.eclipse.jgit.lfs.SmudgeFilter;
+import org.eclipse.jgit.lfs.errors.LfsConfigInvalidException;
+import org.eclipse.jgit.lfs.lib.Constants;
+import org.eclipse.jgit.lib.ConfigConstants;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.StoredConfig;
+import org.eclipse.jgit.transport.URIish;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class LfsConnectionFactoryTest extends RepositoryTestCase {
+
+	private static final String SMUDGE_NAME = org.eclipse.jgit.lib.Constants.BUILTIN_FILTER_PREFIX
+			+ Constants.ATTR_FILTER_DRIVER_PREFIX
+			+ org.eclipse.jgit.lib.Constants.ATTR_FILTER_TYPE_SMUDGE;
+
+	private static final String CLEAN_NAME = org.eclipse.jgit.lib.Constants.BUILTIN_FILTER_PREFIX
+			+ Constants.ATTR_FILTER_DRIVER_PREFIX
+			+ org.eclipse.jgit.lib.Constants.ATTR_FILTER_TYPE_CLEAN;
+
+	private Git git;
+
+	@BeforeClass
+	public static void installLfs() {
+		FilterCommandRegistry.register(SMUDGE_NAME, SmudgeFilter.FACTORY);
+		FilterCommandRegistry.register(CLEAN_NAME, CleanFilter.FACTORY);
+	}
+
+	@AfterClass
+	public static void removeLfs() {
+		FilterCommandRegistry.unregister(SMUDGE_NAME);
+		FilterCommandRegistry.unregister(CLEAN_NAME);
+	}
+
+	@Override
+	@Before
+	public void setUp() throws Exception {
+		super.setUp();
+		git = new Git(db);
+	}
+
+	@Test
+	public void lfsUrlFromRemoteUrlWithDotGit() throws Exception {
+		addRemoteUrl("https://localhost/repo.git");
+
+		String lfsUrl = LfsConnectionFactory.getLfsUrl(db,
+				Protocol.OPERATION_DOWNLOAD,
+				new TreeMap<>());
+		assertEquals("https://localhost/repo.git/info/lfs", lfsUrl);
+	}
+
+	@Test
+	public void lfsUrlFromRemoteUrlWithoutDotGit() throws Exception {
+		addRemoteUrl("https://localhost/repo");
+
+		String lfsUrl = LfsConnectionFactory.getLfsUrl(db,
+				Protocol.OPERATION_DOWNLOAD,
+				new TreeMap<>());
+		assertEquals("https://localhost/repo.git/info/lfs", lfsUrl);
+	}
+
+	@Test
+	public void lfsUrlFromLocalConfig() throws Exception {
+		addRemoteUrl("https://localhost/repo");
+
+		StoredConfig cfg = ((Repository) db).getConfig();
+		cfg.setString(ConfigConstants.CONFIG_SECTION_LFS,
+				null,
+				ConfigConstants.CONFIG_KEY_URL,
+				"https://localhost/repo/lfs");
+		cfg.save();
+
+		String lfsUrl = LfsConnectionFactory.getLfsUrl(db,
+				Protocol.OPERATION_DOWNLOAD,
+				new TreeMap<>());
+		assertEquals("https://localhost/repo/lfs", lfsUrl);
+	}
+
+	@Test
+	public void lfsUrlFromOriginConfig() throws Exception {
+		addRemoteUrl("https://localhost/repo");
+
+		StoredConfig cfg = ((Repository) db).getConfig();
+		cfg.setString(ConfigConstants.CONFIG_SECTION_LFS,
+				org.eclipse.jgit.lib.Constants.DEFAULT_REMOTE_NAME,
+				ConfigConstants.CONFIG_KEY_URL,
+				"https://localhost/repo/lfs");
+		cfg.save();
+
+		String lfsUrl = LfsConnectionFactory.getLfsUrl(db,
+				Protocol.OPERATION_DOWNLOAD,
+				new TreeMap<>());
+		assertEquals("https://localhost/repo/lfs", lfsUrl);
+	}
+
+	@Test
+	public void lfsUrlNotConfigured() throws Exception {
+		assertThrows(LfsConfigInvalidException.class, () -> LfsConnectionFactory
+				.getLfsUrl(db, Protocol.OPERATION_DOWNLOAD, new TreeMap<>()));
+	}
+
+	private void addRemoteUrl(String remotUrl) throws Exception {
+		RemoteAddCommand add = git.remoteAdd();
+		add.setUri(new URIish(remotUrl));
+		add.setName(org.eclipse.jgit.lib.Constants.DEFAULT_REMOTE_NAME);
+		add.call();
+	}
+}
diff --git a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsConnectionFactory.java b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsConnectionFactory.java
index e221913bea..5a17d411c7 100644
--- a/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsConnectionFactory.java
+++ b/org.eclipse.jgit.lfs/src/org/eclipse/jgit/lfs/internal/LfsConnectionFactory.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017, Markus Duft <markus.duft@ssi-schaefer.com> and others
+ * Copyright (C) 2017, 2022 Markus Duft <markus.duft@ssi-schaefer.com> and others
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Distribution License v. 1.0 which is available at
@@ -39,6 +39,7 @@ import org.eclipse.jgit.transport.URIish;
 import org.eclipse.jgit.transport.http.HttpConnection;
 import org.eclipse.jgit.util.HttpSupport;
 import org.eclipse.jgit.util.SshSupport;
+import org.eclipse.jgit.util.StringUtils;
 
 /**
  * Provides means to get a valid LFS connection for a given repository.
@@ -64,7 +65,7 @@ public class LfsConnectionFactory {
 	 *            be used for
 	 * @param purpose
 	 *            the action, e.g. Protocol.OPERATION_DOWNLOAD
-	 * @return the url for the lfs server. e.g.
+	 * @return the connection for the lfs server. e.g.
 	 *         "https://github.com/github/git-lfs.git/info/lfs"
 	 * @throws IOException
 	 */
@@ -92,7 +93,24 @@ public class LfsConnectionFactory {
 		return connection;
 	}
 
-	private static String getLfsUrl(Repository db, String purpose,
+	/**
+	 * Get LFS Server URL.
+	 *
+	 * @param db
+	 *            the repository to work with
+	 * @param purpose
+	 *            the action, e.g. Protocol.OPERATION_DOWNLOAD
+	 * @param additionalHeaders
+	 *            additional headers that can be used to connect to LFS server
+	 * @return the URL for the LFS server. e.g.
+	 *         "https://github.com/github/git-lfs.git/info/lfs"
+	 * @throws LfsConfigInvalidException
+	 *             if the LFS config is invalid
+	 * @see <a href=
+	 *      "https://github.com/git-lfs/git-lfs/blob/main/docs/api/server-discovery.md">
+	 *      Server Discovery documentation</a>
+	 */
+	static String getLfsUrl(Repository db, String purpose,
 			Map<String, String> additionalHeaders)
 			throws LfsConfigInvalidException {
 		StoredConfig config = db.getConfig();
@@ -125,8 +143,6 @@ public class LfsConnectionFactory {
 						| CommandFailedException e) {
 					ex = e;
 				}
-			} else {
-				lfsUrl = lfsUrl + Protocol.INFO_LFS_ENDPOINT;
 			}
 		}
 		if (lfsUrl == null) {
@@ -149,7 +165,8 @@ public class LfsConnectionFactory {
 			additionalHeaders.putAll(action.header);
 			return action.href;
 		}
-		return remoteUrl + Protocol.INFO_LFS_ENDPOINT;
+		return StringUtils.nameWithDotGit(remoteUrl)
+				+ Protocol.INFO_LFS_ENDPOINT;
 	}
 
 	private static Protocol.ExpiringAction getSshAuthentication(
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/resolver/FileResolver.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/resolver/FileResolver.java
index 3d15ef5e72..046f395049 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/resolver/FileResolver.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/resolver/FileResolver.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2010, Google Inc. and others
+ * Copyright (C) 2009-2022, Google Inc. and others
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Distribution License v. 1.0 which is available at
@@ -18,11 +18,11 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArrayList;
 
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.RepositoryCache;
 import org.eclipse.jgit.lib.RepositoryCache.FileKey;
 import org.eclipse.jgit.util.FS;
+import org.eclipse.jgit.util.StringUtils;
 
 /**
  * Default resolver serving from the local filesystem.
@@ -67,7 +67,7 @@ public class FileResolver<C> implements RepositoryResolver<C> {
 		if (isUnreasonableName(name))
 			throw new RepositoryNotFoundException(name);
 
-		Repository db = exports.get(nameWithDotGit(name));
+		Repository db = exports.get(StringUtils.nameWithDotGit(name));
 		if (db != null) {
 			db.incrementOpen();
 			return db;
@@ -154,7 +154,7 @@ public class FileResolver<C> implements RepositoryResolver<C> {
 	 *            the repository instance.
 	 */
 	public void exportRepository(String name, Repository db) {
-		exports.put(nameWithDotGit(name), db);
+		exports.put(StringUtils.nameWithDotGit(name), db);
 	}
 
 	/**
@@ -197,12 +197,6 @@ public class FileResolver<C> implements RepositoryResolver<C> {
 			return false;
 	}
 
-	private static String nameWithDotGit(String name) {
-		if (name.endsWith(Constants.DOT_GIT_EXT))
-			return name;
-		return name + Constants.DOT_GIT_EXT;
-	}
-
 	private static boolean isUnreasonableName(String name) {
 		if (name.length() == 0)
 			return true; // no empty paths
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/StringUtils.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/StringUtils.java
index 8ab13385e0..917add3609 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/StringUtils.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/StringUtils.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2010, Google Inc. and others
+ * Copyright (C) 2009-2022, Google Inc. and others
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Distribution License v. 1.0 which is available at
@@ -15,6 +15,7 @@ import java.util.Collection;
 
 import org.eclipse.jgit.annotations.NonNull;
 import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.lib.Constants;
 
 /**
  * Miscellaneous string comparison utility methods.
@@ -37,6 +38,10 @@ public final class StringUtils {
 			LC[c] = (char) ('a' + (c - 'A'));
 	}
 
+	private StringUtils() {
+		// Do not create instances
+	}
+
 	/**
 	 * Convert the input to lowercase.
 	 * <p>
@@ -269,8 +274,20 @@ public final class StringUtils {
 		return sb.toString();
 	}
 
-	private StringUtils() {
-		// Do not create instances
+	/**
+	 * Appends {@link Constants#DOT_GIT_EXT} unless the given name already ends
+	 * with that suffix.
+	 *
+	 * @param name
+	 *            to complete
+	 * @return the name ending with {@link Constants#DOT_GIT_EXT}
+	 * @since 6.1
+	 */
+	public static String nameWithDotGit(String name) {
+		if (name.endsWith(Constants.DOT_GIT_EXT)) {
+			return name;
+		}
+		return name + Constants.DOT_GIT_EXT;
 	}
 
 	/**