Summary

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

Review Request #14943 — Created March 18, 2026 and updated March 18, 2026, 9:54 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-7.1.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`.	be82b52f07607f360f9bb63fcfa26fbd479b320e

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`.

be82b52f07607f360f9bb63fcfa26fbd479b320e

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	March 18, 2026, 9:54 p.m.
line too long (101 > 79 characters) Column: 80 Error code: E501	reviewbot	March 18, 2026, 8:33 p.m.
Typo: Reqest -> Request	david	March 18, 2026, 9:54 p.m.
Should we be getting urlopen from reviewboard.certs.http here? This change is going back to just pure regular stdlib urlopen.	david	March 18, 2026, 9:54 p.m.
This used to be guarded with the if context is not None check. We probably should only add the handler …	david	March 18, 2026, 9:54 p.m.
multiple spaces before keyword Column: 28 Error code: E272	reviewbot	March 18, 2026, 8:33 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/certs/http.py (Diff revision 1)
An issue was opened. Show all issues
```
line too long (101 > 79 characters)

Column: 80
Error code: E501
```
reviewboard/scmtools/tests/test_scmclient.py (Diff revision 1)
An issue was opened. Show all issues
```
multiple spaces before keyword

Column: 28
Error code: E272
```

Fix it!

An issue was opened. 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)
An issue was opened. Show all issues
```
Typo: Reqest -> Request
```

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

An issue was opened. Show all issues

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

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

An issue was opened. 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'