Skip to content

Commit 6192987

Browse files
committed
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. (cherry picked from commit d209482)
1 parent 478d3c3 commit 6192987

File tree

2 files changed

+68
-86
lines changed

2 files changed

+68
-86
lines changed

lib/CurlWrapper.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,10 @@ inline CurlWrapper::Result CurlWrapper::get(const std::string& url, const std::s
172172
if (responseCode == 307 || responseCode == 302 || responseCode == 301) {
173173
char* url;
174174
curl_easy_getinfo(handle_, CURLINFO_REDIRECT_URL, &url);
175-
result.redirectUrl = url;
175+
// `url` is null when the host of the redirect URL cannot be resolved
176+
if (url) {
177+
result.redirectUrl = url;
178+
}
176179
}
177180
return result;
178181
}

lib/HTTPLookupService.cc

Lines changed: 64 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -191,98 +191,77 @@ Result HTTPLookupService::sendHTTPRequest(std::string completeUrl, std::string &
191191

192192
Result HTTPLookupService::sendHTTPRequest(std::string completeUrl, std::string &responseData,
193193
long &responseCode) {
194-
uint16_t reqCount = 0;
195-
Result retResult = ResultOk;
196-
while (++reqCount <= maxLookupRedirects_) {
197-
// Authorization data
198-
AuthenticationDataPtr authDataContent;
199-
Result authResult = authenticationPtr_->getAuthData(authDataContent);
200-
if (authResult != ResultOk) {
201-
LOG_ERROR("Failed to getAuthData: " << authResult);
202-
return authResult;
203-
}
194+
// Authorization data
195+
AuthenticationDataPtr authDataContent;
196+
Result authResult = authenticationPtr_->getAuthData(authDataContent);
197+
if (authResult != ResultOk) {
198+
LOG_ERROR("Failed to getAuthData: " << authResult);
199+
return authResult;
200+
}
204201

205-
CurlWrapper curl;
206-
if (!curl.init()) {
207-
LOG_ERROR("Unable to curl_easy_init for url " << completeUrl);
208-
return ResultLookupError;
209-
}
202+
CurlWrapper curl;
203+
if (!curl.init()) {
204+
LOG_ERROR("Unable to curl_easy_init for url " << completeUrl);
205+
return ResultLookupError;
206+
}
210207

211-
std::unique_ptr<CurlWrapper::TlsContext> tlsContext;
212-
if (isUseTls_) {
213-
tlsContext.reset(new CurlWrapper::TlsContext);
214-
tlsContext->trustCertsFilePath = tlsTrustCertsFilePath_;
215-
tlsContext->validateHostname = tlsValidateHostname_;
216-
tlsContext->allowInsecure = tlsAllowInsecure_;
217-
if (authDataContent->hasDataForTls()) {
218-
tlsContext->certPath = authDataContent->getTlsCertificates();
219-
tlsContext->keyPath = authDataContent->getTlsPrivateKey();
220-
} else {
221-
tlsContext->certPath = tlsCertificateFilePath_;
222-
tlsContext->keyPath = tlsPrivateFilePath_;
223-
}
208+
std::unique_ptr<CurlWrapper::TlsContext> tlsContext;
209+
if (isUseTls_) {
210+
tlsContext.reset(new CurlWrapper::TlsContext);
211+
tlsContext->trustCertsFilePath = tlsTrustCertsFilePath_;
212+
tlsContext->validateHostname = tlsValidateHostname_;
213+
tlsContext->allowInsecure = tlsAllowInsecure_;
214+
if (authDataContent->hasDataForTls()) {
215+
tlsContext->certPath = authDataContent->getTlsCertificates();
216+
tlsContext->keyPath = authDataContent->getTlsPrivateKey();
217+
} else {
218+
tlsContext->certPath = tlsCertificateFilePath_;
219+
tlsContext->keyPath = tlsPrivateFilePath_;
224220
}
221+
}
225222

226-
LOG_INFO("Curl [" << reqCount << "] Lookup Request sent for " << completeUrl);
227-
CurlWrapper::Options options;
228-
options.timeoutInSeconds = lookupTimeoutInSeconds_;
229-
options.userAgent = std::string("Pulsar-CPP-v") + PULSAR_VERSION_STR;
230-
options.maxLookupRedirects = 1; // redirection is implemented by the outer loop
231-
auto result = curl.get(completeUrl, authDataContent->getHttpHeaders(), options, tlsContext.get());
232-
const auto &error = result.error;
233-
if (!error.empty()) {
234-
LOG_ERROR(completeUrl << " failed: " << error);
235-
return ResultConnectError;
236-
}
223+
LOG_INFO("Curl Lookup Request sent for " << completeUrl);
224+
CurlWrapper::Options options;
225+
options.timeoutInSeconds = lookupTimeoutInSeconds_;
226+
options.userAgent = std::string("Pulsar-CPP-v") + PULSAR_VERSION_STR;
227+
options.maxLookupRedirects = maxLookupRedirects_;
228+
auto result = curl.get(completeUrl, authDataContent->getHttpHeaders(), options, tlsContext.get());
229+
const auto &error = result.error;
230+
if (!error.empty()) {
231+
LOG_ERROR(completeUrl << " failed: " << error);
232+
return ResultConnectError;
233+
}
237234

238-
responseData = result.responseData;
239-
responseCode = result.responseCode;
240-
auto res = result.code;
241-
LOG_INFO("Response received for url " << completeUrl << " responseCode " << responseCode
242-
<< " curl res " << res);
243-
244-
const auto &redirectUrl = result.redirectUrl;
245-
switch (res) {
246-
case CURLE_OK:
247-
if (responseCode == 200) {
248-
retResult = ResultOk;
249-
} else if (!redirectUrl.empty()) {
250-
LOG_INFO("Response from url " << completeUrl << " to new url " << redirectUrl);
251-
completeUrl = redirectUrl;
252-
retResult = ResultLookupError;
253-
} else {
254-
retResult = ResultLookupError;
255-
}
256-
break;
257-
case CURLE_COULDNT_CONNECT:
258-
LOG_ERROR("Response failed for url " << completeUrl << ". Error Code " << res);
259-
retResult = ResultRetryable;
260-
break;
261-
case CURLE_COULDNT_RESOLVE_PROXY:
262-
case CURLE_COULDNT_RESOLVE_HOST:
263-
case CURLE_HTTP_RETURNED_ERROR:
264-
LOG_ERROR("Response failed for url " << completeUrl << ". Error Code " << res);
265-
retResult = ResultConnectError;
266-
break;
267-
case CURLE_READ_ERROR:
268-
LOG_ERROR("Response failed for url " << completeUrl << ". Error Code " << res);
269-
retResult = ResultReadError;
270-
break;
271-
case CURLE_OPERATION_TIMEDOUT:
272-
LOG_ERROR("Response failed for url " << completeUrl << ". Error Code " << res);
273-
retResult = ResultTimeout;
274-
break;
275-
default:
276-
LOG_ERROR("Response failed for url " << completeUrl << ". Error Code " << res);
277-
retResult = ResultLookupError;
278-
break;
279-
}
280-
if (redirectUrl.empty()) {
281-
break;
282-
}
235+
responseData = result.responseData;
236+
responseCode = result.responseCode;
237+
auto res = result.code;
238+
if (res == CURLE_OK) {
239+
LOG_INFO("Response received for url " << completeUrl << " responseCode " << responseCode);
240+
} else if (res == CURLE_TOO_MANY_REDIRECTS) {
241+
LOG_ERROR("Response received for url " << completeUrl << ": " << curl_easy_strerror(res)
242+
<< ", curl error: " << result.serverError
243+
<< ", redirect URL: " << result.redirectUrl);
244+
} else {
245+
LOG_ERROR("Response failed for url " << completeUrl << ": " << curl_easy_strerror(res)
246+
<< ", curl error: " << result.serverError);
283247
}
284248

285-
return retResult;
249+
switch (res) {
250+
case CURLE_OK:
251+
return ResultOk;
252+
case CURLE_COULDNT_CONNECT:
253+
return ResultRetryable;
254+
case CURLE_COULDNT_RESOLVE_PROXY:
255+
case CURLE_COULDNT_RESOLVE_HOST:
256+
case CURLE_HTTP_RETURNED_ERROR:
257+
return ResultConnectError;
258+
case CURLE_READ_ERROR:
259+
return ResultReadError;
260+
case CURLE_OPERATION_TIMEDOUT:
261+
return ResultTimeout;
262+
default:
263+
return ResultLookupError;
264+
}
286265
}
287266

288267
LookupDataResultPtr HTTPLookupService::parsePartitionData(const std::string &json) {

0 commit comments

Comments
 (0)