CsvParser's ReadAsync can read stream synchronously

[AdamDotNet]

Describe the bug When CsvParser has to read a quoted field, that quoted field has white space to trim, and the buffer needs to be filled, then a synchronous read instead of an asynchronous read from the underlying TextReader occurs. Asp.Net Core 3.0 throws System.InvalidOperationException: Synchronous operations are disallowed. when the underlying TextReader wraps the Request stream.

To Reproduce This test case is enough to trigger the exception. The test will pass if only async methods are called on the underlying TextReader; otherwise, InvalidOperationException is thrown on sync method calls.

using Microsoft.VisualStudio.TestTools.UnitTesting;
using System;
using System.IO;
using System.Text;
using System.Threading.Tasks;

namespace CsvHelper.Tests.Async
{
    [TestClass]
    public class ParserAsyncRegressionTests
    {
        [TestMethod]
        public async Task ReadAsync_WithQuotedFieldToTrim_WillNotReadSynchronously()
        {
            var s = new StringBuilder();
            s.Append("\"1  \"");
            var config = new CsvHelper.Configuration.Configuration
            {
                BufferSize = 4
            };
            using (var reader = new NoSyncStringReader(s.ToString()))
            using (var parser = new CsvParser(reader, config))
            {
                var row = await parser.ReadAsync().ConfigureAwait(false);
            }
        }

        private class NoSyncStringReader : StringReader
        {
            public NoSyncStringReader(string s) : base(s) { }

            // Cause synchronous methods to throw.
            public override int Read() => throw new InvalidOperationException("Synchronous operations are disallowed.");

            public override int Read(char[] buffer, int index, int count) => throw new InvalidOperationException("Synchronous operations are disallowed.");

            public override string ReadLine() => throw new InvalidOperationException("Synchronous operations are disallowed.");

            public override int ReadBlock(char[] buffer, int index, int count) => throw new InvalidOperationException("Synchronous operations are disallowed.");

            public override string ReadToEnd() => throw new InvalidOperationException("Synchronous operations are disallowed.");

            // Cause asynchronous methods to call base class to avoid throwing due to above overrides.
            public override Task<int> ReadAsync(char[] buffer, int index, int count) => Task.FromResult(base.Read(buffer, index, count));

            public override Task<int> ReadBlockAsync(char[] buffer, int index, int count) => Task.FromResult(base.ReadBlock(buffer, index, count));

            public override Task<string> ReadLineAsync() => Task.FromResult(base.ReadLine());

            public override Task<string> ReadToEndAsync() => Task.FromResult(base.ReadToEnd());
        }
    }
}

Expected behavior ReadAsync should never read synchronously.

To fix Change this ReadSpaces() to ReadSpacesAsync().

[mudnug]

I've reproduced this issue and will work on submitting a PR

[mudnug]

The example test won't work due to the way the .NET framework is implemented https://referencesource.microsoft.com/#mscorlib/system/io/stringreader.cs,188

I've created a draft PR, but haven't yet added tests

[karlra]

The same bug occurs if you attempt to write to the Response.Body stream and the data contains a field that contains a space.

var data = ...
using(var writer = new StreamWriter(Response.Body))
using(var csv = new CsvWriter(writer, CultureInfo.InvariantCulture))
{
	await csv.WriteRecordsAsync(data); //InvalidOperationException: Synchronous operations are disallowed. Call WriteAsync or set AllowSynchronousIO to true instead.
}

[anuith]

I found a workaround to this by calling DisposeAsync() manually in a try...finally block. Instead of using.

var csvwriter = new CsvWriter(writer, configuration);
try
{
    await csvwriter.WriteRecordsAsync(records);
}
finally
{
    await csvwriter.DisposeAsync();
}

[JoshClose]

The parser has been completely rewritten. I just tried your unit test in the existing code and it passes.