Store objects for an upstream key with a dot in it, such as gitlab.com [patch]

GitLfsCache.Tests/Integration/ProxyFlowTests.cs

@@ -332,6 +332,38 @@
 		Assert.IsTrue(fixture.Store.Exists("github", oid), "A pushed object should be cached for the next fetch.");
 	}
 
+	[TestMethod]
+	public async Task DottedUpstreamKey_DownloadsAndUploadsAreStored()
+	{
+		const string dottedPath = "/gitlab.com/owner/repo.git/info/lfs";
+		await using ProxyFixture fixture = await ProxyFixture.StartAsync(new(StringComparer.Ordinal)
+		{
+			["GitLfsCache:Upstreams:gitlab.com:BaseUrl"] = "https://upstream.example",
+			["GitLfsCache:Upstreams:gitlab.com:Repositories:0"] = "**",
+		});
+		using HttpClient client = fixture.Client;
+
+		(byte[] downloaded, string downloadOid) = Object("fetched through a dotted upstream");
+		fixture.Upstream.AddObject(downloadOid, downloaded);
+		JsonNode downloadBatch = await PostBatchAsync(fixture, "download", downloadOid, downloaded.Length, lfsPath: dottedPath);
+
+		using HttpResponseMessage download = await client.GetAsync(Relative(HrefOf(downloadBatch, "download")));
+
+		Assert.AreEqual(HttpStatusCode.OK, download.StatusCode);
+		CollectionAssert.AreEqual(downloaded, await download.Content.ReadAsByteArrayAsync());
+		Assert.IsTrue(fixture.Store.Exists("gitlab.com", downloadOid));
+
+		(byte[] uploaded, string uploadOid) = Object("pushed through a dotted upstream");
+		JsonNode uploadBatch = await PostBatchAsync(fixture, "upload", uploadOid, uploaded.Length, lfsPath: dottedPath);
+		using ByteArrayContent body = new(uploaded);
+
+		using HttpResponseMessage upload = await client.PutAsync(Relative(HrefOf(uploadBatch, "upload")), body);
+
+		Assert.AreEqual(HttpStatusCode.OK, upload.StatusCode);
+		CollectionAssert.AreEqual(uploaded, fixture.Upstream.Uploaded[uploadOid]);
+		Assert.IsTrue(fixture.Store.Exists("gitlab.com", uploadOid));
+	}
+
 	/// <summary>
 	/// Puts a file where the upstream's staging directory belongs, so no staging file can be opened.
 	/// </summary>

GitLfsCache.Tests/Storage/ObjectStoreTests.cs

@@ -263,14 +263,62 @@
 	}
 
 	[TestMethod]
+	public void OpenStaging_EmptyUpstream_Throws()
+	{
+		(ObjectStore store, _, _) = Create();
+
+		Assert.ThrowsExactly<ArgumentException>(() => store.OpenStaging(string.Empty));
+	}
+
+	[TestMethod]
+	[DataRow("gitlab.com")]
 	[DataRow("../escape")]
 	[DataRow("with/slash")]
-	[DataRow("")]
-	public void OpenStaging_MalformedUpstream_Throws(string upstream)
+	[DataRow("with\\backslash")]
+	[DataRow("..")]
+	[DataRow("dé.example")]
+	public async Task AnyNonEmptyUpstream_IsStoredInOneDirectoryUnderTheRootAndReadBack(string upstream)
 	{
-		(ObjectStore store, _, _) = Create();
+		(ObjectStore store, MockFileSystem fileSystem, _) = Create();
+
+		string oid = await StoreAsync(store, upstream, $"stored under {upstream}");
+
+		Assert.IsTrue(store.Exists(upstream, oid));
+		using (Stream? read = store.OpenRead(upstream, oid, out _))
+		{
+			Assert.IsNotNull(read);
+		}
+
+		string directory = fileSystem.Directory.EnumerateDirectories(Root).Single();
+		Assert.AreEqual(Root, fileSystem.Path.GetDirectoryName(directory), "The key must not name a directory outside the root.");
+		Assert.AreEqual(ObjectStore.DirectoryNameFor(upstream), fileSystem.Path.GetFileName(directory));
+		Assert.AreEqual(upstream, store.Enumerate().Single().Upstream);
+	}
+
+	[TestMethod]
+	public void DirectoryNameFor_DottedKey_EscapesOnlyTheDot()
+	{
+		Assert.AreEqual("gitlab%2Ecom", ObjectStore.DirectoryNameFor("gitlab.com"));
+	}
+
+	[TestMethod]
+	[DataRow("github")]
+	[DataRow("My_Upstream-2")]
+	public void DirectoryNameFor_PlainKey_IsTheKeyItself(string upstream)
+	{
+		// The layout a store written before escaping existed already has, so it stays readable.
+		Assert.AreEqual(upstream, ObjectStore.DirectoryNameFor(upstream));
+	}
+
+	[TestMethod]
+	public void DirectoryNameFor_KeysThatLookAlikeOnceEscaped_StayDistinct()
+	{
+		string[] keys = ["gitlab.com", "gitlab%2Ecom", "gitlab%252Ecom", "gitlab_com", "gitlab-com"];
+
+		string[] names = [.. keys.Select(ObjectStore.DirectoryNameFor)];
 
-		Assert.ThrowsExactly<ArgumentException>(() => store.OpenStaging(upstream));
+		Assert.HasCount(keys.Length, names.Distinct(StringComparer.Ordinal).ToList());
+		CollectionAssert.AreEqual(keys, names.Select(ObjectStore.UpstreamFor).ToArray());
 	}
 
 	[TestMethod]
@@ -287,7 +335,7 @@
 	}
 
 	[TestMethod]
 	public void Touch_MissingObject_DoesNotThrow()
 	{
 		(ObjectStore store, _, _) = Create();
 

GitLfsCache/Storage/ObjectStore.cs

@@ -3,7 +3,9 @@
 namespace ktsu.GitLfsCache.Storage;
 
 using System.Diagnostics.CodeAnalysis;
+using System.Globalization;
 using System.IO.Abstractions;
+using System.Text;
 using ktsu.GitLfsCache.Configuration;
 using ktsu.Semantics.Paths;
 using ktsu.Semantics.Strings;
@@ -15,7 +17,8 @@ namespace ktsu.GitLfsCache.Storage;
 /// </summary>
 /// <remarks>
 /// Layout is <c>{root}/{upstream}/objects/{first two}/{next two}/{oid}</c> with staging under
-/// <c>{root}/{upstream}/staging</c>. Staging shares the volume with the objects so publishing is an
+/// <c>{root}/{upstream}/staging</c>, where <c>{upstream}</c> is the key's directory name (see
+/// <see cref="DirectoryNameFor"/>). Staging shares the volume with the objects so publishing is an
 /// atomic rename, and the two-level fan-out mirrors the git-lfs client's own layout, which keeps
 /// directory sizes reasonable into the hundreds of thousands of objects.
 /// <para>
@@ -225,7 +228,7 @@ public IEnumerable<StoredObject> Enumerate()
 
 		foreach (string upstreamDirectory in fileSystem.Directory.EnumerateDirectories(_root))
 		{
-			string upstream = fileSystem.Path.GetFileName(upstreamDirectory);
+			string upstream = UpstreamFor(fileSystem.Path.GetFileName(upstreamDirectory));
 			string objectsDirectory = fileSystem.Path.Combine(upstreamDirectory, ObjectsDirectoryName);
 
 			if (!fileSystem.Directory.Exists(objectsDirectory))
@@ -346,11 +349,11 @@ private bool TryDeleteFile(AbsoluteFilePath path)
 	}
 
 	private AbsoluteFilePath ObjectPath(string upstream, string oid) => fileSystem.Path
-		.Combine(_root, upstream, ObjectsDirectoryName, oid[..2], oid[2..4], oid)
+		.Combine(_root, DirectoryNameFor(upstream), ObjectsDirectoryName, oid[..2], oid[2..4], oid)
 		.As<AbsoluteFilePath>();
 
 	private AbsoluteDirectoryPath StagingDirectory(string upstream) => fileSystem.Path
-		.Combine(_root, upstream, StagingDirectoryName)
+		.Combine(_root, DirectoryNameFor(upstream), StagingDirectoryName)
 		.As<AbsoluteDirectoryPath>();
 
 	/// <summary>
@@ -364,9 +367,56 @@ private AbsoluteDirectoryPath StagingDirectory(string upstream) => fileSystem.Pa
 	private static bool IsValidOid([NotNullWhen(true)] string? oid) =>
 		oid is not null && oid.Length == OidLength && oid.All(char.IsAsciiHexDigitLower);
 
-	private static bool IsValidUpstream([NotNullWhen(true)] string? upstream) =>
-		!string.IsNullOrEmpty(upstream) && upstream.All(IsUpstreamCharacter);
+	private static bool IsValidUpstream([NotNullWhen(true)] string? upstream) => !string.IsNullOrEmpty(upstream);
 
-	private static bool IsUpstreamCharacter(char character) =>
+	/// <summary>
+	/// Maps an upstream key to the name of the directory its tree lives in.
+	/// </summary>
+	/// <remarks>
+	/// A key made only of ASCII letters, digits, <c>-</c> and <c>_</c> is its own directory name, which
+	/// is the layout every store written before other keys were supported already has. Any other key,
+	/// such as the natural <c>gitlab.com</c>, has each byte of its UTF-8 form outside that set written
+	/// as <c>%XX</c>. That is deterministic and reversible, so no two keys share a directory, and an
+	/// escaped name always contains a <c>%</c>, which a plain one never does. It also leaves nothing a
+	/// filesystem would read as a separator or a parent reference, so no key can name a directory
+	/// outside the store root.
+	/// </remarks>
+	/// <param name="upstream">The upstream key.</param>
+	/// <returns>The directory name.</returns>
+	internal static string DirectoryNameFor(string upstream)
+	{
+		if (upstream.All(IsPlainCharacter))
+		{
+			return upstream;
+		}
+
+		StringBuilder name = new(upstream.Length * 3);
+
+		foreach (byte value in Encoding.UTF8.GetBytes(upstream))
+		{
+			if (value < 0x80 && IsPlainCharacter((char)value))
+			{
+				name.Append((char)value);
+			}
+			else
+			{
+				name.Append('%').Append(value.ToString("X2", CultureInfo.InvariantCulture));
+			}
+		}
+
+		return name.ToString();
+	}
+
+	/// <summary>
+	/// Recovers the upstream key from a directory name <see cref="DirectoryNameFor"/> produced.
+	/// </summary>
+	/// <param name="directoryName">The directory name.</param>
+	/// <returns>The upstream key.</returns>
+	internal static string UpstreamFor(string directoryName) =>
+		directoryName.Contains('%', StringComparison.Ordinal)
+			? Uri.UnescapeDataString(directoryName)
+			: directoryName;
+
+	private static bool IsPlainCharacter(char character) =>
 		char.IsAsciiLetterOrDigit(character) || character is '-' or '_';
 }

docs/superpowers/specs/2026-08-18-gitlfscache-design.md

@@ -125,6 +125,8 @@ Layout, one tree per upstream key, with `ktsu.Semantics.Paths` types for the roo
 <root>/<upstream>/staging/<guid>.tmp
 ```
 
+`<upstream>` is the key itself when it is made only of ASCII letters, digits, `-` and `_`. Any other key, such as `gitlab.com`, has each UTF-8 byte outside that set written as `%XX` (`gitlab%2Ecom`), which is reversible, cannot collide with a plain name, and cannot reach outside the root.
+
 Staging sits on the same volume as the objects so publishing is an atomic rename. The two-level fan-out mirrors the git-lfs client's own layout and keeps directory sizes reasonable into the hundreds of thousands of objects.
 
 Namespacing per upstream duplicates bytes when two upstreams hold the same object, and content addressing means those bytes are identical. The duplication is accepted to bound the blast radius if the token codec is ever wrong. A single shared tree is recorded as a deferred option, not a configuration flag.