BewareMyPower commented on a change in pull request #13112:
URL: https://github.com/apache/pulsar/pull/13112#discussion_r761706789
##########
File path: pulsar-client-cpp/lib/HTTPLookupService.cc
##########
@@ -148,132 +148,145 @@ void
HTTPLookupService::handleNamespaceTopicsHTTPRequest(NamespaceTopicsPromise
}
}
-Result HTTPLookupService::sendHTTPRequest(const std::string completeUrl,
std::string &responseData) {
- CURL *handle;
- CURLcode res;
- std::string version = std::string("Pulsar-CPP-v") +
_PULSAR_VERSION_INTERNAL_;
- handle = curl_easy_init();
-
- if (!handle) {
- LOG_ERROR("Unable to curl_easy_init for url " << completeUrl);
- // No curl_easy_cleanup required since handle not initialized
- return ResultLookupError;
- }
- // set URL
- curl_easy_setopt(handle, CURLOPT_URL, completeUrl.c_str());
-
- // Write callback
- curl_easy_setopt(handle, CURLOPT_WRITEFUNCTION, curlWriteCallback);
- curl_easy_setopt(handle, CURLOPT_WRITEDATA, &responseData);
+Result HTTPLookupService::sendHTTPRequest(std::string completeUrl, std::string
&responseData) {
+ uint16_t reqCount = 0;
+ Result retResult = ResultOk;
+ while (++reqCount <= MAX_HTTP_REDIRECTS) {
+ CURL *handle;
+ CURLcode res;
+ std::string version = std::string("Pulsar-CPP-v") +
_PULSAR_VERSION_INTERNAL_;
+ handle = curl_easy_init();
+
+ if (!handle) {
+ LOG_ERROR("Unable to curl_easy_init for url " << completeUrl);
+ // No curl_easy_cleanup required since handle not initialized
+ return ResultLookupError;
+ }
+ // set URL
+ curl_easy_setopt(handle, CURLOPT_URL, completeUrl.c_str());
- // New connection is made for each call
- curl_easy_setopt(handle, CURLOPT_FRESH_CONNECT, 1L);
- curl_easy_setopt(handle, CURLOPT_FORBID_REUSE, 1L);
+ // Write callback
+ curl_easy_setopt(handle, CURLOPT_WRITEFUNCTION, curlWriteCallback);
+ curl_easy_setopt(handle, CURLOPT_WRITEDATA, &responseData);
- // Skipping signal handling - results in timeouts not honored during the
DNS lookup
- curl_easy_setopt(handle, CURLOPT_NOSIGNAL, 1L);
+ // New connection is made for each call
+ curl_easy_setopt(handle, CURLOPT_FRESH_CONNECT, 1L);
+ curl_easy_setopt(handle, CURLOPT_FORBID_REUSE, 1L);
- // Timer
- curl_easy_setopt(handle, CURLOPT_TIMEOUT, lookupTimeoutInSeconds_);
+ // Skipping signal handling - results in timeouts not honored during
the DNS lookup
+ curl_easy_setopt(handle, CURLOPT_NOSIGNAL, 1L);
- // Set User Agent
- curl_easy_setopt(handle, CURLOPT_USERAGENT, version.c_str());
+ // Timer
+ curl_easy_setopt(handle, CURLOPT_TIMEOUT, lookupTimeoutInSeconds_);
- // Redirects
- curl_easy_setopt(handle, CURLOPT_FOLLOWLOCATION, 1L);
- curl_easy_setopt(handle, CURLOPT_MAXREDIRS, MAX_HTTP_REDIRECTS);
+ // Set User Agent
+ curl_easy_setopt(handle, CURLOPT_USERAGENT, version.c_str());
- // Fail if HTTP return code >=400
- curl_easy_setopt(handle, CURLOPT_FAILONERROR, 1L);
+ // Fail if HTTP return code >=400
+ curl_easy_setopt(handle, CURLOPT_FAILONERROR, 1L);
- // Authorization data
- AuthenticationDataPtr authDataContent;
- Result authResult = authenticationPtr_->getAuthData(authDataContent);
- if (authResult != ResultOk) {
- LOG_ERROR("Failed to getAuthData: " << authResult);
- curl_easy_cleanup(handle);
- return authResult;
- }
- struct curl_slist *list = NULL;
- if (authDataContent->hasDataForHttp()) {
- list = curl_slist_append(list,
authDataContent->getHttpHeaders().c_str());
- }
- curl_easy_setopt(handle, CURLOPT_HTTPHEADER, list);
-
- // TLS
- if (isUseTls_) {
- if (curl_easy_setopt(handle, CURLOPT_SSLENGINE, NULL) != CURLE_OK) {
- LOG_ERROR("Unable to load SSL engine for url " << completeUrl);
+ // Authorization data
+ AuthenticationDataPtr authDataContent;
+ Result authResult = authenticationPtr_->getAuthData(authDataContent);
+ if (authResult != ResultOk) {
+ LOG_ERROR("Failed to getAuthData: " << authResult);
curl_easy_cleanup(handle);
- return ResultConnectError;
+ return authResult;
}
- if (curl_easy_setopt(handle, CURLOPT_SSLENGINE_DEFAULT, 1L) !=
CURLE_OK) {
- LOG_ERROR("Unable to load SSL engine as default, for url " <<
completeUrl);
- curl_easy_cleanup(handle);
- return ResultConnectError;
+ struct curl_slist *list = NULL;
+ if (authDataContent->hasDataForHttp()) {
+ list = curl_slist_append(list,
authDataContent->getHttpHeaders().c_str());
}
- curl_easy_setopt(handle, CURLOPT_SSLCERTTYPE, "PEM");
+ curl_easy_setopt(handle, CURLOPT_HTTPHEADER, list);
+
+ // TLS
+ if (isUseTls_) {
+ if (curl_easy_setopt(handle, CURLOPT_SSLENGINE, NULL) != CURLE_OK)
{
+ LOG_ERROR("Unable to load SSL engine for url " << completeUrl);
+ curl_easy_cleanup(handle);
+ return ResultConnectError;
+ }
+ if (curl_easy_setopt(handle, CURLOPT_SSLENGINE_DEFAULT, 1L) !=
CURLE_OK) {
+ LOG_ERROR("Unable to load SSL engine as default, for url " <<
completeUrl);
+ curl_easy_cleanup(handle);
+ return ResultConnectError;
+ }
+ curl_easy_setopt(handle, CURLOPT_SSLCERTTYPE, "PEM");
- if (tlsAllowInsecure_) {
- curl_easy_setopt(handle, CURLOPT_SSL_VERIFYPEER, 0L);
- } else {
- curl_easy_setopt(handle, CURLOPT_SSL_VERIFYPEER, 1L);
- }
+ if (tlsAllowInsecure_) {
+ curl_easy_setopt(handle, CURLOPT_SSL_VERIFYPEER, 0L);
+ } else {
+ curl_easy_setopt(handle, CURLOPT_SSL_VERIFYPEER, 1L);
+ }
- if (!tlsTrustCertsFilePath_.empty()) {
- curl_easy_setopt(handle, CURLOPT_CAINFO,
tlsTrustCertsFilePath_.c_str());
- }
+ if (!tlsTrustCertsFilePath_.empty()) {
+ curl_easy_setopt(handle, CURLOPT_CAINFO,
tlsTrustCertsFilePath_.c_str());
+ }
- curl_easy_setopt(handle, CURLOPT_SSL_VERIFYHOST, tlsValidateHostname_
? 1L : 0L);
+ curl_easy_setopt(handle, CURLOPT_SSL_VERIFYHOST,
tlsValidateHostname_ ? 1L : 0L);
- if (authDataContent->hasDataForTls()) {
- curl_easy_setopt(handle, CURLOPT_SSLCERT,
authDataContent->getTlsCertificates().c_str());
- curl_easy_setopt(handle, CURLOPT_SSLKEY,
authDataContent->getTlsPrivateKey().c_str());
+ if (authDataContent->hasDataForTls()) {
+ curl_easy_setopt(handle, CURLOPT_SSLCERT,
authDataContent->getTlsCertificates().c_str());
+ curl_easy_setopt(handle, CURLOPT_SSLKEY,
authDataContent->getTlsPrivateKey().c_str());
+ }
}
- }
-
- LOG_INFO("Curl Lookup Request sent for " << completeUrl);
-
- // Make get call to server
- res = curl_easy_perform(handle);
-
- // Free header list
- curl_slist_free_all(list);
- Result retResult = ResultOk;
-
- switch (res) {
- case CURLE_OK:
- long response_code;
- curl_easy_getinfo(handle, CURLINFO_RESPONSE_CODE, &response_code);
- LOG_INFO("Response received for url " << completeUrl << " code "
<< response_code);
- if (response_code == 200) {
- retResult = ResultOk;
- } else {
+ LOG_INFO("Curl [" << reqCount << "] Lookup Request sent for " <<
completeUrl);
+
+ // Make get call to server
+ res = curl_easy_perform(handle);
+
+ long response_code;
+ curl_easy_getinfo(handle, CURLINFO_RESPONSE_CODE, &response_code);
+ LOG_INFO("Response received for url " << completeUrl << "
response_code " << response_code
+ << " curl res " << res);
+
+ // Free header list
+ curl_slist_free_all(list);
+
+ switch (res) {
+ case CURLE_OK:
+ long response_code;
+ curl_easy_getinfo(handle, CURLINFO_RESPONSE_CODE,
&response_code);
+ LOG_INFO("Response received for url " << completeUrl << " code
" << response_code);
+ if (response_code == 200) {
+ retResult = ResultOk;
+ } else if (response_code == 307 || response_code == 302 ||
response_code == 301) {
Review comment:
It's better to add a common function to check if the response code needs
redirection because the check is used in two places.
```c++
inline bool needRedirection(long code) {
return code % 300 < 10; // Here I just assume other 30x code also needs
redirection
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
