dotnet / runtime

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

[API Proposal]: CngProperty constructor with ReadOnlySpan<byte> overload #69061

Open vcsjones opened 2 years ago

vcsjones commented 2 years ago

Background and motivation

There is an internal constructor on CngProperty that accepts the value as a ReadOnlySpan<byte>.

https://github.com/dotnet/runtime/blob/be20c44826bc55d201c406f4ac007d24ed7a9137/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngProperty.cs#L30

I propose making this public so that callers that have data as a ReadOnlySpan<byte> don't need to convert it to a byte[] first, and then get copied, again, since it keeps a defensive copy.

API Proposal

namespace System.Security.Cryptography {
    public partial struct CngProperty {
-       internal CngProperty(string name, ReadOnlySpan<byte> value, CngPropertyOptions options);
+       public CngProperty(string name, ReadOnlySpan<byte> value, CngPropertyOptions options);
    }
}

API Usage

ReadOnlySpan<byte> value = "I need a nap"u8;
CngProperty test = new(
    "CustomLabel",
    value,
    CngPropertyOptions. CustomProperty);

Alternative Designs

No response

Risks

No response

ghost commented 2 years ago

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

Issue Details
### Background and motivation There is an internal constructor on `CngProperty` that accepts the `value` as a `ReadOnlySpan`. https://github.com/dotnet/runtime/blob/be20c44826bc55d201c406f4ac007d24ed7a9137/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngProperty.cs#L30 I propose making this public so that callers that have data as a `ReadOnlySpan` don't need to convert it to a `byte[]` first, and then get copied, again, since it keeps a defensive copy. ### API Proposal ```diff namespace System.Security.Cryptography { public partial struct CngProperty { - internal CngProperty(string name, ReadOnlySpan value, CngPropertyOptions options); + public CngProperty(string name, ReadOnlySpan value, CngPropertyOptions options); } } ``` ### API Usage ```csharp ReadOnlySpan value = "I need a nap"u8; CngProperty test = new( "CustomLabel", value, CngPropertyOptions. CustomProperty); ``` ### Alternative Designs _No response_ ### Risks _No response_
Author: vcsjones
Assignees: -
Labels: `api-suggestion`, `area-System.Security`
Milestone: -
bartonjs commented 1 year ago

Video

Looks good as proposed.

There may be some interesting edge cases around null vs empty (and therefore the default ReadOnlySpan vs a zero-width slice) which probably warrant testing (or proving that null and empty are the same to CNG)

namespace System.Security.Cryptography {
    public partial struct CngProperty {
       public CngProperty(string name, ReadOnlySpan<byte> value, CngPropertyOptions options);
    }
}
deeprobin commented 1 year ago

I can take a look at it. You are welcome to assign me :smile: