You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by xy...@apache.org on 2023/11/24 04:35:06 UTC
(pulsar-client-cpp) branch main updated: Fix HTTP lookup segfault when the redirect host cannot be resolved (#356)
This is an automated email from the ASF dual-hosted git repository.
xyz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pulsar-client-cpp.git
The following commit(s) were added to refs/heads/main by this push:
new d209482 Fix HTTP lookup segfault when the redirect host cannot be resolved (#356)
d209482 is described below
commit d2094828f9fe756b315ba194e1f8a69ca24ac6b4
Author: Yunze Xu <xy...@163.com>
AuthorDate: Fri Nov 24 12:35:01 2023 +0800
Fix HTTP lookup segfault when the redirect host cannot be resolved (#356)
### Motivation
When the host of the redirect URL cannot be resolved, segmentation fault
will happen:
https://github.com/apache/pulsar-client-cpp/blob/0bbc15502905d19c630d237b5e102bfb996bb098/lib/CurlWrapper.h#L173-L175
In this case, `curl` will be `nullptr`. Assigning a nullptr to a
`std::string` is an undefined behavior that might cause segfault.
### Modifications
Check if `url` is nullptr in `CurlWrapper::get` and before assigning it
to the `redirectUrl` field. Improve the
`HTTPLookupService::sendHTTPRequest` method by configuring the
`maxLookupRedirects` instead of a loop and print more detailed error
messages.
---
lib/CurlWrapper.h | 5 +-
lib/HTTPLookupService.cc | 149 ++++++++++++++++++++---------------------------
2 files changed, 68 insertions(+), 86 deletions(-)
diff --git a/lib/CurlWrapper.h b/lib/CurlWrapper.h
index 89b7919..749a79c 100644
--- a/lib/CurlWrapper.h
+++ b/lib/CurlWrapper.h
@@ -172,7 +172,10 @@ inline CurlWrapper::Result CurlWrapper::get(const std::string& url, const std::s
if (responseCode == 307 || responseCode == 302 || responseCode == 301) {
char* url;
curl_easy_getinfo(handle_, CURLINFO_REDIRECT_URL, &url);
- result.redirectUrl = url;
+ // `url` is null when the host of the redirect URL cannot be resolved
+ if (url) {
+ result.redirectUrl = url;
+ }
}
return result;
}
diff --git a/lib/HTTPLookupService.cc b/lib/HTTPLookupService.cc
index 4ec72c1..0959af2 100644
--- a/lib/HTTPLookupService.cc
+++ b/lib/HTTPLookupService.cc
@@ -191,98 +191,77 @@ Result HTTPLookupService::sendHTTPRequest(std::string completeUrl, std::string &
Result HTTPLookupService::sendHTTPRequest(std::string completeUrl, std::string &responseData,
long &responseCode) {
- uint16_t reqCount = 0;
- Result retResult = ResultOk;
- while (++reqCount <= maxLookupRedirects_) {
- // Authorization data
- AuthenticationDataPtr authDataContent;
- Result authResult = authenticationPtr_->getAuthData(authDataContent);
- if (authResult != ResultOk) {
- LOG_ERROR("Failed to getAuthData: " << authResult);
- return authResult;
- }
+ // Authorization data
+ AuthenticationDataPtr authDataContent;
+ Result authResult = authenticationPtr_->getAuthData(authDataContent);
+ if (authResult != ResultOk) {
+ LOG_ERROR("Failed to getAuthData: " << authResult);
+ return authResult;
+ }
- CurlWrapper curl;
- if (!curl.init()) {
- LOG_ERROR("Unable to curl_easy_init for url " << completeUrl);
- return ResultLookupError;
- }
+ CurlWrapper curl;
+ if (!curl.init()) {
+ LOG_ERROR("Unable to curl_easy_init for url " << completeUrl);
+ return ResultLookupError;
+ }
- std::unique_ptr<CurlWrapper::TlsContext> tlsContext;
- if (isUseTls_) {
- tlsContext.reset(new CurlWrapper::TlsContext);
- tlsContext->trustCertsFilePath = tlsTrustCertsFilePath_;
- tlsContext->validateHostname = tlsValidateHostname_;
- tlsContext->allowInsecure = tlsAllowInsecure_;
- if (authDataContent->hasDataForTls()) {
- tlsContext->certPath = authDataContent->getTlsCertificates();
- tlsContext->keyPath = authDataContent->getTlsPrivateKey();
- } else {
- tlsContext->certPath = tlsCertificateFilePath_;
- tlsContext->keyPath = tlsPrivateFilePath_;
- }
+ std::unique_ptr<CurlWrapper::TlsContext> tlsContext;
+ if (isUseTls_) {
+ tlsContext.reset(new CurlWrapper::TlsContext);
+ tlsContext->trustCertsFilePath = tlsTrustCertsFilePath_;
+ tlsContext->validateHostname = tlsValidateHostname_;
+ tlsContext->allowInsecure = tlsAllowInsecure_;
+ if (authDataContent->hasDataForTls()) {
+ tlsContext->certPath = authDataContent->getTlsCertificates();
+ tlsContext->keyPath = authDataContent->getTlsPrivateKey();
+ } else {
+ tlsContext->certPath = tlsCertificateFilePath_;
+ tlsContext->keyPath = tlsPrivateFilePath_;
}
+ }
- LOG_INFO("Curl [" << reqCount << "] Lookup Request sent for " << completeUrl);
- CurlWrapper::Options options;
- options.timeoutInSeconds = lookupTimeoutInSeconds_;
- options.userAgent = std::string("Pulsar-CPP-v") + PULSAR_VERSION_STR;
- options.maxLookupRedirects = 1; // redirection is implemented by the outer loop
- auto result = curl.get(completeUrl, authDataContent->getHttpHeaders(), options, tlsContext.get());
- const auto &error = result.error;
- if (!error.empty()) {
- LOG_ERROR(completeUrl << " failed: " << error);
- return ResultConnectError;
- }
+ LOG_INFO("Curl Lookup Request sent for " << completeUrl);
+ CurlWrapper::Options options;
+ options.timeoutInSeconds = lookupTimeoutInSeconds_;
+ options.userAgent = std::string("Pulsar-CPP-v") + PULSAR_VERSION_STR;
+ options.maxLookupRedirects = maxLookupRedirects_;
+ auto result = curl.get(completeUrl, authDataContent->getHttpHeaders(), options, tlsContext.get());
+ const auto &error = result.error;
+ if (!error.empty()) {
+ LOG_ERROR(completeUrl << " failed: " << error);
+ return ResultConnectError;
+ }
- responseData = result.responseData;
- responseCode = result.responseCode;
- auto res = result.code;
- LOG_INFO("Response received for url " << completeUrl << " responseCode " << responseCode
- << " curl res " << res);
-
- const auto &redirectUrl = result.redirectUrl;
- switch (res) {
- case CURLE_OK:
- if (responseCode == 200) {
- retResult = ResultOk;
- } else if (!redirectUrl.empty()) {
- LOG_INFO("Response from url " << completeUrl << " to new url " << redirectUrl);
- completeUrl = redirectUrl;
- retResult = ResultLookupError;
- } else {
- retResult = ResultLookupError;
- }
- break;
- case CURLE_COULDNT_CONNECT:
- LOG_ERROR("Response failed for url " << completeUrl << ". Error Code " << res);
- retResult = ResultRetryable;
- break;
- case CURLE_COULDNT_RESOLVE_PROXY:
- case CURLE_COULDNT_RESOLVE_HOST:
- case CURLE_HTTP_RETURNED_ERROR:
- LOG_ERROR("Response failed for url " << completeUrl << ". Error Code " << res);
- retResult = ResultConnectError;
- break;
- case CURLE_READ_ERROR:
- LOG_ERROR("Response failed for url " << completeUrl << ". Error Code " << res);
- retResult = ResultReadError;
- break;
- case CURLE_OPERATION_TIMEDOUT:
- LOG_ERROR("Response failed for url " << completeUrl << ". Error Code " << res);
- retResult = ResultTimeout;
- break;
- default:
- LOG_ERROR("Response failed for url " << completeUrl << ". Error Code " << res);
- retResult = ResultLookupError;
- break;
- }
- if (redirectUrl.empty()) {
- break;
- }
+ responseData = result.responseData;
+ responseCode = result.responseCode;
+ auto res = result.code;
+ if (res == CURLE_OK) {
+ LOG_INFO("Response received for url " << completeUrl << " responseCode " << responseCode);
+ } else if (res == CURLE_TOO_MANY_REDIRECTS) {
+ LOG_ERROR("Response received for url " << completeUrl << ": " << curl_easy_strerror(res)
+ << ", curl error: " << result.serverError
+ << ", redirect URL: " << result.redirectUrl);
+ } else {
+ LOG_ERROR("Response failed for url " << completeUrl << ": " << curl_easy_strerror(res)
+ << ", curl error: " << result.serverError);
}
- return retResult;
+ switch (res) {
+ case CURLE_OK:
+ return ResultOk;
+ case CURLE_COULDNT_CONNECT:
+ return ResultRetryable;
+ case CURLE_COULDNT_RESOLVE_PROXY:
+ case CURLE_COULDNT_RESOLVE_HOST:
+ case CURLE_HTTP_RETURNED_ERROR:
+ return ResultConnectError;
+ case CURLE_READ_ERROR:
+ return ResultReadError;
+ case CURLE_OPERATION_TIMEDOUT:
+ return ResultTimeout;
+ default:
+ return ResultLookupError;
+ }
}
LookupDataResultPtr HTTPLookupService::parsePartitionData(const std::string &json) {