diff --git a/GitLfsCache.Tests/Integration/LockFanOutTests.cs b/GitLfsCache.Tests/Integration/LockFanOutTests.cs index 8f81f3b..0c153dc 100644 --- a/GitLfsCache.Tests/Integration/LockFanOutTests.cs +++ b/GitLfsCache.Tests/Integration/LockFanOutTests.cs @@ -57,6 +57,21 @@ public async Task ManyPaths_BecomeOneClientRoundTripAndManyUpstreamCalls() Assert.AreEqual(50, fixture.Upstream.LockChangeRequests); } + [TestMethod] + public async Task RefThatIsNotAnObject_IsRefusedWithoutCallingUpstream() + { + // The listing endpoint takes the ref as a refspec string, so flattening it here is an easy + // mistake for a client to make. It is a malformed request, not a proxy failure. + await using ProxyFixture fixture = await ProxyFixture.StartAsync(); + + using HttpResponseMessage response = await PostBatchAsync( + fixture, + """{"operation":"lock","paths":["Content/A.uasset"],"ref":"refs/heads/main"}"""); + + Assert.AreEqual(HttpStatusCode.BadRequest, response.StatusCode); + Assert.AreEqual(0, fixture.Upstream.LockChangeRequests); + } + [TestMethod] public async Task EveryCall_CarriesTheCallersOwnCredential() { diff --git a/GitLfsCache.Tests/Locks/LockFanOutRequestTests.cs b/GitLfsCache.Tests/Locks/LockFanOutRequestTests.cs new file mode 100644 index 0000000..ba24d35 --- /dev/null +++ b/GitLfsCache.Tests/Locks/LockFanOutRequestTests.cs @@ -0,0 +1,45 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitLfsCache.Tests.Locks; + +using System.Text.Json.Nodes; +using ktsu.GitLfsCache.Locks; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +[TestClass] +public class LockFanOutRequestTests +{ + private static JsonNode Parse(string json) => JsonNode.Parse(json)!; + + [TestMethod] + public void TryParse_RefObject_ReadsItsName() + { + JsonNode body = Parse("""{"operation":"lock","paths":["a.uasset"],"ref":{"name":"refs/heads/main"}}"""); + + Assert.IsTrue(LockFanOutRequest.TryParse(body, out LockFanOutRequest? request)); + Assert.AreEqual("refs/heads/main", request.Ref); + } + + [TestMethod] + public void TryParse_NoRef_HasNoRef() + { + JsonNode body = Parse("""{"operation":"lock","paths":["a.uasset"]}"""); + + Assert.IsTrue(LockFanOutRequest.TryParse(body, out LockFanOutRequest? request)); + Assert.IsNull(request.Ref); + } + + [TestMethod] + [DataRow("\"refs/heads/main\"")] + [DataRow("1")] + [DataRow("true")] + [DataRow("[\"refs/heads/main\"]")] + public void TryParse_RefThatIsNotAnObject_IsRefused(string refJson) + { + // Taking the lock with no ref would not be the lock the client asked for. + JsonNode body = Parse($$"""{"operation":"lock","paths":["a.uasset"],"ref":{{refJson}}}"""); + + Assert.IsFalse(LockFanOutRequest.TryParse(body, out LockFanOutRequest? request)); + Assert.IsNull(request); + } +} diff --git a/GitLfsCache/Locks/LockFanOutRequest.cs b/GitLfsCache/Locks/LockFanOutRequest.cs index 0918bcb..7041f66 100644 --- a/GitLfsCache/Locks/LockFanOutRequest.cs +++ b/GitLfsCache/Locks/LockFanOutRequest.cs @@ -56,10 +56,15 @@ public static bool TryParse(JsonNode? body, [NotNullWhen(true)] out LockFanOutRe return false; } + if (!TryReadRef(root["ref"], out string? refName)) + { + return false; + } + request = new LockFanOutRequest( operation, targets, - JsonValues.String(root["ref"]?["name"]), + refName, JsonValues.Bool(root["force"]) ?? false); return true; @@ -73,6 +78,32 @@ private static LockFanOutOperation ReadOperation(JsonObject root) => _ => LockFanOutOperation.Unknown, }; + /// + /// Reads the ref's name, treating an absent ref as none. + /// + /// + /// A ref that is present but not an object (a client that flattened it to the ref name, as the + /// listing endpoint's refspec query takes it) refuses the whole body. Taking the lock with no + /// ref would not be the lock the client asked for, and indexing into a non-object throws. + /// + private static bool TryReadRef(JsonNode? node, out string? name) + { + name = null; + + if (node is null) + { + return true; + } + + if (node is not JsonObject refObject) + { + return false; + } + + name = JsonValues.String(refObject["name"]); + return true; + } + /// /// Reads the paths, and for a release the ids as well. ///