Report truncated non-null-terminated input as unexpected_end - #2735
Merged
Conversation
A non-null-terminated read raises end_reached to say "the buffer ran out
here". Whether that is a completed read or a truncated one is settled at the
top level from ctx.depth: zero means the value closed cleanly and the buffer
ended exactly where it did, anything else means containers were still open.
finalize_read_context cleared the first case and let the second escape, so a
truncated parse returned end_reached to the caller -- a code the enum
documents as a non-error, which reads as a success that stopped early:
glz::read<{.null_terminated = false}>(v, "[1,2") -> end_reached
glz::read_json(v, istream_buffer) on "[[1,2],[3" -> end_reached
Both halves now settle in one place and it returns unexpected_end.
This reaches the wire. Since #2732 every registry read runs without a
terminator, so a truncated REPE body answered its client with end_reached in
the response header. The registry test that covered truncation only asserted
"not none" and so passed either way; it names the code now.
end_reached is left in the enum and keeps its value -- the readers use it
internally and REPE puts these codes on the wire. Its comment now says it is
internal, and names json_stream_reader as the one place that still surfaces
it, as its own end-of-stream signal rather than a parse result.
Three gaps around the fix, none in settle_end_reached itself. A completed partial read left ctx.depth standing. A partial read stops once it has the fields it wants and unwinds on an error code, so its enclosing containers never decrement, and finalize_read_context cleared the error without clearing the depth. Since this change makes depth the sole arbiter of truncation, a context reused after a partial read then settled the next well-formed buffer to unexpected_end -- a valid read reported as failing. Cleared where the error is cleared. Resetting depth at read entry instead would have been wrong: a custom reader may call glz::read on an in-flight context, and that would erase its accounting. The enum comment claimed a read never returns end_reached. read_jmespath and get_view_json return ctx.error without finalizing, so they still do. They navigate by matching braces rather than by entering depth, so their ctx.depth is zero throughout and settle_end_reached cannot simply be dropped in -- it would settle their truncated reads to none, which is worse than leaking. The comment now says which entry points are covered and why those two are not. read_json_stream had no test, and it is where this mattered most: a stream whose last record is cut off stops on end_reached, which the reader treats as end of stream, so the caller got a short vector and a success. Silent data loss in a public API. Covered now, along with a stream that ends between records, which must still succeed.
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.
error_code::end_reachedis documented in the enum as a non-error code. It is meant to be internal to a non-null-terminated read, saying only "the buffer ran out here" — which is a completed read or a truncated one depending on where it happened. Nothing settled which, so it escaped to callers.Any non-null-terminated read of truncated JSON returned it:
Since #2732 every registry read runs non-null-terminated, so this reached the wire: a truncated REPE body answered its client with
end_reachedin the response header, telling it the request parsed and merely stopped early. The existingtruncated_body_is_an_errortest asserted only!= none, so it passed either way.The fix
ctx.depthalready carries the answer. A value that closed cleanly leaves it at zero, so the buffer ended exactly where the value did and the read succeeded. Any other depth means the buffer ran out with containers still open, which is truncated input.settle_end_reachedapplies that at the top level, called fromfinalize_read_contextso buffered and streaming reads settle identically and no call site can forget.end_reachedno longer escapes a read at all;json_stream_reader, which raises it itself to signal end of stream, is unaffected.The enum comment now says what the code is for and who is allowed to see it.
Tests
json_test: truncated non-null-terminated input reportsunexpected_endacross six shapes ([1,2,[[1,2],[3,{"a":1, ...), and six complete documents still reportnone.istream_buffer_test: a truncated stream, and a truncation only discovered after several refills.registry_view_test:truncated_body_is_an_errornow namesunexpected_endinstead of accepting any non-nonecode.Full suite: 108/108.