Summary

Add and use a new HTTPS handler, error reporting, and urlopen wrapper.

Review Request #14943 — Created March 18, 2026 and updated June 25, 2026, 8:32 a.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-8.x

Bugs

Depends On

~~14942~~

Reviewers

Groups

reviewboard

People

Description

This introduces reviewboard.certs.http, which contains a URL opener,
CertificateVerificationHTTPSHandler. This takes care of generating a
suitable SSL context based on the certificate manager, and handles the
conversion of SSL/TLS errors to our own exceptions.

If an SSLError is raised, it will be converted into a
CertificateVerificationError, complete with a Certificate instance
fetched from the server (as SSLError doesn't contain this). This means
individual call sites don't have to convert SSL errors anymore.

A urlopen() method in the same module is available, which can be used
as a direct replacement for urllib.request.urlopen(). If communicating
over HTTPS, this will make use of our new handler. Otherwise, it's the
exact same as calling the original urlopen().

All urlopen() call sites have been updated to use this. They were
recently updated to use build_urlopen_kwargs(), but that's no longer
required. In fact, this method is now deprecated in favor of the new
handler.

The hosting service HTTP support makes use of the new handler directly,
allowing it to leverage the central context building and error handling.

Unless a legacy UnverifiedCertificateError is explicitly returned by a
hosting service or SCMTool, all call sites will now use the modern
CertificateVerificationError.

Testing Done

All unit tests pass.

Manually tried adding repositories using bare SCMTools and hosting
services, verifying I received the cert verification banner each time.
Tested this with a variety of https://badssl.com endpoints.

Verified that trusting the cert created a stored cert entry and that
further access worked.

Commits

Summary	ID
Add and use a new HTTPS handler, error reporting, and urlopen wrapper. This introduces `reviewboard.certs.http`, which contains a URL opener, `CertificateVerificationHTTPSHandler`. This takes care of generating a suitable SSL context based on the certificate manager, and handles the conversion of SSL/TLS errors to our own exceptions. If an `SSLError` is raised, it will be converted into a `CertificateVerificationError`, complete with a `Certificate` instance fetched from the server (as `SSLError` doesn't contain this). This means individual call sites don't have to convert SSL errors anymore. A `urlopen()` method in the same module is available, which can be used as a direct replacement for `urllib.request.urlopen()`. If communicating over HTTPS, this will make use of our new handler. Otherwise, it's the exact same as calling the original `urlopen()`. All `urlopen()` call sites have been updated to use this. They were recently updated to use `build_urlopen_kwargs()`, but that's no longer required. In fact, this method is now deprecated in favor of the new handler. The hosting service HTTP support makes use of the new handler directly, allowing it to leverage the central context building and error handling. Unless a legacy `UnverifiedCertificateError` is explicitly returned by a hosting service or SCMTool, all call sites will now use the modern `CertificateVerificationError`.	6648518ba46881bba3facf22632e436a527650d8

Summary

Add and use a new HTTPS handler, error reporting, and urlopen wrapper.

This introduces `reviewboard.certs.http`, which contains a URL opener, `CertificateVerificationHTTPSHandler`. This takes care of generating a suitable SSL context based on the certificate manager, and handles the conversion of SSL/TLS errors to our own exceptions. If an `SSLError` is raised, it will be converted into a `CertificateVerificationError`, complete with a `Certificate` instance fetched from the server (as `SSLError` doesn't contain this). This means individual call sites don't have to convert SSL errors anymore. A `urlopen()` method in the same module is available, which can be used as a direct replacement for `urllib.request.urlopen()`. If communicating over HTTPS, this will make use of our new handler. Otherwise, it's the exact same as calling the original `urlopen()`. All `urlopen()` call sites have been updated to use this. They were recently updated to use `build_urlopen_kwargs()`, but that's no longer required. In fact, this method is now deprecated in favor of the new handler. The hosting service HTTP support makes use of the new handler directly, allowing it to leverage the central context building and error handling. Unless a legacy `UnverifiedCertificateError` is explicitly returned by a hosting service or SCMTool, all call sites will now use the modern `CertificateVerificationError`.

6648518ba46881bba3facf22632e436a527650d8

Issues

Description	From	Last Updated
Can we add some tests for the new urlopen? Would be nice to at least check these: https URL uses …	david	June 24, 2026, 7:49 p.m.
line too long (101 > 79 characters) Column: 80 Error code: E501	reviewbot	June 24, 2026, 3:40 p.m.
Typo: Reqest -> Request	david	June 24, 2026, 3:40 p.m.
Should we be getting urlopen from reviewboard.certs.http here? This change is going back to just pure regular stdlib urlopen.	david	June 24, 2026, 3:41 p.m.
This used to be guarded with the if context is not None check. We probably should only add the handler …	david	June 24, 2026, 3:43 p.m.
multiple spaces before keyword Column: 28 Error code: E272	reviewbot	June 24, 2026, 3:40 p.m.
This can be if hostname	david	June 25, 2026, 8:32 a.m.
Typo: process_ssl_error -> _process_ssl_error Here and below.	david	June 25, 2026, 8:32 a.m.
Leftover debug output.	david	June 25, 2026, 8:28 a.m.
The deleted code in this file has left a bunch of unused imports: hashlib, ssl, urlparse, x509, NameOID, default_backend, force_str, …	david	June 25, 2026, 8:28 a.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/certs/http.py (Diff revision 1)
The issue has been resolved. Show all issues
```
line too long (101 > 79 characters)

Column: 80
Error code: E501
```
reviewboard/scmtools/tests/test_scmclient.py (Diff revision 1)
The issue has been resolved. Show all issues
```
multiple spaces before keyword

Column: 28
Error code: E272
```

The issue has been resolved. Show all issues

Can we add some tests for the new urlopen? Would be nice to at least check these:

https URL uses the custom handler
Non-https URL falls through to stdlib urlopen
Request objects are handled correctly
local_site and cert_manager kwargs are forwarded

reviewboard/certs/http.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Typo: Reqest -> Request
```

reviewboard/cmdline/rbsite.py (Diff revision 1)

The issue has been dropped. Show all issues

Should we be getting urlopen from reviewboard.certs.http here? This change is going back to just pure regular stdlib urlopen.

chipx86

June 24, 2026, 3:43 p.m.

Yeah, this happens during install before we have any of that, and we might need database access for cert info, so we're just going back to plain urlopen() and using system settings.

reviewboard/hostingsvcs/base/http.py (Diff revision 1)

The issue has been dropped. Show all issues

This used to be guarded with the if context is not None check. We probably should only add the handler when parsed_url.scheme == 'https'

chipx86

June 24, 2026, 3:43 p.m.

We want to unconditionally register the handler here. This ensures that it's in use even if it's an http:// URL, which might redirect to https. Since it's an HTTPSHandler, it won't be used unless it's involved in an HTTPS connection.

Change Summary:


The HTTPS handler no longer gets pre-filled with identifying host information. It's now looked up as needed, allowing for cert management during redirects.
Our opener is used unconditionally, allows for HTTP -> HTTPS redirects.
Fixed typos.
Added new unit tests for the HTTPS handler and urlopen.

Commits:

	Summary	ID
	Add and use a new HTTPS handler, error reporting, and urlopen wrapper. This introduces `reviewboard.certs.http`, which contains a URL opener, `CertificateVerificationHTTPSHandler`. This takes care of generating a suitable SSL context based on the certificate manager, and handles the conversion of SSL/TLS errors to our own exceptions. If an `SSLError` is raised, it will be converted into a `CertificateVerificationError`, complete with a `Certificate` instance fetched from the server (as `SSLError` doesn't contain this). This means individual call sites don't have to convert SSL errors anymore. A `urlopen()` method in the same module is available, which can be used as a direct replacement for `urllib.request.urlopen()`. If communicating over HTTPS, this will make use of our new handler. Otherwise, it's the exact same as calling the original `urlopen()`. All `urlopen()` call sites have been updated to use this. They were recently updated to use `build_urlopen_kwargs()`, but that's no longer required. In fact, this method is now deprecated in favor of the new handler. The hosting service HTTP support makes use of the new handler directly, allowing it to leverage the central context building and error handling. Unless a legacy `UnverifiedCertificateError` is explicitly returned by a hosting service or SCMTool, all call sites will now use the modern `CertificateVerificationError`.	be82b52f07607f360f9bb63fcfa26fbd479b320e
	Add and use a new HTTPS handler, error reporting, and urlopen wrapper. This introduces `reviewboard.certs.http`, which contains a URL opener, `CertificateVerificationHTTPSHandler`. This takes care of generating a suitable SSL context based on the certificate manager, and handles the conversion of SSL/TLS errors to our own exceptions. If an `SSLError` is raised, it will be converted into a `CertificateVerificationError`, complete with a `Certificate` instance fetched from the server (as `SSLError` doesn't contain this). This means individual call sites don't have to convert SSL errors anymore. A `urlopen()` method in the same module is available, which can be used as a direct replacement for `urllib.request.urlopen()`. If communicating over HTTPS, this will make use of our new handler. Otherwise, it's the exact same as calling the original `urlopen()`. All `urlopen()` call sites have been updated to use this. They were recently updated to use `build_urlopen_kwargs()`, but that's no longer required. In fact, this method is now deprecated in favor of the new handler. The hosting service HTTP support makes use of the new handler directly, allowing it to leverage the central context building and error handling. Unless a legacy `UnverifiedCertificateError` is explicitly returned by a hosting service or SCMTool, all call sites will now use the modern `CertificateVerificationError`.	6648518ba46881bba3facf22632e436a527650d8

Branch:

release-7.1.x

release-8.x

Diff:

Revision 2 (+2338 -178)

Show changes

	reviewboard/admin/security_checks.py
	reviewboard/certs/http.py
	reviewboard/certs/manager.py
	reviewboard/certs/tests/test_certificate_manager.py
	reviewboard/certs/tests/test_certificate_verification_https_handler.py
	reviewboard/certs/tests/test_urlopen.py
	reviewboard/certs/tests/testcases.py
	reviewboard/cmdline/rbsite.py
	5 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Fix it!

reviewboard/certs/tests/test_urlopen.py (Diff revision 2)
An issue was opened. Show all issues
```
Leftover debug output.
```

reviewboard/hostingsvcs/base/client.py (Diff revision 2)

An issue was opened. Show all issues

The deleted code in this file has left a bunch of unused imports: hashlib, ssl, urlparse, x509, NameOID, default_backend, force_str, Certificate, and UnverifiedCertificateError

Fix it!

reviewboard/certs/http.py (Diff revision 2)
An issue was opened. Show all issues
```
This can be if hostname
```
reviewboard/certs/tests/test_certificate_verification_https_handler.py (Diff revision 2)
An issue was opened. Show all issues
```
Typo: process_ssl_error -> _process_ssl_error
Here and below.
```