Azure / azure-storage-cpplite

Lite version of C++ Client Library for Microsoft Azure Storage
MIT License
25 stars 43 forks source link

Cannot access Request Headers #51

Closed raghujayan closed 4 years ago

raghujayan commented 4 years ago

I cannot access the request headers. When I use CurlEasyRequest I can get the response headers after performing CurlEasyRequest::perform() Would it be possible to store the request headers as part of the add_header (in a private member m_request_headers)method and let users of the sdk access them via get_request_headers? I understand this might not be an intended feature of the SDK but would help us tremendously in our use case

Our application performs its own curl requests. We are looking to use this sdk to get the curl request headers.

Would this be possible?

Jinming-Hu commented 4 years ago

Hi @raghujayan, we can add a public interface to access request headers, which is not difficult. The difficult part is, how can you use it to get request headers without really firing curl requests from our sdk.

Because most headers are filled the moment before curl_easy_perform gets called, and currently, as far as we know, there's no way you can stop it after headers are filled and before curl_easy_perform is called.

raghujayan commented 4 years ago

Hi @JinmingHu-MSFT ,I may be getting this wrong, but would it not be possible to capture the request headers when they get added to m_slist in the add_headers (libcurl_http_client.h) method and store it as a key value pair?

Jinming-Hu commented 4 years ago

@raghujayan Yes, it's possible and easy to implement.

What i'm worried about is, how do you plan to use it, can you stop the libcurl client underlying our storage sdk from sending http requests? If not, even if you can get those http headers, what do you want to do with them after the http requests are already sent out?

raghujayan commented 4 years ago
std::shared_ptr<azure::storage_lite::storage_credential> _pcCredentials;
_pcCredentials = std::make_shared<azure::storage_lite::token_credential>(_accessToken);
std::shared_ptr<azure::storage_lite::storage_account> account = 
std::make_shared<azure::storage_lite::storage_account>(accountName, _pcCredentials, /* use_https */ true);
  std::shared_ptr<azure::storage_lite::blob_client> blobClient = std::make_shared<azure::storage_lite::blob_client>(account, 1);  

  std::shared_ptr<azure::storage_lite::http_base> http_request = blobClient->client()->get_handle();

  std::unique_ptr<azure::storage_lite::storage_request_base> _pcGetrequestsigner =  
      std::unique_ptr<vdsremote_blob_request>(new vdsremote_blob_request(container, blob, startByte, endByte));

  _pcGetrequestsigner->build_request(*account, *(http_request));

// At this point shouldn't all the request headers have been populated?

for (auto h:http_request->get_request_headers())
  {
    authHeaders.Set(h.first.c_str(), h.second.c_str());
  }
.
.
.

/// Use the authHeaders in our internal code for curl requests

The above code snippet will use the SDK without actually performing the request via the sdk. Before I make the get_request_headers shouldn't all the request headers have been populated?

Note: The get_request_headers does not exist currently. The rest of the code compiles and runs. Do you see a problem with this approach?

raghujayan commented 4 years ago

Any suggestions on the above @JinmingHu-MSFT ?

Jinming-Hu commented 4 years ago

@raghujayan I'm going to look into it tomorrow.

raghujayan commented 4 years ago

Thank you very much for taking the time @JinmingHu-MSFT

Jinming-Hu commented 4 years ago

Hi @raghujayan , I tried your method, it actually works, although very tricky.

I added a get_request_headers() function in this branch, please give it a try. If everything is ok, we're going to merge it into master.

Plus this is how I tried this function, just for your reference.

auto cred = std::make_shared<azure::storage_lite::shared_key_credential>(account_name, account_key);
auto account = std::make_shared<azure::storage_lite::storage_account>(account_name, cred);
auto blob_client = std::make_shared<azure::storage_lite::blob_client>(account, 1);

auto http_request = blob_client->client()->get_handle();

auto request_signer = std::make_shared<azure::storage_lite::create_container_request>("container1");

request_signer->build_request(*account, *http_request);

for (auto header : http_request->get_request_headers())
{
    std::cout << header.first << ": " << header.second << std::endl;
}

In addition, please keep in mind that, several functions in your code snippet aren't supposed to be called directly, they are only for internal use. It's okay if you want to use them. But they may change in the future, although unlikely. We do not guarantee.

raghujayan commented 4 years ago

Thank you very much @JinmingHu-MSFT! Your updates worked and I am now able to use your SDK for our use case.

Also making a distinction between request and response headers makes the code much cleaner.

I understand that these APIs are internal and therefore can break in future. We are planning on integrating this repo into our CI/CD an should be able to catch such breaking changes.

Appreciate you taking the time to incorporate this feature request.

Cheers