4

I'm updating some netstandard2.0 code to not read from the HttpRequest.Body (a Stream) synchronously (this throws exceptions in netcoreapp3.0 unless you set the AllowSynchronousIO option which is obviously not a good idea).

I've converted the JSON deserialization part (to use System.Text.Json), but before it does that it does a sneaky .Peek() using a StreamReader (to see if the body is an object or an array) and I'm not exactly sure where to look for a modern alternative that is asynchronous and won't consume the Stream.

using var reader = new StreamReader(stream);
switch (reader.Peek()) // TODO: Find an async equivalent!
{
    case '{':
        return await JsonSerializer.DeserializeAsync<GraphQLRequest>(stream);
    case '[':
        return await JsonSerializer.DeserializeAsync<GraphQLRequest[]>(stream);
    default:
        // ...
}
benmccallum
  • 1,241
  • 12
  • 27
  • 1
    There isn't, use Task.Run() to make it async. – Hans Passant Jan 06 '20 at 18:25
  • 1
    Could you perhaps use a reader the way you are and then `await reader.ReadAsync`. you then are not blocking can check before hand with you're working with array etc... – Trevor Jan 06 '20 at 18:25
  • 1
    @HansPassant why would you wrap an already available reader that supports async in another task? – Trevor Jan 06 '20 at 18:27
  • Deserialize without specifying the type, then inspect the returned object and cast it to the appropriate type – Andrew Williamson Jan 06 '20 at 18:39
  • On another note, instead of case statements, you could however use a `try/catch` and try and deserialize to the first type, if it fails try the other type instead. There would be no blocking, no boxing/unboxing. – Trevor Jan 06 '20 at 18:55
  • @Çöđěxěŕ Would that try to consume the stream twice? – Andrew Williamson Jan 06 '20 at 19:42
  • @AndrewWilliamson yes, unfortunately it would make two passes if the first failed. The only benefit you would have at this time is the asynchronous operations that are indeed useful as nothing is blocked. – Trevor Jan 06 '20 at 19:59
  • Why should one want to run a nanosecond long method, returning one byte, in async ? Reading a stream sometimes with a reader and without reader at the same time is not a good approach. – Holger Jan 06 '20 at 20:26
  • I didn't write the existing code, but I guess they're trying to fail fast in that default case if the wrong thing is passed. The thing is, in real-world use, incorrect API use is an uncommon path that shouldn't be optimised for. So I agree, reading it all upfront makes more sense, as long as it's not into a `string` or something I don't need/want to create. Mayo's answer seems the way to go :) Thanks all! – benmccallum Jan 07 '20 at 09:58

2 Answers2

1

Actually, Peek method reads the stream into its buffer at first and then give you the value from it (take a look at dotnet runtime repo). So it moves the position of a stream. However, peek functionality cannot be implemented without reading and thus seeking the stream.

As you cannot seek HttpRequestStream back, better strategy would be to read the entire content and then use Utf8JsonReader to determine whether the request is an object or an array. Note that Utf8JsonReader cannot be declared in async methods, so I had to put it in a separate non-async method.

private static JsonTokenType GetTokenType(byte[] bytes)
{
    var reader = new Utf8JsonReader(bytes.AsSpan());
    reader.Read();
    return reader.TokenType;
}
var ms = new MemoryStream();
await Request.Body.CopyToAsync(ms);
var jsonBytes = ms.ToArray();

switch (GetTokenType(jsonBytes)) 
{
    case JsonTokenType.StartObject:
        return JsonSerializer.Deserialize<GraphQLRequest>(jsonBytes);
    case JsonTokenType.StartArray:
        return JsonSerializer.Deserialize<GraphQLRequest[]>(jsonBytes);
    default:
        // ...
}
Mayo
  • 859
  • 2
  • 10
  • 14
  • Awesome, thanks @Mayo. One thing that I didn't include in my sample was that I was using a deserialize overload that accepted the `JsonSerializerOptions`. Doesn't look like the overload that takes bytes also takes the options obj. Would I be fine to do `JsonSerializer.Deserialize(jsonBytes.AsSpan(), _serializerOptions)`? – benmccallum Jan 07 '20 at 09:52
  • 1
    Yes you can do that, AsSpan makes no allocation, however it cannot be used inside async method so you have to move the code to a non-async one. – Mayo Jan 07 '20 at 11:02
  • Interesting. It seemed to work in my async method. Will it blow up silently? Should I just move them out to be sure? – benmccallum Jan 08 '20 at 18:57
  • 1
    @benmccallum sorry I was wrong, It is okay to pass it directly as an argument, but you can't assign Span to a variable inside async method. – Mayo Jan 08 '20 at 20:11
0

After going back and forward on this due to our unique circumstances, we (luckily!) got some help from @davidfowl who spun us up an example using PipeReader and it's AdvanceTo method to peek the token but not consume the stream so we could use Deserialize straight from the stream.

Full source here.

benmccallum
  • 1,241
  • 12
  • 27