Skip to content

Use http-client-openssl instead of http-client-tls#137

Merged
HugoPeters1024 merged 2 commits intochannable:masterfrom
jfroche:openssl
Dec 27, 2023
Merged

Use http-client-openssl instead of http-client-tls#137
HugoPeters1024 merged 2 commits intochannable:masterfrom
jfroche:openssl

Conversation

@jfroche
Copy link
Contributor

@jfroche jfroche commented Dec 20, 2023

Using http-client-openssl that also handle the SSL_CERT_FILE environment variable

refs #99

Using http-client-openssl that also handle the SSL_CERT_FILE environment variable

refs channable#99
{ osslSettingsVerifyMode =
if not $ getOptionsValue oValidateCerts opts
then VerifyNone
else VerifyPeer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you use record syntax here to make it more clear what these settings do?

package.yaml Outdated
- optparse-applicative

ghc-options: -Wall -Werror
ghc-options: -threaded -rtsopts -Wall -Werror
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain why these extra options are necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not an expert in this openssl library but without the -threaded option we face
ConnectionFailure user error (RTS doesn't support multiple OS threads (use ghc -threaded when linking)) at run time when trying to connect to vault. So it seems that openssl requires a threaded runtime.

-rtsopts enable all RTS options processing and can be configured at runtime through command line or environment variable. It enables users to configure the number of threads, the heap size, ... We could remove it until we know that we really need it as it might have security implications.

Copy link
Contributor

@HugoPeters1024 HugoPeters1024 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for taking the time to make a contribution :)

LGTM!

@HugoPeters1024 HugoPeters1024 merged commit 3e4cb41 into channable:master Dec 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants