IISU: fixed a null object reference bug with the legacy SANs resolver#184
IISU: fixed a null object reference bug with the legacy SANs resolver#184KueYang wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses a null object reference when resolving legacy SANs from config.JobProperties["SAN"], ensuring reenrollment SAN resolution doesn’t throw when that legacy value is null/empty.
Changes:
- Hardened
ResolveSANStringto handle a null legacy SAN value fromJobProperties. - Updated SAN resolver unit tests to exercise additional legacy-SAN inputs (null/empty).
- Added a
3.0.2changelog entry describing the fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
WindowsCertStore.UnitTests/SANsUnitTests.cs |
Expands test coverage for legacy SAN handling, but currently asserts an incorrect expected value for null/empty cases. |
IISU/ClientPSCertStoreReEnrollment.cs |
Prevents null dereference when JobProperties["SAN"] exists but is null/whitespace. |
CHANGELOG.md |
Documents the SAN resolver null-reference fix in version 3.0.2. |
Comments suppressed due to low confidence (1)
WindowsCertStore.UnitTests/SANsUnitTests.cs:83
- This theory includes empty-string and null legacy SAN values, but the assertion always expects the legacy SAN string. With the updated resolver logic, null/whitespace legacy values should result in an empty string (falling through to the "none" case), so these InlineData cases will fail. Consider splitting into separate tests or asserting based on the input (e.g., expected empty when null/"" and expected legacy string when provided).
[Theory]
[InlineData("dns=legacy.example.com&dns=old.example.com")]
[InlineData("")]
[InlineData(null)]
public void ResolveSanString_UsesLegacySAN_WhenConfigSANsMissing(string legacySan)
{
// Arrange
var config = new ReenrollmentJobConfiguration
{
JobProperties = new Dictionary<string, object>
{
{ "SAN", legacySan }
},
SANs = new Dictionary<string, string[]>()
};
// Act
string result = enrollment.ResolveSANString(config);
// Assert
Assert.Equal("dns=legacy.example.com&dns=old.example.com", result);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rcpokorny
left a comment
There was a problem hiding this comment.
Hi Kue, I have reviewed your changes, and they have been updated in the upcoming release of 4.0. Additionally, we don't merge PRs directly into main, these changes would go into the Release-3.0 branch.
Also, I want to make sure there was a bug report or dev ticket indicating there was a problem so we can track it.
I appreciate your efforts on finding this issue.
|
@rcpokorny thank you for the update. I was not sure of the process of creating a pull request and pointed to main as I figure my PR will get blocked and reviewed, with additional guidance on what to do next. |
|
If there is a problem a Dev Ticket normally gets created and then gets assigned, fixed, scheduled and released.
I appreciate your effort in finding/fixing the bug.
If you have further questions, let me know.
Thanks,
Bob
From: Kue Yang ***@***.***>
Sent: Friday, May 15, 2026 11:11 AM
To: Keyfactor/iis-orchestrator ***@***.***>
Cc: Bob Pokorny ***@***.***>; Mention ***@***.***>
Subject: Re: [Keyfactor/iis-orchestrator] IISU: fixed a null object reference bug with the legacy SANs resolver (PR #184)
KueYang left a comment (Keyfactor/iis-orchestrator#184) @rcpokorny thank you for the update. I was not sure of the process of creating a pull request and pointed to main as I figure my PR will get blocked and reviewed, with additional guidance
[https://avatars.githubusercontent.com/u/9170155?s=20&v=4]KueYang left a comment (Keyfactor/iis-orchestrator#184)<#184 (comment)>
@rcpokorny<https://github.com/rcpokorny> thank you for the update. I was not sure of the process of creating a pull request and pointed to main as I figure my PR will get blocked and reviewed, with additional guidance on what to do next.
—
Reply to this email directly, view it on GitHub<#184 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ANII75OII6BRC7X2SDESBHT4246RNAVCNFSM6AAAAACYTM7APGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DINRRGM4DANRRHE>.
Triage notifications on the go with GitHub Mobile for iOS<https://urldefense.com/v3/__https:/apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675__;!!BjbSd3t9V7AnTp3tuV-82YaK!xpPKpz4uoq-BUiiVLX3Gl2slK9AY9-VIu_KK36YxC35iG5YoSggsaIhWur3ZwzEer7xpT4nIQJAB_LsbshvfIOf2Iu0UIA$> or Android<https://urldefense.com/v3/__https:/play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign*3Dnotification-email*26utm_medium*3Demail*26utm_source*3Dgithub__;JSUlJSU!!BjbSd3t9V7AnTp3tuV-82YaK!xpPKpz4uoq-BUiiVLX3Gl2slK9AY9-VIu_KK36YxC35iG5YoSggsaIhWur3ZwzEer7xpT4nIQJAB_LsbshvfIOd0PWh_eQ$>.
You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>
|
Summary
Fixed a object null exception bug in the reenrollment SANs resolver logic.