From cd765bc12ed7753275277528b6360aed85269a61 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 20:33:51 +0000 Subject: [PATCH] Key the cache by the configured upstream spelling, not the request's [minor] Upstream keys resolve regardless of case, but the request's own spelling was then used as the identity for store paths, fetch coalescing, transfer tokens, lock snapshots and metrics. On a case-sensitive filesystem a client addressing /GitHub/ and another addressing /github/ each filled their own cold cache and fetched the same object from upstream twice. IUpstreamRegistry gains a TryResolve overload that reports the key as configured (a default implementation keeps existing implementers working), and the handler replaces the route's upstream with it right after resolution, so everything downstream shares one identity. Fixes #49 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Hnw6WDDjwQ8HvxinnWS9CF --- .../Integration/ProxyFlowTests.cs | 26 +++++++++++++++++-- .../Upstreams/UpstreamRegistryTests.cs | 16 ++++++++++++ GitLfsCache/Endpoints/GitLfsCacheHandler.cs | 7 ++++- GitLfsCache/Upstreams/IUpstreamRegistry.cs | 19 ++++++++++++++ GitLfsCache/Upstreams/UpstreamRegistry.cs | 15 +++++++---- 5 files changed, 75 insertions(+), 8 deletions(-) diff --git a/GitLfsCache.Tests/Integration/ProxyFlowTests.cs b/GitLfsCache.Tests/Integration/ProxyFlowTests.cs index dcc58e5..76c30d8 100644 --- a/GitLfsCache.Tests/Integration/ProxyFlowTests.cs +++ b/GitLfsCache.Tests/Integration/ProxyFlowTests.cs @@ -33,13 +33,14 @@ private static async Task PostBatchAsync( string operation, string oid, long size, - string? authorization = "Basic dXNlcjp0b2tlbg==") + string? authorization = "Basic dXNlcjp0b2tlbg==", + string lfsPath = LfsPath) { using HttpClient client = fixture.Client; using StringContent body = BatchRequest(operation, oid, size); - using HttpRequestMessage request = new(HttpMethod.Post, $"{LfsPath}/objects/batch") + using HttpRequestMessage request = new(HttpMethod.Post, $"{lfsPath}/objects/batch") { Content = body, }; @@ -144,6 +145,27 @@ public async Task Download_ColdThenWarm_FetchesUpstreamOnceAndServesFromTheStore Assert.AreEqual(1, fixture.Upstream.FetchCount(oid)); } + [TestMethod] + public async Task Download_UpstreamKeyCasingDiffers_SharesOneCacheUnderTheConfiguredKey() + { + await using ProxyFixture fixture = await ProxyFixture.StartAsync(); + (byte[] content, string oid) = Object("one object, two spellings"); + fixture.Upstream.AddObject(oid, content); + using HttpClient client = fixture.Client; + + // The fixture configures the upstream as "github"; this client addresses it as "GitHub". + JsonNode mixedCaseBatch = await PostBatchAsync(fixture, "download", oid, content.Length, lfsPath: "/GitHub/owner/repo.git/info/lfs"); + string mixedCaseHref = HrefOf(mixedCaseBatch, "download"); + CollectionAssert.AreEqual(content, await client.GetByteArrayAsync(Relative(mixedCaseHref))); + + JsonNode configuredBatch = await PostBatchAsync(fixture, "download", oid, content.Length); + CollectionAssert.AreEqual(content, await client.GetByteArrayAsync(Relative(HrefOf(configuredBatch, "download")))); + + Assert.AreEqual(1, fixture.Upstream.FetchCount(oid), "The second spelling should be served from the first spelling's cache"); + Assert.StartsWith($"https://cache.example/github/owner/repo.git/info/lfs/objects/{oid}?t=", mixedCaseHref); + Assert.IsTrue(fixture.Store.Exists("github", oid)); + } + [TestMethod] public async Task Download_NeverShortCircuitsTheBatchCall() { diff --git a/GitLfsCache.Tests/Upstreams/UpstreamRegistryTests.cs b/GitLfsCache.Tests/Upstreams/UpstreamRegistryTests.cs index 87dde8f..f98fbb0 100644 --- a/GitLfsCache.Tests/Upstreams/UpstreamRegistryTests.cs +++ b/GitLfsCache.Tests/Upstreams/UpstreamRegistryTests.cs @@ -32,6 +32,22 @@ public void TryResolve_KeyCasingDiffers_StillResolves() Assert.AreEqual(new Uri("https://github.com"), baseUrl); } + [TestMethod] + public void TryResolve_KeyCasingDiffers_ReportsTheConfiguredKey() + { + Assert.IsTrue(Registry().TryResolve("GitHub", out string canonicalKey, out Uri? baseUrl)); + Assert.AreEqual("github", canonicalKey); + Assert.AreEqual(new Uri("https://github.com"), baseUrl); + } + + [TestMethod] + public void TryResolve_UnknownKey_ReportsTheKeyUnchanged() + { + Assert.IsFalse(Registry().TryResolve("GitLab", out string canonicalKey, out Uri? baseUrl)); + Assert.AreEqual("GitLab", canonicalKey); + Assert.IsNull(baseUrl); + } + [TestMethod] public void TryResolve_UnknownKey_ReturnsFalseAndNull() { diff --git a/GitLfsCache/Endpoints/GitLfsCacheHandler.cs b/GitLfsCache/Endpoints/GitLfsCacheHandler.cs index 01f7067..7b0ddbb 100644 --- a/GitLfsCache/Endpoints/GitLfsCacheHandler.cs +++ b/GitLfsCache/Endpoints/GitLfsCacheHandler.cs @@ -49,13 +49,18 @@ public async Task HandleAsync(HttpContext context) return; } - if (!registry.TryResolve(route.Upstream, out Uri? resolved) || resolved is null) + if (!registry.TryResolve(route.Upstream, out string canonicalKey, out Uri? resolved) || resolved is null) { EndpointLog.UnknownUpstream(logger, route.Upstream); context.Response.StatusCode = StatusCodes.Status404NotFound; return; } + // The key matched regardless of case, so from here on it is the configured spelling. Everything + // downstream (store paths, fetch coalescing, transfer tokens, lock snapshots and metrics) is + // keyed by it, and /GitHub/ and /github/ must share one cache rather than each fill their own. + route = route with { Upstream = canonicalKey }; + // Checked here, once, rather than in each branch below, so batch, transfer, verify and relay // are all covered and any route added later inherits it. Before any upstream call, so a // refused path costs upstream nothing and cannot be used to learn anything about it. A 404 diff --git a/GitLfsCache/Upstreams/IUpstreamRegistry.cs b/GitLfsCache/Upstreams/IUpstreamRegistry.cs index 3c73d15..96876a5 100644 --- a/GitLfsCache/Upstreams/IUpstreamRegistry.cs +++ b/GitLfsCache/Upstreams/IUpstreamRegistry.cs @@ -14,4 +14,23 @@ public interface IUpstreamRegistry /// The configured base URL, or null when the key is unknown. /// when the key is configured. public bool TryResolve(string key, out Uri? baseUrl); + + /// + /// Resolves an upstream key and reports the spelling it is configured under. + /// + /// + /// Keys match regardless of case, so /GitHub/ and /github/ reach the same upstream. + /// Everything keyed by the upstream afterwards (the object store, fetch coalescing, transfer tokens, + /// lock snapshots and metrics) must use , or each spelling gets its + /// own cold cache. The default implementation returns unchanged. + /// + /// The first path segment of the request. + /// The key as configured, or when it is unknown. + /// The configured base URL, or null when the key is unknown. + /// when the key is configured. + public bool TryResolve(string key, out string canonicalKey, out Uri? baseUrl) + { + canonicalKey = key; + return TryResolve(key, out baseUrl); + } } diff --git a/GitLfsCache/Upstreams/UpstreamRegistry.cs b/GitLfsCache/Upstreams/UpstreamRegistry.cs index 72e9dc6..5a54fe2 100644 --- a/GitLfsCache/Upstreams/UpstreamRegistry.cs +++ b/GitLfsCache/Upstreams/UpstreamRegistry.cs @@ -16,16 +16,20 @@ namespace ktsu.GitLfsCache.Upstreams; /// The configured options. public sealed class UpstreamRegistry(IOptions options) : IUpstreamRegistry { - private readonly Dictionary _upstreams = options.Value.Upstreams + private readonly Dictionary _upstreams = options.Value.Upstreams .Where(pair => pair.Value.BaseUrl is not null) .ToDictionary( pair => pair.Key, - pair => pair.Value.BaseUrl!, + pair => (pair.Key, pair.Value.BaseUrl!), StringComparer.OrdinalIgnoreCase); /// - public bool TryResolve(string key, out Uri? baseUrl) + public bool TryResolve(string key, out Uri? baseUrl) => TryResolve(key, out _, out baseUrl); + + /// + public bool TryResolve(string key, out string canonicalKey, out Uri? baseUrl) { + canonicalKey = key; baseUrl = null; if (string.IsNullOrEmpty(key) || key.Contains('/', StringComparison.Ordinal)) @@ -33,9 +37,10 @@ public bool TryResolve(string key, out Uri? baseUrl) return false; } - if (_upstreams.TryGetValue(key, out Uri? resolved)) + if (_upstreams.TryGetValue(key, out (string Key, Uri BaseUrl) resolved)) { - baseUrl = resolved; + canonicalKey = resolved.Key; + baseUrl = resolved.BaseUrl; return true; }