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

Re-using http connection with custom ssl callback / custom certificate validation #384

Open
plougue opened this issue Feb 16, 2021 · 5 comments

Comments

@plougue
Copy link

plougue commented Feb 16, 2021

Hi !

I'm trying to work within a context in which I need to customize certificate validation. For this I use operation_context::set_ssl_context_callback to work directly with the boost::asio::ssl::context object and it works.

However it seems like it's impacting performances by preventing existing connections from being reused. Specifically, it looks like it's because the key used to retrieve a connection is using an std::function object's address :


This std::function has been copied from my context into the http::config object right here
config.set_ssl_context_callback(instance->m_context._get_impl()->get_ssl_context_callback());
so the address shouldn't be the same over 2 calls.
I've also noticed that in some cases the std::function object inside the http::config object is allocated at the same address over and over again so it is actually the same but there is no real guarantee for it.

Would it be possible to allow consistently reusing the same connection when setting an ssl_context ?

Thanks !

@Jinming-Hu
Copy link
Member

Hi @plougue , how did you copy the operation_context among multiple API calls? If you use the same operation_context, the address of the callback object should be the same.

@plougue
Copy link
Author

plougue commented Feb 22, 2021

Hello ! My operation_context is created once and held by the object in which the various API calls are made. It is not copied.

However on API calls, the callback object seems to be copied from the operation_context object to a new cpprest http::config object in the sdk's execute_async() function here :
https://github.com/Azure/azure-storage-cpp/blob/master/Microsoft.WindowsAzure.Storage/src/executor.cpp#L149

Because cpprest's set_ssl_context_callback actually copies the std::function object into its m_ssl_context_callback member :
https://github.com/microsoft/cpprestsdk/blob/master/Release/include/cpprest/http_client.h#L371

The key for retrieving the existing connection is made using the address of the std::function inside the http::config object but from what I understand it has been internally copied at this point

@Jinming-Hu
Copy link
Member

Hello ! My operation_context is created once and held by the object in which the various API calls are made. It is not copied.

However on API calls, the callback object seems to be copied from the operation_context object to a new cpprest http::config object in the sdk's execute_async() function here :
https://github.com/Azure/azure-storage-cpp/blob/master/Microsoft.WindowsAzure.Storage/src/executor.cpp#L149

Because cpprest's set_ssl_context_callback actually copies the std::function object into its m_ssl_context_callback member :
https://github.com/microsoft/cpprestsdk/blob/master/Release/include/cpprest/http_client.h#L371

The key for retrieving the existing connection is made using the address of the std::function inside the http::config object but from what I understand it has been internally copied at this point

Yeah, I got it.

@Jinming-Hu
Copy link
Member

But how can we fix this issue? I'm thinking maybe adding another parameter callback_address to get_http_client() function, and passing the address of the function object in operation_context instead of the copied one.

@plougue
Copy link
Author

plougue commented Mar 1, 2021

Hello again.
I think the solution you suggests works. An alternative could also be to reuse the same connection without testing that the ssl_context_callback is the same since I believe there are other callbacks that are not compared anyway (ssl_native_session_handle_options_callback and such). But if you want to keep the test I'd say what you suggest is the way to go :)

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

No branches or pull requests

2 participants