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

Revert "urldata: move async resolver state from easy handle to connectdata" #12524

Closed
wants to merge 3 commits into from

Conversation

bagder
Copy link
Member

@bagder bagder commented Dec 14, 2023

This reverts commit 56a4db2 (#12198)

We want the c-ares channel to be held in the easy handle, not per connection - for performance.

@bagder bagder added the name lookup DNS and related tech label Dec 14, 2023
@bagder bagder requested a review from icing December 14, 2023 22:32
…tdata"

This reverts commit 56a4db2 (#12198)

We want the c-ares channel to be held in the easy handle, not per
connection - for performance.
@bagder
Copy link
Member Author

bagder commented Dec 14, 2023

If I had just used my brains a little more earlier, I would've thought about/mentioned this before and probably not merged #12198

@bagder
Copy link
Member Author

bagder commented Dec 15, 2023

@icing I could use your keen eyes on this. populate_x509_store() in openssl.c puzzles me. With this revert, the ssl_capath becomes NULL (in test 305) and I can't quite figure out why. What exactly is conn_config vs ssl_config in this context?

@icing
Copy link
Contributor

icing commented Dec 15, 2023

You threw away the call to Curl_ssl_conn_config_init(). The patch below makes it work for me again.

diff --git a/lib/url.c b/lib/url.c
index 94999d20b..7764668dc 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -3744,6 +3744,11 @@ static CURLcode create_conn(struct Curl_easy *data,
        * This is a brand new connection, so let's store it in the connection
        * cache of ours!
        */
+      result = Curl_ssl_conn_config_init(data, conn);
+      if(result) {
+        DEBUGF(fprintf(stderr, "Error: init connection ssl config\n"));
+        goto out;
+      }

       Curl_attach_connection(data, conn);
       result = Curl_conncache_add_conn(data);
diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c
index 1f735e001..0f2dc82ad 100644
--- a/lib/vtls/openssl.c
+++ b/lib/vtls/openssl.c
@@ -3175,6 +3175,8 @@ static CURLcode populate_x509_store(struct Curl_cfilter *cf,
   bool imported_native_ca = false;
   bool imported_ca_info_blob = false;

+  CURL_TRC_CF(data, cf, "populate_x509_store, path=%s, blob=%d",
+              ssl_cafile? ssl_cafile : "none", !!ca_info_blob);
   if(!store)
     return CURLE_OUT_OF_MEMORY;

@bagder
Copy link
Member Author

bagder commented Dec 15, 2023

oops, thank you!

@bagder bagder closed this in 907eea0 Dec 15, 2023
@bagder bagder deleted the bagder/revert-12521 branch December 15, 2023 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
name lookup DNS and related tech
Development

Successfully merging this pull request may close these issues.

None yet

2 participants