Skip to content

Raise an error if peer certificate chain APIs aren't available#149

Merged
sethmlarson merged 10 commits into
mainfrom
graceful-missing-apis
Aug 21, 2024
Merged

Raise an error if peer certificate chain APIs aren't available#149
sethmlarson merged 10 commits into
mainfrom
graceful-missing-apis

Conversation

@sethmlarson

@sethmlarson sethmlarson commented Aug 14, 2024

Copy link
Copy Markdown
Owner

@sethmlarson sethmlarson requested a review from davisagli as a code owner August 14, 2024 17:40

@davisagli davisagli left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Well, I shouldn't have approved it, because it's breaking tests.

@sethmlarson

Copy link
Copy Markdown
Owner Author

@davisagli Yeah the way I'm checking for existence of that method needs some work, figuring out an alternative that works across versions. Appreciate the quick review! :)

@sethmlarson

Copy link
Copy Markdown
Owner Author

I wasn't able to quickly get GraalPy to work with setup-python, even after it installed there was a strange error without a traceback. I tried the module locally and it appears to work as expected:

$ graalpy
Python 3.10.13 (Fri Mar 08 15:15:20 UTC 2024)
[Graal, Oracle GraalVM, Java 22] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import truststore
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/sethmlarson/truststore/src/truststore/__init__.py", line 25, in <module>
    raise ImportError(
ImportError: truststore requires peer certificate APIs to be available

@sethmlarson sethmlarson requested a review from davisagli August 15, 2024 15:00
@sethmlarson

Copy link
Copy Markdown
Owner Author

The failure on Windows has been around for a long time, we should dig into that one but this PR can move forward without it.

Comment thread src/truststore/__init__.py Outdated
Comment thread src/truststore/__init__.py Outdated
@sethmlarson sethmlarson requested a review from davisagli August 19, 2024 20:20
@sethmlarson sethmlarson merged commit 1407864 into main Aug 21, 2024
@sethmlarson sethmlarson deleted the graceful-missing-apis branch August 21, 2024 13:25
@sethmlarson

Copy link
Copy Markdown
Owner Author

Thanks for the review @davisagli!

SaschaCowley pushed a commit to nvaccess/nvda that referenced this pull request Jul 18, 2025
Fixes #18460 
Fixup of #18354 

Summary of the issue:
There is a fragile workaround introduced in
#18402 (comment).
This is also a soft API breakage for add-ons, forcing teleNVDA to use
the workaround

This is likely a bug fixed in truststore
sethmlarson/truststore#149

We are using pip_system_certs to patch in an old version of truststore.
this [is no longer being actively
maintained](https://gitlab.com/alelec/pip-system-certs/-/issues/27#note_2372267321)
and apparently truststore has the same patching feature now.

Description of user facing changes:
None

Description of developer facing changes:
Fix workaround which was required for add-ons such as tele NVDA

Testing strategy:
Ensure #18460 is not
reintroduced.
Make sure NVDA can still use update check and the add-on store in
corporate network environments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants