BoringSSL compatibility fixes for cpp-httplib [crashpad/crashpad : main]
102 views
Skip to first unread message
Mark Mentovai (Gerrit)
Aug 7, 2024, 1:33:12 PM8/7/24
to Joshua Peraza, crashp...@chromium.org
Attention needed from Joshua Peraza
Mark Mentovai voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Joshua Peraza
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joshua Peraza (Gerrit)
Aug 7, 2024, 1:43:19 PM8/7/24
to Mark Mentovai, Crashpad LUCI CQ, crashp...@chromium.org
Attention needed from Mark Mentovai
Joshua Peraza voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Mark Mentovai
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mark Mentovai (Gerrit)
Aug 7, 2024, 1:57:26 PM8/7/24
to Joshua Peraza, Crashpad LUCI CQ, crashp...@chromium.org
Mark Mentovai submitted the change![]( "Open in Gerrit")
Change information
Commit message:
BoringSSL compatibility fixes for cpp-httplib
This fixes errors observed while building
util/http_transport_test_server/http_transport_test_server.cc, shown
below.
The fixes include:
- Library version check: tolerate BoringSSL as an alternative to
OpenSSL 3.
- Don’t call `OPENSSL_thread_stop`, which is not in BoringSSL.
- Use `SSL_get_peer_certificate` (deprecated in OpenSSL 3), the old
name for `SSL_get1_peer_certificate`, because the new name is not in
BoringSSL.
- Call `SSL_set_tlsext_host_name` directly instead of making a quirky
`SSL_ctrl` call that BoringSSL does not support. The feared
-Wold-style-cast warning that occurs when buidling with OpenSSL is
not triggered in BoringSSL.
Compilation errors from
https://chromium-review.googlesource.com/c/5766975?checksPatchset=1&tab=checks
→ https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/1909715/
→ “10. compilator steps (with patch)” → “31. compile (with patch)” →
stdout
(https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8740323272553670737/+/u/compile__with_patch_/stdout):
```
In file included from util/net/http_transport_test_server.cc:42:
third_party/cpp-httplib/cpp-httplib/httplib.h:275:2: error: Sorry, OpenSSL versions prior to 3.0.0 are not supported
275 | #error Sorry, OpenSSL versions prior to 3.0.0 are not supported
| ^
In file included from util/net/http_transport_test_server.cc:42:
third_party/cpp-httplib/cpp-httplib/httplib.h:733:7: error: use of undeclared identifier 'OPENSSL_thread_stop'
733 | OPENSSL_thread_stop ();
| ^
third_party/cpp-httplib/cpp-httplib/httplib.h:9062:30: error: use of undeclared identifier 'SSL_get1_peer_certificate'; did you mean 'SSL_get_peer_certificate'?
9062 | auto server_cert = SSL_get1_peer_certificate(ssl2);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
| SSL_get_peer_certificate
…/boringssl/src/include/openssl/ssl.h:1784:22: note: 'SSL_get_peer_certificate' declared here
1784 | OPENSSL_EXPORT X509 *SSL_get_peer_certificate(const SSL *ssl);
| ^
In file included from util/net/http_transport_test_server.cc:42:
third_party/cpp-httplib/cpp-httplib/httplib.h:9083:24: error: use of undeclared identifier 'doesnt_exist'
9083 | SSL_ctrl(ssl2, SSL_CTRL_SET_TLSEXT_HOSTNAME, TLSEXT_NAMETYPE_host_name,
| ^
…/boringssl/src/include/openssl/ssl.h:5699:38: note: expanded from macro 'SSL_CTRL_SET_TLSEXT_HOSTNAME'
5699 | #define SSL_CTRL_SET_TLSEXT_HOSTNAME doesnt_exist
| ^
4 errors generated.
```
Change-Id: I5798f17323672d70f75335cea61094457b54466e
Reviewed-by: Joshua Peraza <jpe...@chromium.org>
Files:
- M third_party/cpp-httplib/cpp-httplib/httplib.h
Change size: S
Delta: 1 file changed, 8 insertions(+), 3 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Joshua Peraza
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mark Mentovai (Gerrit)
Aug 7, 2024, 2:16:55 PM8/7/24
to Joshua Peraza, Crashpad LUCI CQ, crashp...@chromium.org
Mark Mentovai added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 2 (Latest):
Mark Mentovai . resolved
I’m attempting to upstream these fixes: https://github.com/yhirose/cpp-httplib/pull/1892.
Related details
Attention set is empty
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Benjamin (Gerrit)
Sep 8, 2024, 6:12:40 PM9/8/24
to Mark Mentovai, Joshua Peraza, Crashpad LUCI CQ, crashp...@chromium.org
David Benjamin added 1 comment![]( "Open in Gerrit")
Patchset-level comments
David Benjamin . resolved
Ah fun. This is the first project I've seen that's dropped support for OpenSSL 1.1.1.
It'll probably be a bit before we can start claiming to be OpenSSL 3.x and adding all their APIs[*], but I'll file this away as a reason to start shifting things. (We don't like to pollute external projects with BoringSSL ifdefs and would rather add APIs as needed to reduce burden on them. It's just that that dance gets a little tricky around version bumps.)
Do let us know if you run into more of these.
[*] At least in the 1.0.x -> 1.1.x transition, one thing we learned is that you kinda need to add most of their APIs and bump the version all at once. If we add the APIs but don't bump the version, we break code that looks like this:
```
#if OPENSSL_VERSION_NUMBER < ...
// Now this doesn't compile because BoringSSL already defines some_new_api
static int some_new_api(...) { ... }
#endif
```
But if we bump the version without adding the APIs then we break code that expects the new APIs.
Related details
Attention set is empty
Submit Requirements:
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages