Dev 1.2 #40
Dev 1.2 #40
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the certificate revocation flow to include a numeric revocation code parameter and aligns the proxy and client implementations with the new Sectigo API format.
- Added
revcodeargument to the client and proxy layers - Extended
RevokeReasonToStringto handle code 0 and throw on invalid values - Updated CHANGELOG to 1.2.2 with revocation API change note
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/SectigoCAProxy/SectigoCAProxy.cs | Pass new revocationReason code to API call and revise string mapping logic |
| src/SectigoCAProxy/Client/SectigoApiClient.cs | Add revcode parameter and include it in JSON payload |
| CHANGELOG.md | Bump version to 1.2.2 and note the API call change |
Comments suppressed due to low confidence (2)
CHANGELOG.md:26
- [nitpick] The changelog header format differs from existing entries and lacks a date. Consider using a consistent header style (e.g.,
## [1.2.2] - YYYY-MM-DD) to match the project’s convention.
# 1.2.2
src/SectigoCAProxy/Client/SectigoApiClient.cs:142
- The method signature now includes
revcodebut existing XML comments or summaries may be outdated. Update the method documentation to describe the new parameter.
public async Task<bool> RevokeSslCertificateById(int sslId, int revcode, string revreason)
| @@ -569,7 +569,7 @@ public override int Revoke(string caRequestID, string hexSerialNumber, uint revo | |||
| { | |||
| Logger.MethodEntry(ILogExtensions.MethodLogLevel.Debug); | |||
|
|
|||
| var response = Task.Run(async () => await Client.RevokeSslCertificateById(int.Parse(caRequestID), RevokeReasonToString(revocationReason))).Result; | |||
| var response = Task.Run(async () => await Client.RevokeSslCertificateById(int.Parse(caRequestID), (int)revocationReason, RevokeReasonToString(revocationReason))).Result; | |||
There was a problem hiding this comment.
Blocking on an async Task with .Result inside Task.Run can lead to deadlocks. Consider making Revoke async and using await, or use GetAwaiter().GetResult() if synchronous behavior is required.
Copilot uses AI. Check for mistakes.
| default: | ||
| return "Unspecified"; | ||
| throw new Exception($"Invalid revocation code: {revokeType.ToString()}. Valid values are 0,1,3-5"); | ||
| } |
There was a problem hiding this comment.
Throwing a generic Exception makes it harder for callers to handle specific errors. Use ArgumentOutOfRangeException (or a custom exception) to clearly indicate an invalid code.
Copilot uses AI. Check for mistakes.
| default: | ||
| return "Unspecified"; | ||
| throw new Exception($"Invalid revocation code: {revokeType.ToString()}. Valid values are 0,1,3-5"); |
There was a problem hiding this comment.
Hard-coding the list of valid codes means it must be kept in sync with switch cases. Consider deriving valid values programmatically or centralizing the list to avoid drift.
Copilot uses AI. Check for mistakes.
No description provided.