.Net Core 3 Preview SequenceReader Length Delimited Parsing

I'm trying to use SequenceReader<T> in .Net Core Preview 8 to parse Guacamole Protocol network traffic.

The traffic might look as follows:

5.error,14.some text here,1.0;

This is a single error instruction. There are 3 fields:

OpCode = error
Reason = some text here
Status = 0 (see Status Codes)

The fields are comma delimited (semi-colon terminated), but they also have the length prefixed on each field. I presume that's so that you could parse something like:

5.error,24.some, text, with, commas,1.0;

To produce Reason = some, text, with, commas.

Simple comma delimited parsing is simple enough to do (with or without SequenceReader). However, to utilise the length I've tried the following:

public static bool TryGetNextElement(this ref SerializationContext context, out ReadOnlySequence<byte> element)
{
    element = default;
    var start = context.Reader.Position;
    if (!context.Reader.TryReadTo(out ReadOnlySequence<byte> lengthSlice, Utf8Bytes.Period, advancePastDelimiter: true))
        return false;
    if (!lengthSlice.TryGetInt(out var length))
        return false;
    context.Reader.Advance(length);
    element = context.Reader.Sequence.Slice(start, context.Reader.Position);
    return true;
}

Based on my understanding of the initial proposal, this should work, though also could be simplified I think because some of the methods in the proposal make life a bit easier than that which is available in .Net Core Preview 8.

However, the problem with this code is that the SequenceReader does not seem to Advance as I would expect. It's Position and Consumed properties remain unchanged when advancing, so the element I slice at the end is always an empty sequence.

What do I need to do in order to parse this protocol correctly?

Solution

I'm guessing that .Reader here is a property; this is important because SequenceReader<T> is a mutable struct, but every time you access .SomeProperty you are working with an isolated copy of the reader. It is fine to hide it behind a property, but you'd need to make sure you work with a local and then push back when complete, i.e.

var reader = context.Reader;
var start = reader.Position;
if (!reader.TryReadTo(out ReadOnlySequence<byte> lengthSlice,
        Utf8Bytes.Period, advancePastDelimiter: true))
    return false;
if (!lengthSlice.TryGetInt(out var length))
    return false;
reader.Advance(length);
element = reader.Sequence.Slice(start, reader.Position);

context.Reader = reader; // update position
return true;

Note that a nice feature of this is that in the failure cases (return false), you won't have changed the state yet, because you've only been mutating your local standalone clone.

You could also consider a ref-return property for .Reader.