Jens-G opened a new pull request, #3851: URL: https://github.com/apache/thrift/pull/3851
`TSSLSocket.cpp` has not compiled against OpenSSL 1.1.1 since THRIFT-6174, which has left master's AppVeyor build red. THRIFT-6174 took const-qualified X509 types in `authorize()` from OpenSSL 1.1.0 onwards, on the grounds that every consumer of those values had taken const since 1.1.0. `X509_NAME_get_index_by_NID()` is the exception — it was constified in **3.0**. At the exact release tags: | tag | declaration | |---|---| | `OpenSSL_1_1_1w` | `int X509_NAME_get_index_by_NID(X509_NAME *name, int nid, int lastpos);` | | `openssl-3.0.0` | `int X509_NAME_get_index_by_NID(const X509_NAME *name, int nid, int lastpos);` | AppVeyor is the only CI configuration that builds against a pre-3.0 library — it reports `Found OpenSSL ... (found version "1.1.1w")` — and all five of its MSVC jobs failed: ``` TSSLSocket.cpp(885,14): error C2664: 'int X509_NAME_get_index_by_NID(X509_NAME *,int,int)': cannot convert argument 1 from 'const X509_NAME *' to 'X509_NAME *' ``` Every GitHub Actions job builds against 3.x and stayed green, which is how this got through. ### The change The three guards in that block move from `0x10100000L` to `0x30000000L`. At 3.0 the const path is still compiled by every 3.x CI configuration, which was THRIFT-6174's reason for choosing 1.1.0 over 4.0 rather than shipping an arm that nothing ever compiles. Below 3.0 all three getters return non-const pointers and every consumer takes them either way — that arm is the code that shipped before THRIFT-6174. c_glib needs no matching change: `thrift_ssl_socket.c` does no `X509_NAME` lookup at all, it matches the host through `SSL_set1_host()`. ### Tests The regression test is the existing AppVeyor matrix, which already builds against OpenSSL 1.1.1w — no CI configuration had to be added for this. Verified locally by building `lib/cpp` and its tests against a from-source **1.1.1w** and against the distribution **3.0.2**: - before the change, 1.1.1w fails with exactly one error, the same one MSVC reports - after it, both libraries build clean and `ctest` is 57/57 on each - a preprocessor probe confirms 1.1.1w takes the non-const arm and 3.0.2 the const one, so both arms are genuinely exercised - `SecurityTest` runs its full matrix on both — 29 cells on the 1.1.1w build, which has SSLv3 enabled, and 20 on 3.0.2, which defines `OPENSSL_NO_SSL3` ### One note for reviewers LibreSSL's current `x509.h` does const-qualify this getter, so "keeps the pre-1.1.0 signatures" does not hold for it as a blanket statement. It makes no difference here: the explicit `!defined(LIBRESSL_VERSION_NUMBER)` keeps LibreSSL on the non-const arm, which compiles against it either way. The comment is worded accordingly rather than repeating the broader claim. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
