Use ROS<byte> instead of byte[] where it makes sense on S.S.C.Cose #66741
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
|
Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones Issue DetailsFor v1 of the library we shipped with many methods receiving Contributes to #62600 MVP+1.
|
bartonjs
reviewed
Mar 16, 2022
src/libraries/System.Security.Cryptography.Cose/ref/System.Security.Cryptography.Cose.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 16, 2022
...aries/System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseMessage.cs
Outdated
Show resolved
Hide resolved
jozkee
commented
Mar 23, 2022
.../System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseSign1Message.cs
Show resolved
Hide resolved
bartonjs
reviewed
Mar 25, 2022
src/libraries/System.Security.Cryptography.Cose/ref/System.Security.Cryptography.Cose.cs
Show resolved
Hide resolved
bartonjs
reviewed
Mar 25, 2022
...ies/System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseHeaderMap.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 25, 2022
...ies/System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseHeaderMap.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 25, 2022
.../System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseSign1Message.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 25, 2022
.../System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseSign1Message.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 25, 2022
.../System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseSign1Message.cs
Show resolved
Hide resolved
bartonjs
reviewed
Mar 25, 2022
.../System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseSign1Message.cs
Show resolved
Hide resolved
bartonjs
reviewed
Mar 25, 2022
| // we rent a bigger buffer if that's the case. | ||
| if (buffer.Length < expectedToBeSignedSize) | ||
| { | ||
| rentedToBeSignedBuffer = ArrayPool<byte>.Shared.Rent(expectedToBeSignedSize); |
There was a problem hiding this comment.
I'd like to see this rent go away and us be able to create the TBS with non-contiguous memory (and hashed using IncrementalHash).
But I expect that'll be forced upon us when we add support for Stream-based detached content; so it's OK for now.
bartonjs
reviewed
Mar 25, 2022
.../System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseSign1Message.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 25, 2022
.../System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseSign1Message.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 25, 2022
src/libraries/System.Security.Cryptography.Cose/tests/CoseHeaderMapTests.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 25, 2022
src/libraries/System.Security.Cryptography.Cose/tests/CoseSign1MessageTests.Sign.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 25, 2022
src/libraries/System.Security.Cryptography.Cose/tests/CoseSign1MessageTests.Sign.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 25, 2022
src/libraries/System.Security.Cryptography.Cose/tests/CoseSign1MessageTests.Sign.cs
Outdated
Show resolved
Hide resolved
jozkee
commented
Mar 31, 2022
src/libraries/System.Security.Cryptography/src/Microsoft/Win32/SafeHandles/NCryptSafeHandles.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 31, 2022
src/libraries/System.Security.Cryptography.Cose/tests/CoseTestHelpers.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 31, 2022
.../System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseSign1Message.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 31, 2022
.../System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseSign1Message.cs
Outdated
Show resolved
Hide resolved
* Add comment describing reusability of encoded protected headers * Cache toBeSigned
bartonjs
reviewed
Mar 31, 2022
src/libraries/System.Security.Cryptography.Cose/tests/CoseSign1MessageTests.Verify.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 31, 2022
src/libraries/System.Security.Cryptography.Cose/tests/CoseSign1MessageTests.Verify.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 31, 2022
src/libraries/System.Security.Cryptography.Cose/tests/CoseSign1MessageTests.Verify.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 31, 2022
src/libraries/System.Security.Cryptography.Cose/tests/CoseSign1MessageTests.Verify.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 31, 2022
src/libraries/System.Security.Cryptography.Cose/tests/CoseSign1MessageTests.Verify.cs
Outdated
Show resolved
Hide resolved
bartonjs
reviewed
Mar 31, 2022
src/libraries/System.Security.Cryptography.Cose/tests/CoseSign1MessageTests.Sign.cs
Outdated
Show resolved
Hide resolved
bartonjs
approved these changes
Mar 31, 2022
ghost
locked as resolved and limited conversation to collaborators
bartonjs
added
cryptographic-docs-impact
needs-further-triage
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For v1 of the library we shipped with many methods receiving
byte[]as argument, this was due to defer of thought of where we should properly useReadOnlySpan<byte>andReadOnlyMemory<byte>.This PR should also be a place for further discussion of Span vs Memory usage in the APIs and their interaction with System,Formats.Cbor APIs.
Contributes to #62600 MVP+1.