What's wrong
LockFanOutRequest.TryParse reads the ref with a chained indexer:
// GitLfsCache/Locks/LockFanOutRequest.cs:62
JsonValues.String(root["ref"]?["name"]),
JsonNode's string indexer calls AsObject(). When root["ref"] is present but is not a JsonObject (a string, number, bool or array), it throws InvalidOperationException: The node must be of type 'JsonObject'. before JsonValues.String runs. LockRouteHandler.FanOutAsync only catches System.Text.Json.JsonException around the parse (GitLfsCache/Endpoints/LockRouteHandler.cs:106-115), and TryParse is called outside that try (LockRouteHandler.cs:117). Nothing else in the pipeline handles the exception (GitLfsCacheHandler, MapGitLfsCache, and the tool host have no exception middleware), so the client gets an unhandled-exception 500.
This goes against the locks spec's As built note (locks-subsystem-design.md L270):`` "All untrusted JSON is read through a JsonValues helper ... reading a client's or an upstream's body directly turns a malformed request into an unhandled exception and a 500." This is the one read in the fan-out parser that still does that. The type docs of `LockFanOutRequest.TryParse` also say "Every malformed shape is refused rather than partly honoured".
Failure scenario
POST /github/studio/game.git/info/lfs/locks/batch
{"operation":"lock","paths":["Content/A.uasset"],"ref":"refs/heads/main"}
A client that flattens ref to a string (the listing endpoint takes it as a refspec query string, so this mistake is easy to make when writing the vendored-client patch the spec describes) gets 500 with no body, where it should get 400. "ref": ["x"] and "ref": 1 fail the same way.
Reproduced with a standalone System.Text.Json (net10.0) program: JsonNode.Parse("{\"ref\":\"refs/heads/main\"}")["ref"]?["name"] throws System.InvalidOperationException: The node must be of type 'JsonObject'.
Suggested fix
- Read the ref with a type check first, e.g.
root["ref"] is JsonObject refObject ? JsonValues.String(refObject["name"]) : null, or add a JsonValues.Object(...) helper.
- Decide whether a
ref that is present but not an object should be refused with 400 or treated as absent. Refusing matches the parser's stated "every malformed shape is refused" rule, and means a lock is never taken without the ref the client meant to send.
Acceptance criteria
LockFanOutRequest.TryParse returns false and does not throw for ref given as a string, number, bool or array. There are currently no unit tests for LockFanOutRequest in GitLfsCache.Tests/.- An integration test in
LockFanOutTests shows POST locks/batch with "ref": "refs/heads/main" returns 400 and makes no upstream call.
{"ref": {"name": "refs/heads/main"}} behaves as it does today.
What's wrong
LockFanOutRequest.TryParsereads the ref with a chained indexer:JsonNode's string indexer callsAsObject(). Whenroot["ref"]is present but is not aJsonObject(a string, number, bool or array), it throwsInvalidOperationException: The node must be of type 'JsonObject'.beforeJsonValues.Stringruns.LockRouteHandler.FanOutAsynconly catchesSystem.Text.Json.JsonExceptionaround the parse (GitLfsCache/Endpoints/LockRouteHandler.cs:106-115), andTryParseis called outside that try (LockRouteHandler.cs:117). Nothing else in the pipeline handles the exception (GitLfsCacheHandler,MapGitLfsCache, and the tool host have no exception middleware), so the client gets an unhandled-exception 500.This goes against the locks spec's As built note (locks-subsystem-design.md L270):`` "All untrusted JSON is read through a
JsonValueshelper ... reading a client's or an upstream's body directly turns a malformed request into an unhandled exception and a 500." This is the one read in the fan-out parser that still does that. The type docs of `LockFanOutRequest.TryParse` also say "Every malformed shape is refused rather than partly honoured".Failure scenario
A client that flattens
refto a string (the listing endpoint takes it as arefspecquery string, so this mistake is easy to make when writing the vendored-client patch the spec describes) gets500with no body, where it should get400."ref": ["x"]and"ref": 1fail the same way.Reproduced with a standalone
System.Text.Json(net10.0) program:JsonNode.Parse("{\"ref\":\"refs/heads/main\"}")["ref"]?["name"]throwsSystem.InvalidOperationException: The node must be of type 'JsonObject'.Suggested fix
root["ref"] is JsonObject refObject ? JsonValues.String(refObject["name"]) : null, or add aJsonValues.Object(...)helper.refthat is present but not an object should be refused with 400 or treated as absent. Refusing matches the parser's stated "every malformed shape is refused" rule, and means a lock is never taken without the ref the client meant to send.Acceptance criteria
LockFanOutRequest.TryParsereturns false and does not throw forrefgiven as a string, number, bool or array. There are currently no unit tests forLockFanOutRequestinGitLfsCache.Tests/.LockFanOutTestsshowsPOST locks/batchwith"ref": "refs/heads/main"returns 400 and makes no upstream call.{"ref": {"name": "refs/heads/main"}}behaves as it does today.