Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Proposal]: System.Formats.Asn1.AsnValueReader #109019

Open
vcsjones opened this issue Oct 18, 2024 · 1 comment
Open

[API Proposal]: System.Formats.Asn1.AsnValueReader #109019

vcsjones opened this issue Oct 18, 2024 · 1 comment
Assignees
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Formats.Asn1
Milestone

Comments

@vcsjones
Copy link
Member

vcsjones commented Oct 18, 2024

Background and motivation

Today .NET has two ways to read ASN.1 encoded data.

  1. AsnDecoder, which is a stateless decoder that can read slices of data and tell you how much is read. It operates on ReadOnlySpan<byte>.
  2. AsnReader which is a "higher abstracted", stateful reader. It operating on ReadOnlyMemory<byte> and works on top of AsnDecoder.

If you want a stateful decoder that works on ReadOnlySpan<byte>, you have to build it yourself on top of AsnDecoder.

Within .NET Libraries, that is exactly what we have done with the internal AsnValueReader.

This type is very useful as it makes working with ReadOnlySpan<byte> of ASN.1 encoded data easier. Because our own public APIs want to accept ReadOnlySpan<byte>, we need a way to read ReadOnlySpan<byte> of ASN.1 data. Other library authors are in a similar situation. If they want to expose ReadOnlySpan<byte> APIs and then interpret it as ASN.1, they are forced to use AsnDecoder, which is low level, or build their own abstraction.

The .NET Libraries in fact vastly prefer to use the internal AsnValueReader instead of the public AsnReader. This to me indicates there is a gap in the public API.

I propose then that we expose an AsnValueReader. Its API shape is identical to AsnReader, save for the fact that it accepts and returns ReadOnlySpan<byte> instead of ReadOnlyMemory<byte>, does not need a Clone, and it returns AsnValueReader for child readers.

API Proposal

namespace System.Formats.Asn1;

public partial ref struct AsnValueReader {
    public AsnValueReader(ReadOnlySpan<byte> data, AsnEncodingRules ruleSet, AsnReaderOptions options = default);
    public readonly bool HasData { get; }
    public readonly AsnEncodingRules RuleSet { get; }

    // Clone is not needed since AsnValueReader is a struct and can be cloned with copy-by-value.
    // We can have it for consistency sake, if we really want.
    //public AsnValueReader Clone();

    public readonly ReadOnlySpan<byte> PeekContentBytes();
    public readonly ReadOnlySpan<byte> PeekEncodedValue();
    public readonly Asn1Tag PeekTag();

    public byte[] ReadBitString(out int unusedBitCount, Asn1Tag? expectedTag = default);
    public bool ReadBoolean(Asn1Tag? expectedTag = default);
    public string ReadCharacterString(UniversalTagNumber encodingType, Asn1Tag? expectedTag = default);
    public ReadOnlySpan<byte> ReadEncodedValue();
    public ReadOnlySpan<byte> ReadEnumeratedBytes(Asn1Tag? expectedTag = default);
    public Enum ReadEnumeratedValue(Type enumType, Asn1Tag? expectedTag = default);
    public TEnum ReadEnumeratedValue<TEnum>(Asn1Tag? expectedTag = default) where TEnum : Enum;
    public DateTimeOffset ReadGeneralizedTime(Asn1Tag? expectedTag = default);
    public BigInteger ReadInteger(Asn1Tag? expectedTag = default);
    public ReadOnlySpan<byte> ReadIntegerBytes(Asn1Tag? expectedTag = default);
    public BitArray ReadNamedBitList(Asn1Tag? expectedTag = default);
    public Enum ReadNamedBitListValue(Type flagsEnumType, Asn1Tag? expectedTag = default);
    public TFlagsEnum ReadNamedBitListValue<TFlagsEnum>(Asn1Tag? expectedTag = default) where TFlagsEnum : Enum;
    public void ReadNull(Asn1Tag? expectedTag = default);
    public string ReadObjectIdentifier(Asn1Tag? expectedTag = default);
    public byte[] ReadOctetString(Asn1Tag? expectedTag = default);
    public AsnValueReader ReadSequence(Asn1Tag? expectedTag = default);
    public AsnValueReader ReadSetOf(bool skipSortOrderValidation, Asn1Tag? expectedTag = default);
    public AsnValueReader ReadSetOf(Asn1Tag? expectedTag = default);
    public DateTimeOffset ReadUtcTime(int twoDigitYearMax, Asn1Tag? expectedTag = default);
    public DateTimeOffset ReadUtcTime(Asn1Tag? expectedTag = default);
    public readonly void ThrowIfNotEmpty();
    public bool TryReadBitString(Span<byte> destination, out int unusedBitCount, out int bytesWritten, Asn1Tag? expectedTag = default);
    public bool TryReadCharacterString(Span<char> destination, UniversalTagNumber encodingType, out int charsWritten, Asn1Tag? expectedTag = default);
    public bool TryReadCharacterStringBytes(Span<byte> destination, Asn1Tag expectedTag, out int bytesWritten);
    public bool TryReadInt32(out int value, Asn1Tag? expectedTag = default);
    public bool TryReadInt64(out long value, Asn1Tag? expectedTag = default);
    public bool TryReadOctetString(Span<byte> destination, out int bytesWritten, Asn1Tag? expectedTag = default);
    public bool TryReadPrimitiveBitString(out int unusedBitCount, out ReadOnlySpan<byte> value, Asn1Tag? expectedTag = default);
    public bool TryReadPrimitiveCharacterStringBytes(Asn1Tag expectedTag, out ReadOnlySpan<byte> contents);
    public bool TryReadPrimitiveOctetString(out ReadOnlySpan<byte> contents, Asn1Tag? expectedTag = default);
    [CLSCompliantAttribute(false)]
    public bool TryReadUInt32(out uint value, Asn1Tag? expectedTag = default);
    [CLSCompliantAttribute(false)]
    public bool TryReadUInt64(out ulong value, Asn1Tag? expectedTag = default);
}

API Usage

It will be used the same as AsnReader.

Alternative Designs

I don't feel strongly about the name. We could call it AsnValueReader, ValueAsnReader, AsnRefReader, etc.


If we want to keep the "reader" APIs on AsnReader, we could instead introduce static APIs on it that accept a ref struct. Something like

public ref struct AsnValue {
    public AsnValue(ReadOnlySpan<byte> data, AsnEncodingRules ruleSet, AsnReaderOptions options = default);
    public bool HasData { get; }
    public AsnEncodingRules RuleSet { get; }
}

public partial class AsnReader { // Or on AsnDecoder
    public static ReadOnlySpan<byte> PeekContentBytes(ref readonly AsnValue asnValue); // Peek can be readonly
    public static string ReadObjectIdentifier(ref AsnValue asnValue, Asn1Tag? expectedTag = default);

    // Returns a new AsnValue which is a slice into the sequence
    public static AsnValue ReadSequence(ref AsnValue value, Asn1Tag? expectedTag = default);
}
    // etc

I don't love this because it does not meaningfully improve the situation over AsnDecoder, and it would make the sub-readers like ReadSequence be a little awkward. We have instance methods for a reason, so we should use them.

Risks

The largest risk with this is that since it is a mutable struct, the ergonomics of this make it a tad easy to clone it unintentionally. Such example might be forgetting to pass it by ref to a method, or the method having an API shape that forces the compiler to make a hidden copy. For example:

AsnValueReader reader; // Set up the reader
ConsumeSequence(reader);
// oops - reader is still unmodified by ConsumeSequence because we passed it by value

static void ConsumeSequence(AsnValueReader reader)
{
    AsnValueReader seq = reader.ReadSequence();
    // Do stuff
}

I think this risk is acceptable.

@vcsjones vcsjones added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Oct 18, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Oct 18, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Oct 18, 2024
@vcsjones vcsjones added area-System.Formats.Asn1 and removed untriaged New issue has not been triaged by the area owner needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Oct 18, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-formats-asn1, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

@vcsjones vcsjones self-assigned this Oct 18, 2024
@vcsjones vcsjones added this to the 10.0.0 milestone Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Formats.Asn1
Projects
None yet
Development

No branches or pull requests

1 participant