Skip to content

Add winBindings for crypt32 and replace raw ctypes calls. Move some structures into winBindings.crypt32 also.#18956

Merged
michaelDCurran merged 7 commits into
masterfrom
crypt32
Sep 24, 2025
Merged

Add winBindings for crypt32 and replace raw ctypes calls. Move some structures into winBindings.crypt32 also.#18956
michaelDCurran merged 7 commits into
masterfrom
crypt32

Conversation

@michaelDCurran

@michaelDCurran michaelDCurran commented Sep 22, 2025

Copy link
Copy Markdown
Member

Link to issue number:

None

Summary of the issue:

Continuing to move to 64 bit, crypt32.dll is one of the last dlls that requires correct ctypes definitions.

Description of user facing changes:

None

Description of developer facing changes:

Some structures have been moved from updateCheck to winBindings.crypt32.

Description of development approach:

Add winBindings.crypt32 required definitions, and move some structures.

Testing strategy:

Spoof buildVersion.updateVersionType to stable, and checked for and downloaded an update. Note however that as NV Access's TLS certificate is trusted by my system, the changed code shouldn't have been exercised.

System and unit tests.

Further testing tbd

Known issues with pull request:

None

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@SaschaCowley SaschaCowley marked this pull request as ready for review September 22, 2025 06:18
@SaschaCowley SaschaCowley requested a review from a team as a code owner September 22, 2025 06:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves Windows crypto32.dll definitions from raw ctypes calls to a structured winBindings module as part of the 64-bit migration effort. The changes consolidate crypto32 API bindings into a dedicated module with proper type definitions and documentation.

  • Adds new winBindings.crypt32 module with structured API definitions and documentation
  • Moves CERT_USAGE_MATCH and CERT_CHAIN_PARA structures from updateCheck to winBindings.crypt32
  • Replaces raw ctypes calls with structured bindings in certificate validation code

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
user_docs/en/changes.md Documents deprecation of moved symbols from updateCheck to winBindings.crypt32
source/winBindings/crypt32.py New module with structured crypt32 API definitions, type aliases, and function bindings
source/updateCheck.py Updates to use new crypt32 bindings and adds deprecation handling for moved symbols

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread source/winBindings/crypt32.py
@seanbudd seanbudd requested review from seanbudd and removed request for SaschaCowley September 22, 2025 23:21
@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Sep 23, 2025
Comment thread source/updateCheck.py
Comment thread user_docs/en/changes.md Outdated
@michaelDCurran michaelDCurran merged commit 3f9151f into master Sep 24, 2025
40 checks passed
@michaelDCurran michaelDCurran deleted the crypt32 branch September 24, 2025 00:20
@github-actions github-actions Bot added this to the 2026.1 milestone Sep 24, 2025
Comment thread user_docs/en/changes.md
* The `inputButtonCaps` property on `hwIo.hid.Hid` objects now correctly returns an array of `hidpi.HIDP_BUTTON_CAPS` structures rather than HIDP_VALUE_CAPS` structures. (#18902)
* `speech.speech.IDT_TONE_DURATION` has been removed.
Call `speech.speech.getIndentToneDuration` instead. (#18898)
* the `rgpszUsageIdentifier` member of the `updateCheck.CERT_USAGE_MATCH` struct is now of type `POINTER(LPSTR)` rather than `c_void_p` to correctly align with Microsoft documentation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you also please make sure to announce the api breaking changes and deprecations on the mailing list

Suggested change
* the `rgpszUsageIdentifier` member of the `updateCheck.CERT_USAGE_MATCH` struct is now of type `POINTER(LPSTR)` rather than `c_void_p` to correctly align with Microsoft documentation.
* The `rgpszUsageIdentifier` member of the `updateCheck.CERT_USAGE_MATCH` struct is now of type `POINTER(LPSTR)` rather than `c_void_p` to correctly align with Microsoft documentation. (#18956)

@SaschaCowley SaschaCowley mentioned this pull request Apr 1, 2026
5 tasks
SaschaCowley added a commit that referenced this pull request Apr 9, 2026
### Summary of the issue:

The user guide and changelog for NVDA 2026.1 contain various typos,
inconsistent capitalisation, broken anchors, incorrect key bindings, and
minor wording issues that should be resolved before the translation
freeze.

### Description of user facing changes:

- Fixed spelling and grammar throughout changes.md and userGuide.md
(e.g. "recognising" → "recognizing", "aides" → "aids", "inside of" →
"inside").
- Corrected inconsistent capitalisation (e.g. "unicode" → "Unicode",
"32bit" → "32-bit", "both" → "Both" at start of list items, consistent
"Enhanced mode" / "Simple mode" / "Character mode").
- Fixed incorrect key binding in the MathCAT navigation table: "Describe
placemarker" now correctly reads control+shift+1 through control+shift+0
instead of control+shift+1 through shift+0.
- Fixed a broken anchor {MathSpeechStyle} → {#MathSpeechStyle} and
corrected cross-reference anchors (#reportSpellingErrors →
#ReportSpellingErrors).
- Added missing heading anchor {#MathTypicalUse} to the "Typical Use"
section.
- Removed the obsolete "AsCompound" MathCAT chemistry option and renamed
"SpellOut" to "Spell it out" to match the current MathCAT UI.
- Improved clarity of wording in the spelling/grammar error reporting
settings, add-on copy to system configuration, and add-on store
changelog sections.
- Added missing issue reference (#18956) to a changelog entry.
- Fixed minor formatting issues (missing space before backtick, extra
space before comma).

### Description of developer facing changes:

None

### Description of development approach:

Instructed Claude Code to review `user_docs/en/changes.md` and
`user_docs/en/userGuide.md`. Manually actioned its suggestions.

### Testing strategy:

- Manual review of the rendered documentation for correctness and
consistency.
- Verified that all cross-reference anchors resolve correctly.

### Known issues with pull request:

None

---------

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>
Co-authored-by: Luke Davis <8139760+XLTechie@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-breaking-change conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants