fix: complete the malformed-input guards, including the two paths still live - #3663
Merged
Conversation
…ll live
Review of the previous change found three of its claims unmet.
The image-generation crash it reported fixed is still reachable. The assert was
scattered across three helpers, and Gemini and OpenAI call `isHttpUrl` directly
on `input_images` without going through any of them — so two of seven providers
still 500 on a non-string. `isHttpUrl` now refuses a non-string itself, and the
shape is settled once in `ImageGenerationDriver.generate`, where the driver call
arrives, rather than per helper. That also covers `input_images` that isn't an
array, which produced a different crash per provider.
The sixth case in the ticket, previously unlocated, is
`Messages.js` reading `tool_call.function.name` with no guard — reachable with
`{"messages":[{"role":"assistant","tool_calls":[{"id":"x"}]}]}`. Guarded, along
with the same shape in `make_claude_tools`: a TypeError there carries no status,
so the retry loop reads it as a provider failure and marks the route unhealthy
for every caller.
`#hardExpiryFromExpiresIn` returning null for a bad type moved the failure past
the session INSERT, leaving an orphaned non-expiring row and still answering
500. Reverted; the controller guard is the fix, now covering fractions,
negatives and unparseable durations rather than only wrong types.
Also: the batch write handlers check that the body is an array but not what is
in it, so a null element 500s the same way; `#requireObjectBody` accepted an
array despite its name; `handleCreateAccessToken` destructured a body that may
be absent; and two AGPL notices had been rewrapped with a Markdown link.
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review of the previous change found three of its claims unmet.
The image-generation crash it reported fixed is still reachable. The assert was scattered across three helpers, and Gemini and OpenAI call
isHttpUrldirectly oninput_imageswithout going through any of them — so two of seven providers still 500 on a non-string.isHttpUrlnow refuses a non-string itself, and the shape is settled once inImageGenerationDriver.generate, where the driver call arrives, rather than per helper. That also coversinput_imagesthat isn't an array, which produced a different crash per provider.The sixth case in the ticket, previously unlocated, is
Messages.jsreadingtool_call.function.namewith no guard — reachable with{"messages":[{"role":"assistant","tool_calls":[{"id":"x"}]}]}. Guarded, along with the same shape inmake_claude_tools: a TypeError there carries no status, so the retry loop reads it as a provider failure and marks the route unhealthy for every caller.#hardExpiryFromExpiresInreturning null for a bad type moved the failure past the session INSERT, leaving an orphaned non-expiring row and still answering500. Reverted; the controller guard is the fix, now covering fractions, negatives and unparseable durations rather than only wrong types.
Also: the batch write handlers check that the body is an array but not what is in it, so a null element 500s the same way;
#requireObjectBodyaccepted an array despite its name;handleCreateAccessTokendestructured a body that may be absent; and two AGPL notices had been rewrapped with a Markdown link.