Skip to content

Answer 502, not 500, when upstream's 2xx batch body cannot be used [patch] - #104

Merged
matt-edmondson merged 1 commit into
mainfrom
fix/81-malformed-batch-502
Oct 9, 2026
Merged

matt-edmondson merged 1 commit into
mainfrom
fix/81-malformed-batch-502

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #81

What was wrong

  • ObjectRouteHandler.BatchAsync parsed a 2xx batch body with no try/catch. When a forge sign-in page, an SSO interstitial, or a captive portal answered with 200 text/html, the parser threw JsonException and git-lfs got a bare 500.
  • BatchRewriter read oid, size, the action href, and header values with GetValue<T>. A field of the wrong type, such as "size": "123", threw InvalidOperationException, which also surfaced as a 500.

Both are upstream sending something the proxy can't use. Answering 500 points operators at the proxy as the broken component.

Change

  • BatchRewriter now reads these fields through JsonValues with type checks. A malformed entry throws JsonException; this is documented on Rewrite. Such an entry can't be rewritten, and passing it through untouched would hand the client upstream's header credentials, so the whole response is refused rather than the entry being skipped.
  • BatchAsync turns a JsonException from either the parse or the rewrite into a 502, the same answer a null body already got.
  • Fields that are missing behave as before.

Tests

  • ProxyFlowTests.Batch_UpstreamSuccessThatCannotBeUsed_IsABadGateway sends three upstream bodies through the stub and checks each gets a 502: a 200 text/html page, "size": "123", and a numeric header value. StubUpstream.BatchSuccessBody is a new hook that lets the stub return a raw 2xx body.
  • BatchRewriterTests.Rewrite_FieldOfTheWrongType_IsRefusedAsMalformed covers a numeric oid, a string size, a fractional size, a numeric href, and a numeric header value.

With the fix reverted, all 8 new cases fail. The full suite passes locally: 362 tests, 0 failed.

This branch touches ObjectRouteHandler.cs in a different method from #101, and merges cleanly with #101, #102 and #103.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Md2Tr7FWcbqPdwq2BjeG79


Generated by Claude Code

…atch]

BatchAsync parsed a successful batch response with no try/catch, so a sign-in
or SSO page served with 200 threw JsonException and the client got a bare
500. BatchRewriter read oid, size, href and header values with GetValue<T>,
so a field of the wrong type ("size": "123") threw InvalidOperationException
and also surfaced as a 500.

The rewriter now reads those fields through JsonValues and type checks, and
throws JsonException for a malformed entry, which can neither be rewritten
nor passed through with upstream's credentials still in it. BatchAsync turns
a JsonException from the parse or the rewrite into a 502, the same answer a
null body already got.

Fixes #81

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Md2Tr7FWcbqPdwq2BjeG79
@sonarqubecloud

sonarqubecloud Bot commented Oct 8, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A 2xx batch response that isn't valid JSON, or has a mistyped field, returns 500 instead of 502 (e.g. an SSO page served with 200)

2 participants