From d31bb7ae7cbfb037045f272d7e077c5267c87306 Mon Sep 17 00:00:00 2001 From: Pichatorn-A4K Date: Fri, 11 Sep 2026 02:17:52 +0700 Subject: [PATCH] Stop dereferencing a null Errors list on unexpected response bodies Every caller in CertCentralClient did Errors errors = JsonConvert.DeserializeObject(response.Response); x.Errors = errors.errors; which produces a null Errors list, or throws, in three cases that are all reachable against the live API: * an empty body - DeserializeObject("") returns null outright, so the assignment itself NREs; * a JSON body that is not the {"errors":[...]} envelope - .errors is null. GET /services/v2/account/metadata answers literally {} on an account with no custom fields; * a non-JSON body - a proxy or WAF error page raises JsonReaderException from *outside* Request()'s try/catch, so it does not surface as a CA error at all. DigiCert's responses are fronted by a CDN, so an HTML body is a real shape on the wire. Downstream there are eleven Errors[0] / .First() / .Count sites, so any of those cases became a NullReferenceException instead of a diagnosable message. ParseErrors() never returns null and never throws, and labels which of the three cases occurred. CertCentralResponse also carries the HTTP status now, so the message can name it - previously 4xx and 5xx were indistinguishable to every caller. --- .../Client/CertCentralClient.cs | 131 ++++++++++++------ 1 file changed, 87 insertions(+), 44 deletions(-) diff --git a/digicert-certcentral-caplugin/Client/CertCentralClient.cs b/digicert-certcentral-caplugin/Client/CertCentralClient.cs index 753f54f..07112bf 100644 --- a/digicert-certcentral-caplugin/Client/CertCentralClient.cs +++ b/digicert-certcentral-caplugin/Client/CertCentralClient.cs @@ -64,6 +64,47 @@ public CertCentralResponse() public bool Success { get; set; } public string Response { get; set; } + public int StatusCode { get; set; } + } + + /// + /// Turn a DigiCert response body into a non-empty list of Errors, without throwing. + /// + /// Callers used to do `JsonConvert.DeserializeObject<Errors>(body)` and read `.errors` + /// directly, which yields null in three reachable cases - an empty body, a JSON body that is + /// not the {"errors":[...]} envelope (GET /services/v2/account/metadata answers literally {} + /// on an account with no custom fields), and a non-JSON body such as a proxy or WAF error page + /// - so the Errors[0] / .First() / .Count sites downstream threw NullReferenceException, and a + /// non-JSON body threw JsonReaderException from outside Request()'s try/catch. + /// + internal static List ParseErrors(string body, int statusCode) + { + string codeSuffix = statusCode > 0 ? $" (HTTP {statusCode})" : ""; + + if (string.IsNullOrWhiteSpace(body)) + { + return new List { new Error { code = "empty_response_body", message = $"DigiCert returned no response body{codeSuffix}." } }; + } + + try + { + Errors parsed = JsonConvert.DeserializeObject(body); + if (parsed?.errors != null && parsed.errors.Count > 0) + { + return parsed.errors; + } + return new List { new Error { code = "unrecognized_error_response", message = $"DigiCert returned a response with no 'errors' array{codeSuffix}: {Truncate(body)}" } }; + } + catch (JsonException) + { + return new List { new Error { code = "non_json_response", message = $"DigiCert returned a non-JSON response{codeSuffix}: {Truncate(body)}" } }; + } + } + + private static string Truncate(string s) + { + s = s.Replace("\r", " ").Replace("\n", " ").Trim(); + return s.Length <= 500 ? s : s.Substring(0, 500) + "\u2026"; } private CertCentralResponse Request(CertCentralBaseRequest request) @@ -118,6 +159,7 @@ private CertCentralResponse Request(CertCentralBaseRequest request, string param { string respString = new StreamReader(objResponse.GetResponseStream()).ReadToEnd(); oCertCertResponse.Response = respString; + oCertCertResponse.StatusCode = (int)objResponse.StatusCode; Logger.LogTrace($"CertCentral CA (Request ID: {reqID}) has returned Response '{objResponse.StatusCode}: {respString}"); } } @@ -141,6 +183,7 @@ private CertCentralResponse Request(CertCentralBaseRequest request, string param string errorString = reader.ReadToEnd(); oCertCertResponse.Success = false; oCertCertResponse.Response = errorString; + oCertCertResponse.StatusCode = (int)errorResponse.StatusCode; Logger.LogTrace($"CertCentral CA (Request ID: {reqID}) has returned Response '{errorResponse.StatusCode}: {errorString}"); } } @@ -169,9 +212,9 @@ public ListOrganizationsResponse ListOrganizations(ListOrganizationsRequest requ if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); listOrganizationsResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - listOrganizationsResponse.Errors = errors.errors; + listOrganizationsResponse.Errors = errors; } else listOrganizationsResponse = JsonConvert.DeserializeObject(response.Response); @@ -187,9 +230,9 @@ public ListDomainsResponse ListDomains(ListDomainsRequest request) if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); listDomainsResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - listDomainsResponse.Errors = errors.errors; + listDomainsResponse.Errors = errors; } else listDomainsResponse = JsonConvert.DeserializeObject(response.Response); @@ -205,9 +248,9 @@ public ListContainersResponse ListContainers(ListContainersRequest request) if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); listContainersResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - listContainersResponse.Errors = errors.errors; + listContainersResponse.Errors = errors; } else { @@ -225,9 +268,9 @@ public ListDuplicatesResponse ListDuplicates(ListDuplicatesRequest duplicatesReq if (!ccResponse.Success) { - Errors errors = JsonConvert.DeserializeObject(ccResponse.Response); + List errors = ParseErrors(ccResponse.Response, ccResponse.StatusCode); duplicatesResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - duplicatesResponse.Errors = errors.errors; + duplicatesResponse.Errors = errors; } else { @@ -245,9 +288,9 @@ public ListReissueResponse ListReissues(ListReissueRequest reissueRequest) if (!ccResponse.Success) { - Errors errors = JsonConvert.DeserializeObject(ccResponse.Response); + List errors = ParseErrors(ccResponse.Response, ccResponse.StatusCode); reissueResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - reissueResponse.Errors = errors.errors; + reissueResponse.Errors = errors; } else { @@ -265,9 +308,9 @@ public ListRequestsResponse ListRequests(ListRequestsRequest request) if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); listRequestsResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - listRequestsResponse.Errors = errors.errors; + listRequestsResponse.Errors = errors; } else listRequestsResponse = JsonConvert.DeserializeObject(response.Response); @@ -283,9 +326,9 @@ public ListMetadataResponse ListMetadata(ListMetadataRequest request) if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); listMetadataResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - listMetadataResponse.Errors = errors.errors; + listMetadataResponse.Errors = errors; } else listMetadataResponse = JsonConvert.DeserializeObject(response.Response); @@ -304,9 +347,9 @@ public OrderResponse OrderCertificate(OrderRequest request) OrderResponse orderResponse = new OrderResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); orderResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - orderResponse.Errors = errors.errors; + orderResponse.Errors = errors; } else orderResponse = JsonConvert.DeserializeObject(response.Response); @@ -325,9 +368,9 @@ public OrderResponse OrderSmimeCertificate(OrderSmimeRequest request) OrderResponse orderResponse = new OrderResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); orderResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - orderResponse.Errors = errors.errors; + orderResponse.Errors = errors; } else orderResponse = JsonConvert.DeserializeObject(response.Response); @@ -345,9 +388,9 @@ public OrderResponse ReissueCertificate(ReissueRequest request) OrderResponse reissueResponse = new OrderResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); reissueResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - reissueResponse.Errors = errors.errors; + reissueResponse.Errors = errors; } else { @@ -367,9 +410,9 @@ public OrderResponse DuplicateCertificate(DuplicateRequest request) OrderResponse duplicateResponse = new OrderResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); duplicateResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - duplicateResponse.Errors = errors.errors; + duplicateResponse.Errors = errors; } else { @@ -386,9 +429,9 @@ public RevokeCertificateResponse RevokeCertificate(RevokeCertificateRequest requ RevokeCertificateResponse revokeOrderResponse = new RevokeCertificateResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); revokeOrderResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - revokeOrderResponse.Errors = errors.errors; + revokeOrderResponse.Errors = errors; } else revokeOrderResponse = JsonConvert.DeserializeObject(response.Response); @@ -403,9 +446,9 @@ public RevokeCertificateResponse RevokeCertificate(RevokeCertificateByOrderReque RevokeCertificateResponse revokeOrderResponse = new RevokeCertificateResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); revokeOrderResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - revokeOrderResponse.Errors = errors.errors; + revokeOrderResponse.Errors = errors; } else revokeOrderResponse = JsonConvert.DeserializeObject(response.Response); @@ -420,9 +463,9 @@ public UpdateRequestStatusResponse UpdateRequestStatus(UpdateRequestStatusReques UpdateRequestStatusResponse updateRequestResponse = new UpdateRequestStatusResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); updateRequestResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - updateRequestResponse.Errors = errors.errors; + updateRequestResponse.Errors = errors; } else { @@ -443,9 +486,9 @@ public DVCheckDCVResponse DVCheckDCV(DVCheckDCVRequest request) DVCheckDCVResponse checkDCVResponse = new DVCheckDCVResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); checkDCVResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - checkDCVResponse.Errors = errors.errors; + checkDCVResponse.Errors = errors; } else { @@ -461,9 +504,9 @@ public CertificateChainResponse GetCertificateChain(CertificateChainRequest requ CertificateChainResponse chainResponse = new CertificateChainResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); chainResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - chainResponse.Errors = errors.errors; + chainResponse.Errors = errors; } else { @@ -479,9 +522,9 @@ public StatusChangesResponse StatusChanges(StatusChangesRequest request) StatusChangesResponse statusChangeResponse = new StatusChangesResponse(); if (!certResponse.Success) { - Errors errors = JsonConvert.DeserializeObject(certResponse.Response); + List errors = ParseErrors(certResponse.Response, certResponse.StatusCode); statusChangeResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - statusChangeResponse.Errors = errors.errors; + statusChangeResponse.Errors = errors; } else { @@ -496,9 +539,9 @@ public DownloadCertificateByFormatResponse DownloadCertificateByFormat(DownloadC DownloadCertificateByFormatResponse dlCertificateRequestResponse = new DownloadCertificateByFormatResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); dlCertificateRequestResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - dlCertificateRequestResponse.Errors = errors.errors; + dlCertificateRequestResponse.Errors = errors; } else { @@ -545,9 +588,9 @@ public ListCertificateOrdersResponse ListAllCertificateOrders(bool ignoreExpired ListCertificateOrdersResponse listCertificateResponse = new ListCertificateOrdersResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); listCertificateResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - listCertificateResponse.Errors = errors.errors; + listCertificateResponse.Errors = errors; return listCertificateResponse; } @@ -571,9 +614,9 @@ public ViewCertificateOrderResponse ViewCertificateOrder(ViewCertificateOrderReq if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); viewCertResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - viewCertResponse.Errors = errors.errors; + viewCertResponse.Errors = errors; } else { @@ -596,9 +639,9 @@ public CertificateTypeDetailsResponse GetCertificateTypeDetails(CertificateTypeD if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); detailsResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - detailsResponse.Errors = errors.errors; + detailsResponse.Errors = errors; } else { @@ -620,9 +663,9 @@ public CertificateTypesResponse GetAllCertificateTypes() if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); allTypes.Status = CertCentralBaseResponse.StatusType.ERROR; - allTypes.Errors = errors.errors; + allTypes.Errors = errors; } else {