Skip to content
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

Undefine OpenSSL::SSL for no socket platforms #558

Merged
merged 3 commits into from Dec 22, 2022

Conversation

kateinoigakukun
Copy link
Member

This fixes a linkage error about ossl_ssl_type on platforms that do not have socket, like WASI.

Even before this patch, OpenSSL::SSL::Socket.new throws NotImplementedError under OPENSSL_NO_SOCK since ruby/ruby@ee22fad However, due to some new use of OpenSSL::SSL::Socket over the past few years, the build under OPENSSL_NO_SOCK had been broken.

This patch guards those new use of OpenSSL::SSL::Socket by OPENSSL_NO_SOCK.

  • SSLContext#servername_cb=, which sets a callback taking a socket.
  • OpenSSL::Session, which can accept a socket to create a new session.

@rhenium
Copy link
Member

rhenium commented Nov 23, 2022

Should we disable OpenSSL::SSL entirely on defined(OPENSSL_NO_SOCK)? I don't think OpenSSL::SSL::SSLContext or Session without SSLSocket would be useful at all.

@kateinoigakukun kateinoigakukun force-pushed the katei/fix-no-sock-support branch 3 times, most recently from 76d7d1e to 033fac7 Compare December 1, 2022 02:51
@kateinoigakukun kateinoigakukun changed the title Undefine SSLContext#servername_cb= and part of OpenSSL::Session under OPENSSL_NO_SOCK Undefine OpenSSL::SSL for no socket platforms Dec 1, 2022
@kateinoigakukun
Copy link
Member Author

@rhenium That makes sense to me 👍

@rhenium
Copy link
Member

rhenium commented Dec 22, 2022

Let me push some additional commits.

@rhenium
Copy link
Member

rhenium commented Dec 22, 2022

Oops. We crossed each other.

@kateinoigakukun
Copy link
Member Author

Okay, no worries 🙏 now CI is green

rhenium and others added 3 commits December 23, 2022 05:08
This module was introduced in 2015 for internal use within this library.
Neither of the two constants in it is used anymore. I don't think we
will be adding a new constant in the foreseeable future, either.

OPENSSL_NO_SOCK is unused since commit 998d667 (r55191).
HAVE_TLSEXT_HOST_NAME is unused since commit 4eb4b32.
This fixes a linkage error about `ossl_ssl_type` on platforms which do
not have socket, like WASI.

Even before this patch, some items are disabled under `OPENSSL_NO_SOCK` since
ruby/ruby@ee22fad
However, due to some new use of OpenSSL::SSL::Socket over the past few years,
the build under `OPENSSL_NO_SOCK` had been broken.

This patch guards whole `OpenSSL::SSL` items by `OPENSSL_NO_SOCK`.

[ky: adjusted to apply on top of my previous commit that removed the
OpenSSL::ExtConfig, and added a guard to lib/openssl/ssl.rb.]
@rhenium rhenium merged commit 75bbc8a into ruby:master Dec 22, 2022
@rhenium
Copy link
Member

rhenium commented Dec 22, 2022

Merged, thank you!

@kateinoigakukun kateinoigakukun deleted the katei/fix-no-sock-support branch December 23, 2022 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants