Automatically port some remarks for System.Security.Cryptography (.NET Core 2.X) #3176
Conversation
carlossanlop
added
new-content
carlossanlop
requested review from
GrabYourPitchforks,
bartonjs,
mairaw and
rpetrusha
| ## Remarks | ||
| May be `null` or empty to indicate that no hash algorithm is applicable. |
There was a problem hiding this comment.
null and empty have no special defined meaning on this type. It's simply a struct, and therefore can have been initialized to default.
Remove this remark.
There was a problem hiding this comment.
This is just a repetition of the <value> section, where it is better placed. This should be deleted, and the <value> section modified to reflect the comment by @bartonjs.
rpetrusha
left a comment
rpetrusha
left a comment
There was a problem hiding this comment.
I've left some comments for you to address, @carlossanlop.
| ## Remarks | ||
| May be `null` or empty to indicate that no hash algorithm is applicable. |
There was a problem hiding this comment.
This is just a repetition of the <value> section, where it is better placed. This should be deleted, and the <value> section modified to reflect the comment by @bartonjs.
| <MemberValue>1</MemberValue> | ||
| <Docs> | ||
| <summary>Optimal Asymmetric Encryption Padding. It is recommended for new applications.</summary> | ||
| <remarks> |
There was a problem hiding this comment.
This should be deleted. It appears in the <MemberGroup> section, and individual enum <remarks> sections are suppressed from the build.
There was a problem hiding this comment.
I don't see a MemberGroup, but I think you meant the general remark for the whole Type, @rpetrusha. I'll remove it.
| <MemberValue>0</MemberValue> | ||
| <Docs> | ||
| <summary>PKCS #1 v1.5. It is supported for compatibility with existing applications.</summary> | ||
| <remarks> |
| <MemberValue>0</MemberValue> | ||
| <Docs> | ||
| <summary>PKCS #1 v1.5</summary> | ||
| <remarks> |
| <MemberValue>1</MemberValue> | ||
| <Docs> | ||
| <summary>Probabilistic Signature Scheme</summary> | ||
| <remarks> |
There was a problem hiding this comment.
This should be deleted; see above. The PSS correction is already present in the text.
Co-Authored-By: Jeremy Barton <[email protected]>
| <Docs> | ||
| <summary>Gets the underlying string representation of the algorithm name.</summary> | ||
| <value>The string representation of the algorithm name, or <see langword="null" /> or <see cref="F:System.String.Empty" /> if no hash algorithm is available.</value> | ||
| <value>The string representation of the algorithm name, or <see langword="default" /> if no hash algorithm is available.</value> |
There was a problem hiding this comment.
@bartonjs I addressed your comment about null and empty, by removing the remark and (as suggested by @rpetrusha) modifying the value: substituted null and empty with default. Please confirm this was the correct change.
There was a problem hiding this comment.
I'd just leave it as it is entirely, or delete everything in the "or" clause.
Like all null values it's "do nothing", "do the default thing", or "be invalid", determined by the called method.
| `Pkcs1` corresponds to the RSASSA-PKCS1-v1.5 signature scheme of the [PKCS #1: RSA Encryption Standard](http://www.emc.com/emc-plus/rsa-labs/standards-initiatives/pkcs-rsa-cryptography-standard.htm). It is supported for compatibility with existing applications. | ||
| `Pss` corresponds to the Probabilistic Signature Scheme (PSS) of the [PKCS #1: RSA Encryption Standard](http://www.emc.com/emc-plus/rsa-labs/standards-initiatives/pkcs-rsa-cryptography-standard.htm). It is recommended for new applications. | ||
| `Pss` corresponds to the Probabilistic Signature Scheme (RSASSA-PSS) of the [PKCS #1: RSA Encryption Standard](http://www.emc.com/emc-plus/rsa-labs/standards-initiatives/pkcs-rsa-cryptography-standard.htm). It is recommended for new applications. |
There was a problem hiding this comment.
@bartonjs I addressed the suggestion you originally gave me in the remark for the PPS enum value by adding your acronym to the general description found in the enum Type. Please let me know if this was the correct change to make.
There was a problem hiding this comment.
"Probabilistic Signature Scheme" is "PSS". RSA-signature-using-PSS is "RSASSA-PSS" (RSA Signature Scheme with Appendix - Probabilistic Signature Scheme).
So where this is used, it was correct as-is, and is incorrect with the change.
If we want to include RSASSA-PSS in the string, it'd be something like
| `Pss` corresponds to the Probabilistic Signature Scheme (RSASSA-PSS) of the [PKCS #1: RSA Encryption Standard](http://www.emc.com/emc-plus/rsa-labs/standards-initiatives/pkcs-rsa-cryptography-standard.htm). It is recommended for new applications. | |
| `Pss` corresponds to the RSASSA-PSS signature scheme of the [PKCS #1: RSA Encryption Standard](http://www.emc.com/emc-plus/rsa-labs/standards-initiatives/pkcs-rsa-cryptography-standard.htm). It is recommended for new applications. |
I'd just leave it as-is.
|
I'll merge this now, @carlossanlop. |
mairaw
removed
the
waiting-on-reviews
4 participants
Total modified files: 3
- D:\dotnet-api-docs\xml\System.Security.Cryptography\RSAEncryptionPaddingMode.xml
- D:\dotnet-api-docs\xml\System.Security.Cryptography\RSASignaturePaddingMode.xml
- D:\dotnet-api-docs\xml\System.Security.Cryptography\HashAlgorithmName.xml
Total modified assemblies: 3
- System.Security.Cryptography.RSAEncryptionPaddingMode
- System.Security.Cryptography.RSASignaturePaddingMode
- System.Security.Cryptography.HashAlgorithmName
Total modified containers: 0
Total modified APIs: 5
- F:System.Security.Cryptography.RSAEncryptionPaddingMode.Pkcs1
- F:System.Security.Cryptography.RSAEncryptionPaddingMode.Oaep
- F:System.Security.Cryptography.RSASignaturePaddingMode.Pkcs1
- F:System.Security.Cryptography.RSASignaturePaddingMode.Pss
- P:System.Security.Cryptography.HashAlgorithmName.Name
Total problematic APIs: 0
Total added exceptions: 0
Total modified individual elements: 5