dotnet / dotnet/runtime

[API Proposal]: System.Text.Decoder: "Validate" method that can be repeatedly called to validate a sequence of encoded bytes split into multiple blocks

Open
#129,146 5 comments 0 reactions 0 assignees View on GitHub
api-suggestion area-System.Text.Encoding
Dominant language
C#
Stars
18.3k
Forks
5.6k
PR merge metrics
PR metrics pending

Description

### Background and motivation

In [System.Text.Decoder](https://learn.microsoft.com/en-us/dotnet/api/system.text.decoder?view=net-11.0), add the capability to statefully validate a sequence of byte blocks (as performantly as possible), one block at a time, without converting them to characters.

It is required to support a single character being split between blocks, for encodings where that is relevant (e.g. UTF-8).

In Decoder (an abstract class), this could be implemented on top of Decoder.Convert, but more performant implementations in subclasses would be needed.

I looked at GetCharCount, but [it says](https://learn.microsoft.com/en-us/dotnet/api/system.text.decoder.getcharcount?view=net-11.0#system-text-decoder-getcharcount(system-readonlyspan((system-byte))-system-boolean)), "This method does not affect the state of the decoder." which means it's difficult to call consecutively on multiple blocks.

Because GetCharCount only returns the number of _characters produced_, not _bytes consumed_, I also don't see a way to implement this new functionality on top of GetCharCount. If there is a character split between blocks, the caller wouldn't know the first byte to send back in to GetCharCount when a new block arrived.

A possible new API could be:

```cs
public virtual void Validate(ReadOnlyMemory bytes, bool isLastBlock);
```

throwing on the first byte that can not be part of a valid byte sequence in that encoding (I am not sure if Decoder.Fallback should be supported here (or it should always throw), but it could be).

There could be another non-throwing version. E.g.

```cs
public virtual bool TryValidate(ReadOnlyMemory bytes, bool isLastBlock, out int firstInvalidIndex);
```

with `firstInvalidIndex` being the index of the first invalid byte (see above). `isLastBlock` would be set to true when the caller knows there is no further byte data (e.g. last message), meaning a still-incomplete byte sequence is invalid.

The idea is that it could be called as little as once (a single block that contains the entire byte sequence) to multiple times (a byte sequence split in three messages) to (in theory) once per byte (not recommended; only for illustration). Only if a byte is found that breaks the byte sequence (an **incomplete** byte sequence is okay, as long as it's not the last block) would it fail.

Motivation:

ManagedWebSocket has its own UTF-8 validator, [TryValidateUtf8](https://github.com/dotnet/dotnet/blob/09f01f27414383b562595cb7bfb7b2cc0e6c778d/src/runtime/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/ManagedWebSocket.cs#L1775), with similar requirements.

It would be wasteful to use Convert, since (for ManagedWebSocket and similar) the actual character data can not be returned to the ultimate calling code. ([ReceiveAsync](https://learn.microsoft.com/en-us/dotnet/api/system.net.websockets.websocket.receiveasync?view=net-11.0) is a byte-oriented API (even in Text mode). The caller is responsible for converting to text.)

The motivations for this feature request are:
a. Other .NET networking or I/O libraries may have the same need (validate a multi-block byte sequence, which may split bytes between sequences, without doing the actual conversion to a character sequence)
b. In my opinion, this would be better maintained in System.Text rather than in System.Net.WebSockets.
c. Encoding is security-critical, so it is worth having a public API that does this.

### API Proposal

```csharp
namespace System.Text;

public class Decoder
{
// [...]

public virtual void Validate(ReadOnlyMemory bytes, bool isLastBlock);

public virtual bool TryValidate(ReadOnlyMemory bytes, bool isLastBlock, out int firstInvalidIndex);
}
```

### API Usage

```csharp

Decoder decoder = Encoding.UTF8.GetDecoder();

do {
Memory buffer = // ...

await int bytesReceived = GetNetworkData(buffer, out bool isLastBlock, CancellationToken.None);
// Error-handling omitted
ReadOnlyMemory currentBlock = buffer.Slice(0, bytesReceived);
decoder.Validate( currentBlock, isLastBlock );
} while ( !isLastBlock );
```

### Alternative Designs

1. Add a parameter/overload/configuration that makes GetCharCount stateful, and return/out parameter the number of bytes processed. The downside is that the requirement to still calculate the character count may itself limit performance optimizations. It also may be unintuitive to vary statefulness within a method like this.
2. Add an overload to Convert to accept /dev/null (metaphorically) as an output, in which case it would not produce actual characters. It could optimize this case for performance. This is not idiomatic and has the same issues as alternative 1.

I don't recommend either of these alternatives.

### Risks

1. Encoding is security-critical, so correctness is important.
2. (Only when calling new API) Poor performance for encodings that fallback to Convert (but no worse than using Convert directly) instead of implementing an optimized version. This would include all third-party encodings that don't update quickly, even if the built-in encodings are updated in unison.

On the other hand, it reduces these risks:
1. Incorrect implementations outside of System.Text (e.g. user code). In theory, it is better to have one implementation that (we hope) is heavily reviewed.
2. Some protocols not validating "for performance", which could lead to security issues

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.