Skip to content

Fix updateCheck._updateWindowsRootCertificates() for Python 3. #11253

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 12, 2020

Conversation

jcsteh
Copy link
Contributor

@jcsteh jcsteh commented Jun 12, 2020

Link to issue number:

Related to #4803.

Summary of the issue:

updateCheck._updateWindowsRootCertificates is broken in Python 3. This will mean that a system which doesn't have the root SSL certificate used by nvaccess.org (e.g. a clean Windows install) won't be able to check for updates.

Description of how this pull request fixes the issue:

This was missed in the migration to Python 3.
There are two problems that needed to be addressed here:

  1. We use https://www.nvaccess.org/nvdaUpdateCheck as the URL to get the certificate. However, that returns a 404. In Python 2, urllib didn't raise an exception for errors. In Python 3, it does. So, this was raising an exception and preventing us from getting any further.
    To fix this, pass versionType=stable as the query string, which will stop the server from throwing 404.
  2. When getting the peer certificate, we need the raw SSL socket. In Python 3, the way to get that raw socket has changed slightly, so this code had to be adjusted accordingly.

Testing performed:

I don't have a clean system to test this on, but I confirmed that I don't get any exceptions when I do this in the NVDA Python console:

import updateCheck
updateCheck._updateWindowsRootCertificates()

Known issues with pull request:

None.

Change log entry:

Bug fixes:
- It is once again possible to check for NVDA updates on certain systems; e.g. clean Windows installs.

This was missed in the migration to Python 3.
There are two problems that needed to be addressed here:

1. We use https://www.nvaccess.org/nvdaUpdateCheck as the URL to get the certificate. However, that returns a 404. In Python 2, urllib didn't raise an exception for errors. In Python 3, it does. So, this was raising an exception and preventing us from getting any further.
    To fix this, pass versionType=stable as the query string, which will stop the server from throwing 404.
2. When getting the peer certificate, we need the raw SSL socket. In Python 3, the way to get that raw socket has changed slightly, so this code had to be adjusted accordingly.
@josephsl
Copy link
Collaborator

josephsl commented Jun 12, 2020 via email

Copy link
Collaborator

@josephsl josephsl left a comment

Choose a reason for hiding this comment

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

Thanks for catching this.

@LeonarddeR
Copy link
Collaborator

I wonder whether it could be helpful to hardcode the url in a function kwarg. This way the behavior doesn't differ when calling the function, but third party can reuse it for other urls.

@jcsteh
Copy link
Contributor Author

jcsteh commented Jun 12, 2020 via email

@josephsl
Copy link
Collaborator

josephsl commented Jun 12, 2020 via email

@feerrenrut feerrenrut added this to the 2020.2 milestone Jun 12, 2020
Copy link
Contributor

@feerrenrut feerrenrut left a comment

Choose a reason for hiding this comment

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

Thanks @jcsteh

Note: While #5871 is similar, it is likely to remain an issue after this PR is merged.

@jcsteh
Copy link
Contributor Author

jcsteh commented Jun 12, 2020 via email

@Brian1Gaff
Copy link

Brian1Gaff commented Jun 12, 2020 via email

@feerrenrut feerrenrut merged commit 4fc9cfe into nvaccess:master Jun 12, 2020
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.

5 participants