Skip to content

[API Proposal]: Add incremental chunked reading support to CborReader similar to Utf8JsonReader style #129718

Closed

Description

@muhammad-othman

Background and motivation

CborReader uses single buffer and when a buffer ends mid item PeekState() and Read* methods throw CborContentException , and Reset(ReadOnlyMemory<byte>) the only API to update the buffer discards all nesting state.
So consumers reading CBOR from a Stream in fixed chunks must either load the whole stream or drive control flow with try/catch.
Utf8JsonReader on the other hand fixes this gap by adding isFinalBlock and CurrentState. CBOR should have similar synchronous incremental mode, with the caller owning I/O (sync or async).

API Proposal

namespace System.Formats.Cbor;

public partial enum CborReaderState
{
    NeedsMoreData,
}

public partial class CborReader
{
    // Existing:
    // public CborReader(ReadOnlyMemory<byte> data, CborReaderOptions? options);
    // So isFinalBlock could be defaulted, or not.
    //
    // throws ArgumentException if isFinalBlock is false and options.ConformanceMode != Lax.
    public CborReader(ReadOnlyMemory<byte> data, CborReaderOptions? options, bool isFinalBlock);

    // Replaces the current _data and sets the instance _offset to 0, but does not reset the
    // array/map nesting stack.
    //
    // throws InvalidOperationException if _isFinalBlock is true (cannot Slide once the end is known)
    public void SlideData(ReadOnlyMemory<byte> data, bool isFinalBlock);

    // Replaces the current _data and sets the instance _offset to 0, and resets
    // array/map nesting stack.
    //
    // Existing: public void Reset(ReadOnlyMemory<byte> data);
    public void Reset(ReadOnlyMemory<byte> data, bool isFinalBlock);

    // true: All cases where SkipValue would not throw.  _offset has moved to the end of the value.
    // false: SkipValue would have thrown "unexpected end of buffer" and _isFinalBlock is false
    // throws: All other reasons (e.g. malformed input)
    //
    // Note: Until/unless a solution is landed on to enable isFinalBlock for !Lax,
    // the boolean has no measurable effect.  But it matches the signature of SkipValue.
    public bool TrySkipValue(bool disableConformanceModeChecks = false);
    public bool TrySkipToParent(bool disableConformanceModeChecks = false);
}
  • ctor or ResetData with isFinalBlock:true preserve today's behavior for all methods
    • The new SlideData method will throw InvalidOperationException
  • isFinalBlock:false
    • Ctor throws unless conformance mode is Lax (v1 restriction)
    • Read* still succeeds or throws as today
    • SkipValue and SkipToParent still succeed or throw as today
    • PeekState reports NeedsMoreData when the object at the current offset is incomplete, rather than throwing CborContentException. Still throws in other cases.
    • TrySkipValue/TrySkipToParent return true when SkipValue/SkipToParent would succeed, false when they would fail due to data truncation, throw when data is invalid (e.g. an indefinite map with an odd number of entries, violating key/value parity)
    • SlideData resets _data to the new value and _offset to 0, but remembers CurrentDepth and the state push/pop stack.
    • Reset resets _data to the new value, _offset to 0, and clears the state push/pop stack (resetting CurrentDepth to 0).

API Usage

    static async Task<MyValue> ReadCborFromStreamAsync(Stream stream, CancellationToken ct)
    {
        // Two-arg Reset is supported only in Lax conformance mode.
        var reader = new CborReader(ReadOnlyMemory<byte>.Empty, CborConformanceMode.Lax);

        byte[] buffer = new byte[4096];
        int dataLength = await stream.ReadAsync(buffer, ct);
        bool isFinalBlock = dataLength == 0;
        reader.Reset(buffer.AsMemory(0, dataLength), isFinalBlock);
        MyValue result = default;

        while (true)
        {
            switch (reader.PeekState())
            { 
                case CborReaderState.NeedsMoreData:
                    // In Lax the unconsumed tail IS the leftover, so BytesRemaining is all we need.
                    int keep  = reader.BytesRemaining;
                    int start = dataLength - keep;
                    
                    if (keep == buffer.Length)                 // one item bigger than the buffer
                        Array.Resize(ref buffer, buffer.Length * 2);
                    
                    Buffer.BlockCopy(buffer, start, buffer, 0, keep);
                    int read = await stream.ReadAsync(buffer.AsMemory(keep), ct);
                    dataLength   = keep + read;
                    isFinalBlock = read == 0;
                
                    reader.SlideData(buffer.AsMemory(0, dataLength), isFinalBlock); // nesting preserved
                    continue;

                case CborReaderState.Finished:
                    return result;                             // whole document consumed

                case CborReaderState.*: //One token read is safe as  `PeekState() == NeedsMoreData` checked before as the non-throwing gate
                    result  = reader.Read*();       
                    break;
         
                // ... other typed cases ...

                default:
                    throw new InvalidOperationException($"Unexpected state {reader.PeekState()}.");
            }
        }
    }

Alternative Designs

Stream/async overloads on CborReader (the rejected #99993 ask). Implies async variants of every Read* (the maintainer objection). The isFinalBlock model keeps the reader synchronous and the caller owns I/O.

External JsonReaderState-style state struct + new-reader-per-chunk. Exists only because Utf8JsonReader is a ref struct however CborReader is a class, so in-place preservation via the Reset overload is simpler.

Changing TryReadByteString/TryReadTextString to also return false on needs-more-data: overloads false with two incompatible recoveries (grow output vs refill input) the caller can't disambiguate, and is redundant because the PeekState() gate already guarantees a definite string is buffered before the read. TrySkipValue is the sole Try* that signals needs-more-data, justified because no single-token peek can gate a multi-token subtree skip and its false is unambiguous.

Risks

Additive enum member. CborReaderState.NeedsMoreData can appear in a switch with { default: throw } but it is returned only when the caller opts into non-final mode, so existing single-buffer callers shouldn't observe it.
PeekState() behavior change is scoped to non-final mode. Final mode (and the one-arg Reset) is byte-for-byte unchanged.
Buffer-size floor. Non-final mode can't progress if the buffer is smaller than the largest single definite string value. Same as the documented Utf8JsonReader constraint.
TrySkipValue buffers the skipped subtree (rewind-and-retry); fine for small unknown fields, resumable streaming skip is can be added as a follow up if needed.
NotSupportedException for non-Lax is a deliberate scope limit and doesn't have to be permanent.

Activity

  1. dotnet-policy-service commented on Jun 22, 2026

    @dotnet-policy-service
    Contributor

    Tagging subscribers to this area: @dotnet/area-system-io
    See info in area-owners.md if you want to be subscribed.

  2. dotnet-policy-service commented on Jun 22, 2026

    @dotnet-policy-service
    Contributor

    Tagging subscribers to this area: @dotnet/area-system-formats-cbor, @bartonjs, @vcsjones
    See info in area-owners.md if you want to be subscribed.

  3. bartonjs commented on Jun 22, 2026

    @bartonjs
    Member

    I certainly wouldn't expect a method called "Reset" to be state-preserving. Instead, I'd expect that to be "I've finished reading that thing, here's some new data, start back at 0". This seems more like "ExtendData" or maybe even just "Extend". (I like "ExtendData" over "ReplaceData" because it better suggests currentData.SequenceEquals(newData.Slice(0, currentData.Length))... whether or not we choose to enforce that at runtime)

    @eiriktsarpalis Do you remember if we had any thoughts regarding this back when we first built it?

  4. muhammad-othman commented on Jun 23, 2026

    @muhammad-othman
    ContributorAuthor

    Thanks you @bartonjs for the quick review of this proposal.
    I agree "Reset" is a bit misleading and I'm good with renaming it. I was borrowing from Utf8JsonReader with its isFinalBlock, and the caller owning the read/refill loop. JSON reconstructs the reader each chunk so I was thinking that Reset can act similar to JSON construction in addition to keeping the API change small. But yeah it can be misleading.

    One thing I want to make sure I understand, because there are two ways I read "extend" and they behave very differently:

    • Keep the whole buffer and append to it. The caller never drops anything; each refill makes the buffer bigger. This matches your newData starts with all of currentData description literally, but it means consumed bytes are never released and for a large response the buffer just keeps growing, which defeats the point of streaming.
    • Drop what's already been consumed, then append (what I intend and hope we implement). When the reader stops at NeedsMoreData, the caller takes the bytes the reader hasn't consumed yet (the trailing BytesRemaining bytes), moves them to the front of the buffer, reads more bytes in after them, and passes that. The reader keeps its nesting state but starts again from index 0 of this new buffer.
      And I agree it shouldn't be enforced on every call since comparing the bytes each refill would degrade the performance, just documenting the expectation and maybe add Debug.Assert,.
  5. bartonjs commented on Jun 23, 2026

    @bartonjs
    Member

    "Keep the whole buffer and append to it" is what I had in mind with a name like ExtendData. If it's "reset the cursor position to 0 while maintaining the stack" that feels like another verb. Shift/Slide/Augment/Transition/...?

  6. eiriktsarpalis commented on Jun 23, 2026

    @eiriktsarpalis
    Member

    @eiriktsarpalis Do you remember if we had any thoughts regarding this back when we first built it?

    The key motivating use case at the time was CTAP and we were operating under the assumption that large payloads were out of scope.

  7. muhammad-othman commented on Jun 23, 2026

    @muhammad-othman
    ContributorAuthor

    @bartonjs I was trying to move toward the bounded version where the consumed bytes are discarded to decrease the memory allocation and the leftover are moved to the start of the buffer then the rest is filled with a new read so we don't have to expand the buffer size unless a single item doesn't fit. From your suggestions I feel Slide makes mode sense as the sliding window mental model seems the closest to what I'm trying to describe.

    @eiriktsarpalis Thank you for the context, now it make sense why streaming wasn't in the scope initially. For our use case we're deserializing AWS service responses over HTTPS, where payloads can be tens of megabytes or even more, the larges service that uses CBOR right now is CloudWatch and other services are expected to support too.

  8. muhammad-othman commented on Jul 10, 2026

    @muhammad-othman
    ContributorAuthor

    I believe we have agreed on Slide as the naming. Is there anything else needed before this can move to review?

  9. bartonjs commented on Jul 20, 2026

    @bartonjs
    Member

    I don't know that we have "agreed" on Slide, though it was a word I had spat out, so in that sense I guess it has two party assent 😄. I don't know that we've ever done something like that in the BCL, so I don't know that we have naming prior art. But it could be a placeholder to make progress into API Review.

    However, I don't know that it can even function all that well. ReadStartMap pushes the offset onto the state stack, and that offset is restored on pop. I believe the reader uses the offset to go back and check on dictionary key conflicts (when it needs to), so either a) Slide can't slide beyond the first offset saved in that stack, or b) the stack needs to change to making copies of the keys when in non-final mode.

    Whether the continuation mode be from Expand or Slide, I imagine we'd also want

    public partial class CborReader
    {
        public CborReader(ReadOnlyMemory<byte> data, CborReaderOptions? options, bool isFinalBlock);
    }

    So that the expected operation isn't

    CborReader reader = new(ReadOnlyMemory.Empty, options);
    reader.Reset(data, true);

    but just

    CborReader reader = new(data, options, true);
  10. muhammad-othman commented on Jul 20, 2026

    @muhammad-othman
    ContributorAuthor

    @bartonjs You said Slide, it means you agreed on it 😅. The closest pattern I have found is Utf8JsonReader constructor which accepts isFinalBlock in addition to the previous state, though I don't think we should imitate that in CborReader constructor as CborReader class that doesn't have the same limitations that Utf8JsonReader struct has.
    I initially suggest using Reset to avoid creating a new method and limit the surface API changes, but the names that you have suggested works too, I lean toward Shift or Slide but I don't really have a strong opinion regarding that.

    For the check on dictionary key conflicts issue, based on my understand I believe that this doesn't happen when ConformanceMode == Lax which is why I suggested supporting Lax only for this iteration.
    If we want to support other ConformanceMode we will need to implement the key copying you have mentioned, and I think it is reasonable to implement this as a follow up rather than on shot all the ConformanceModes.

    The constructor looks much nicer than construct then reset approach for the first block, but we still need the continuation method to add new chunks while preserving the state.

    Please let me know if I missed something or any of my assumptions are incorrect.

  11. 11 remaining items

  12. muhammad-othman commented on Jul 23, 2026

    @muhammad-othman
    ContributorAuthor

    Thank you so much @bartonjs for taking care of that.

  13. bartonjs commented on Aug 11, 2026

    @bartonjs
    Member

    Video

    Looks good as proposed

    namespace System.Formats.Cbor;
    
    public partial enum CborReaderState
    {
        NeedsMoreData,
    }
    
    public partial class CborReader
    {
        public CborReader(ReadOnlyMemory<byte> data, CborReaderOptions? options, bool isFinalBlock);
    
        public void SlideData(ReadOnlyMemory<byte> data, bool isFinalBlock);
    
        public void Reset(ReadOnlyMemory<byte> data, bool isFinalBlock);
    
        public bool TrySkipValue(bool disableConformanceModeChecks = false);
        public bool TrySkipToParent(bool disableConformanceModeChecks = false);
    }
  14. added
    api-approvedAPI was approved in API review, it can be implemented
    and removed
    api-ready-for-reviewAPI is ready for review, it is NOT ready for implementation
    on Aug 11, 2026
  15. muhammad-othman commented on Aug 12, 2026

    @muhammad-othman
    ContributorAuthor

    Thank you @bartonjs for discussing and approving this, I will work on the implementation and open a PR soon.

  16. modified the milestones: 11.0.0, 12.0.0 on Sep 9, 2026
  17. modified the milestones: 12.0.0, 11.0.0 on Sep 21, 2026
  18. vcsjones commented on Oct 3, 2026

    @vcsjones
    Member

    @muhammad-othman @bartonjs it looks like this has been merged and backported to 11.0 - is there anything pending here or can this be closed?

  19. muhammad-othman commented on Oct 4, 2026

    @muhammad-othman
    ContributorAuthor

    Yes Kevin we can close it, nothing else needed after #133602 was merged.
    Thank you Jeremy for closing it.

  20. locked and limited conversation to collaborators on Nov 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions