dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.26k stars 4.73k forks source link

[API Proposal]: Add constructor to StreamReader and StreamWriter accepting ArrayPools #107490

Open halter73 opened 2 months ago

halter73 commented 2 months ago

Background and motivation

Microsoft.AspNetCore.WebUtilities.HttpRequestStreamReader is essentially a less complete and efficient version of StreamReader that allows you to specify the ArrayPool<byte> and ArrayPool<char> it uses. It could probably be updated to inherit from StreamReader if StreamReader had a constructor accepting these parameters reducing unnecessary duplication.

API Proposal

namespace System.IO;

public class StreamReader : TextReader
{
    public StreamReader(Stream stream, Encoding? encoding = null, bool detectEncodingFromByteOrderMarks = true, int bufferSize = -1, bool leaveOpen = false);
+   public StreamReader(Stream stream, Encoding? encoding = null, bool detectEncodingFromByteOrderMarks = true, int bufferSize = -1, bool leaveOpen = false, ArrayPool<byte> bytePool, ArrayPool<char> charPool);
}

public class StreamWriter : TextWriter
{
    public StreamWriter(Stream stream, Encoding? encoding = null, int bufferSize = -1, bool leaveOpen = false);
+   public StreamWriter(Stream stream, Encoding? encoding = null, int bufferSize = -1, bool leaveOpen = false, ArrayPool<byte> bytePool, ArrayPool<char> charPool);
}

API Usage


namespace Microsoft.AspNetCore.WebUtilities;

// There are a bunch of other constructors that call `: this(...)` which are excluded here for brevity.

public class HttpRequestStreamReader : StreamReader
{
    public HttpRequestStreamReader(
        Stream stream,
        Encoding encoding,
        int bufferSize,
        ArrayPool<byte> bytePool,
        ArrayPool<char> charPool)
    : base(stream, encoding, detectEncodingFromByteOrderMarks: false, bufferSize, leaveOpen: true, byteBool, charPool)
    { }
}

public class HttpResponseStreamWriter : StreamWriter
{
    public HttpResponseStreamWriter(
        Stream stream,
        Encoding encoding,
        int bufferSize,
        ArrayPool<byte> bytePool,
        ArrayPool<char> charPool)
    : base(stream, encoding, bufferSize, leaveOpen: true, byteBool, charPool)
    { }
}

Alternative Designs

Do nothing at continue having a substantially similar implementation in ASP.NET Core's HttpRequestStreamReader and HttpRequestStreamWriter.

Risks

There might be some incompatibility or breaking change caused by HttpRequestStreamReader implementing StreamReader and/or HttpRequestStreamWriter implementing StreamWriter that I'm not seeing.

dotnet-policy-service[bot] commented 2 months ago

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

halter73 commented 2 months ago

@GrabYourPitchforks In case you have any thoughts. I see that StreamReader does some things with BOMs and preambles that HttpRequestStreamReader currently does not. I would imagine less duplication is a good idea for security too.