Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update CMS Version to 1 for CodeSigningCIPolicySigning #26993

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

TechyDhaval
Copy link

Description

Mandatory Checklist

  • SHOULD update ChangeLog.md file(s) appropriately
    • For SDK-based development mode, update src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.
      • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header in the past tense.
    • For autorest-based development mode, include the changelog in the PR description.
    • Should not change ChangeLog.md if no new release is required, such as fixing test case only.
  • SHOULD regenerate markdown help files if there is cmdlet API change. Instruction
  • SHOULD have proper test coverage for changes in pull request.
  • SHOULD NOT adjust version of module manually in pull request

Copy link

Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status.

Copy link
Contributor

Thank you for your contribution TechyDhaval! We will review the pull request and get back to you soon.

// Verify that the structure matches the expected pattern
if (signedData[i + 2 + sequenceLength] == cmsVersionTag && // tag for cmsVersion
signedData[i + 3 + sequenceLength] == cmsVersionLength && // CMS version value length
signedData[cmsVersionIndex] == cmsVersionV3) // Version is 3, needs to be updated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the cmsVersion is 1 instead of 3? From what I understand, it would throw an exception.

Perhaps its a better experience to just return the value unchanged?

@TechyDhaval TechyDhaval force-pushed the dhjoshi/CMSVersionUpdate branch from 7ff9b7d to a483492 Compare January 15, 2025 01:42
@Jaxelr
Copy link
Member

Jaxelr commented Jan 15, 2025

@TechyDhaval LGTM.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants