Skip to content

Commit 5a1b25d

Browse files
vicromsvicroms
committed
PR comments WIP 2
1 parent b4d82ea commit 5a1b25d

File tree

4 files changed

+96
-310
lines changed

4 files changed

+96
-310
lines changed

‎include/vcpkg/metrics.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,5 +198,5 @@ namespace vcpkg
198198
extern std::atomic<bool> g_should_send_metrics;
199199

200200
void flush_global_metrics(const Filesystem&);
201-
bool curl_upload_metrics(StringView payload);
201+
bool curl_upload_metrics(const std::string& payload);
202202
}

‎src/vcpkg/base/downloads.cpp‎

Lines changed: 81 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,77 @@ using namespace vcpkg;
2424

2525
namespace
2626
{
27+
struct CurlEasyHandle
28+
{
29+
CurlEasyHandle() : m_ptr(curl_easy_init()) { }
30+
CurlEasyHandle(CurlEasyHandle&& other) noexcept : m_ptr(std::exchange(other.m_ptr, nullptr)) { }
31+
CurlEasyHandle& operator=(CurlEasyHandle&& other) noexcept
32+
{
33+
m_ptr = std::exchange(other.m_ptr, nullptr);
34+
return *this;
35+
}
36+
~CurlEasyHandle()
37+
{
38+
if (m_ptr)
39+
{
40+
curl_easy_cleanup(m_ptr);
41+
}
42+
}
43+
44+
CURL* get()
45+
{
46+
if (!m_ptr)
47+
{
48+
Checks::unreachable(VCPKG_LINE_INFO);
49+
}
50+
return m_ptr;
51+
}
52+
53+
private:
54+
CURL* m_ptr = nullptr;
55+
};
56+
57+
struct CurlHeaders
58+
{
59+
CurlHeaders() : m_headers(nullptr) { }
60+
CurlHeaders(View<std::string> headers) : m_headers(nullptr)
61+
{
62+
for (const auto& header : headers)
63+
{
64+
m_headers = curl_slist_append(m_headers, header.c_str());
65+
}
66+
}
67+
CurlHeaders(CurlHeaders&& other) noexcept : m_headers(std::exchange(other.m_headers, nullptr)) { }
68+
CurlHeaders& operator=(CurlHeaders&& other) noexcept
69+
{
70+
m_headers = std::exchange(other.m_headers, nullptr);
71+
return *this;
72+
}
73+
~CurlHeaders()
74+
{
75+
if (m_headers)
76+
{
77+
curl_slist_free_all(m_headers);
78+
}
79+
}
80+
curl_slist* get() const { return m_headers; }
81+
82+
private:
83+
curl_slist* m_headers = nullptr;
84+
};
85+
2786
constexpr char vcpkg_curl_user_agent[] =
2887
"vcpkg/" VCPKG_BASE_VERSION_AS_STRING "-" VCPKG_VERSION_AS_STRING " (curl)";
2988

30-
void set_common_curl_easy_options(CURL* handle, const char* url, curl_slist* request_headers)
89+
void set_common_curl_easy_options(CURL* handle, const char* url, const CurlHeaders& request_headers)
3190
{
3291
curl_easy_setopt(handle, CURLOPT_USERAGENT, vcpkg_curl_user_agent);
3392
curl_easy_setopt(handle, CURLOPT_URL, url);
3493
curl_easy_setopt(handle,
3594
CURLOPT_FOLLOWLOCATION,
3695
2L); // Follow redirects, change request method based on HTTP response code.
3796
// https://curl.se/libcurl/c/CURLOPT_FOLLOWLOCATION.html#CURLFOLLOWOBEYCODE
38-
curl_easy_setopt(handle, CURLOPT_HTTPHEADER, request_headers);
97+
curl_easy_setopt(handle, CURLOPT_HTTPHEADER, request_headers.get());
3998
curl_easy_setopt(handle, CURLOPT_HEADEROPT, CURLHEADER_SEPARATE); // don't send headers to proxy CONNECT
4099
}
41100
}
@@ -180,22 +239,13 @@ namespace vcpkg
180239
std::vector<WriteFilePointer> write_pointers;
181240
write_pointers.reserve(urls.size());
182241

183-
curl_slist* request_headers = nullptr;
184-
for (auto&& header : headers)
185-
{
186-
request_headers = curl_slist_append(request_headers, header.c_str());
187-
}
188-
242+
CurlHeaders request_headers(headers);
189243
for (size_t request_index = 0; request_index < urls.size(); ++request_index)
190244
{
191245
const auto& url = urls[request_index];
192246

193-
CURL* curl = curl_easy_init();
194-
if (!curl)
195-
{
196-
ret[request_index] = CURLE_FAILED_INIT;
197-
continue;
198-
}
247+
CurlEasyHandle handle;
248+
CURL* curl = handle.get();
199249

200250
set_common_curl_easy_options(curl, url.c_str(), request_headers);
201251
if (outputs.empty())
@@ -242,7 +292,7 @@ namespace vcpkg
242292
}
243293
} while (still_running);
244294

245-
// just drain any messages left in the queue from the previous loop
295+
// drain any messages left in the queue from the previous loop
246296
int messages_in_queue = 0;
247297
while (auto* msg = curl_multi_info_read(multi_handle, &messages_in_queue))
248298
{
@@ -281,13 +331,9 @@ namespace vcpkg
281331
}
282332

283333
curl_multi_remove_handle(multi_handle, handle);
284-
curl_easy_cleanup(handle);
285334
}
286335
}
287-
288-
curl_slist_free_all(request_headers);
289336
curl_multi_cleanup(multi_handle);
290-
291337
return ret;
292338
}
293339

@@ -336,20 +382,19 @@ namespace vcpkg
336382
fmt::format_to(
337383
std::back_inserter(uri), "/repos/{}/dependency-graph/snapshots", url_encode_spaces(github_repository));
338384

339-
CURL* curl = curl_easy_init();
340-
if (!curl)
341-
{
342-
Checks::unreachable(VCPKG_LINE_INFO);
343-
}
385+
CurlEasyHandle handle;
386+
CURL* curl = handle.get();
344387

345388
std::string post_data = Json::stringify(snapshot);
346389

347-
curl_slist* request_headers = nullptr;
348-
request_headers = curl_slist_append(request_headers, "Accept: application/vnd.github+json");
349-
request_headers = curl_slist_append(request_headers, ("Authorization: Bearer " + github_token).c_str());
350-
request_headers = curl_slist_append(request_headers, "X-GitHub-Api-Version: 2022-11-28");
351-
request_headers = curl_slist_append(request_headers, "Content-Type: application/json");
390+
std::string headers[]{
391+
"Accept: application/vnd.github+json",
392+
("Authorization: Bearer " + github_token),
393+
"X-GitHub-Api-Version: 2022-11-28",
394+
"Content-Type: application/json",
395+
};
352396

397+
CurlHeaders request_headers(headers);
353398
set_common_curl_easy_options(curl, uri.c_str(), request_headers);
354399
curl_easy_setopt(curl, CURLOPT_USERAGENT, vcpkg_curl_user_agent);
355400
curl_easy_setopt(curl, CURLOPT_POST, 1L);
@@ -360,9 +405,6 @@ namespace vcpkg
360405
long response_code = 0;
361406
curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &response_code);
362407

363-
curl_slist_free_all(request_headers);
364-
curl_easy_cleanup(curl);
365-
366408
if (result != CURLE_OK)
367409
{
368410
context.report_error(msg::format(msgCurlFailedGeneric, msg::exit_code = static_cast<int>(result))
@@ -394,24 +436,13 @@ namespace vcpkg
394436
}
395437
auto file_size = fileptr.size(VCPKG_LINE_INFO);
396438

397-
CURL* curl = curl_easy_init();
398-
if (!curl)
399-
{
400-
Checks::unreachable(VCPKG_LINE_INFO);
401-
}
402-
403-
curl_slist* request_headers = nullptr;
404-
if (!raw_url.starts_with("ftp://"))
405-
{
406-
for (auto&& header : headers)
407-
{
408-
request_headers = curl_slist_append(request_headers, header.c_str());
409-
}
410-
}
439+
CurlEasyHandle handle;
440+
CURL* curl = handle.get();
411441

442+
auto request_headers = raw_url.starts_with("ftp://") ? CurlHeaders() : CurlHeaders(headers);
412443
auto upload_url = url_encode_spaces(raw_url);
413444
curl_easy_setopt(curl, CURLOPT_USERAGENT, vcpkg_curl_user_agent);
414-
curl_easy_setopt(curl, CURLOPT_HTTPHEADER, request_headers);
445+
curl_easy_setopt(curl, CURLOPT_HTTPHEADER, request_headers.get());
415446
curl_easy_setopt(curl, CURLOPT_URL, upload_url.c_str());
416447
curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
417448
curl_easy_setopt(curl, CURLOPT_READDATA, &fileptr);
@@ -423,8 +454,6 @@ namespace vcpkg
423454
{
424455
context.report_error(msg::format(msgCurlFailedGeneric, msg::exit_code = static_cast<int>(result))
425456
.append_raw(fmt::format(" ({}).", curl_easy_strerror(result))));
426-
curl_easy_cleanup(curl);
427-
curl_slist_free_all(request_headers);
428457
return false;
429458
}
430459

@@ -434,13 +463,9 @@ namespace vcpkg
434463
if ((response_code >= 100 && response_code < 200) || response_code >= 300)
435464
{
436465
context.report_error(msg::format(msgCurlFailedToPut, msg::url = sanitized_url, msg::value = response_code));
437-
curl_easy_cleanup(curl);
438-
curl_slist_free_all(request_headers);
439466
return false;
440467
}
441468

442-
curl_easy_cleanup(curl);
443-
curl_slist_free_all(request_headers);
444469
return true;
445470
}
446471

@@ -563,15 +588,9 @@ namespace vcpkg
563588
return DownloadPrognosis::OtherError;
564589
}
565590

566-
curl_slist* request_headers = nullptr;
567-
for (auto&& header : headers)
568-
request_headers = curl_slist_append(request_headers, header.c_str());
569-
570-
auto curl = curl_easy_init();
571-
if (!curl)
572-
{
573-
Checks::unreachable(VCPKG_LINE_INFO);
574-
}
591+
CurlEasyHandle handle;
592+
CURL* curl = handle.get();
593+
CurlHeaders request_headers(headers);
575594

576595
set_common_curl_easy_options(curl, url_encode_spaces(raw_url).c_str(), request_headers);
577596
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, &write_file_callback);
@@ -639,9 +658,6 @@ namespace vcpkg
639658
}
640659
} while (should_retry && retries_count < retry_delay.size());
641660

642-
curl_easy_cleanup(curl);
643-
curl_slist_free_all(request_headers);
644-
645661
if (!curl_success)
646662
{
647663
return DownloadPrognosis::NetworkErrorProxyMightHelp;

‎src/vcpkg/metrics.cpp‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <math.h>
1919

2020
#include <iterator>
21+
#include <limits>
2122
#include <mutex>
2223
#include <utility>
2324

@@ -568,8 +569,14 @@ namespace vcpkg
568569
return false;
569570
}
570571

571-
bool curl_upload_metrics(StringView payload)
572+
bool curl_upload_metrics(const std::string& payload)
572573
{
574+
if (payload.length() > static_cast<size_t>(std::numeric_limits<long>::max()))
575+
{
576+
Debug::println("Metrics payload too large to upload");
577+
return false;
578+
}
579+
573580
CURL* curl = curl_easy_init();
574581
if (!curl)
575582
{
@@ -579,20 +586,18 @@ namespace vcpkg
579586
curl_slist* headers = nullptr;
580587
headers = curl_slist_append(headers, "Content-Type: application/json");
581588

582-
auto mutable_payload = payload.to_string();
583-
584589
curl_easy_setopt(curl, CURLOPT_URL, "https://dc.services.visualstudio.com/v2/track");
585-
curl_easy_setopt(curl, CURLOPT_POSTFIELDS, mutable_payload.c_str());
586-
curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE, static_cast<long>(mutable_payload.length()));
590+
curl_easy_setopt(curl, CURLOPT_POSTFIELDS, payload.c_str());
591+
curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE, static_cast<long>(payload.length()));
587592
curl_easy_setopt(curl, CURLOPT_HTTPHEADER, headers);
588593
curl_easy_setopt(curl, CURLOPT_TIMEOUT, 60L);
589594
curl_easy_setopt(curl, CURLOPT_SSLVERSION, CURL_SSLVERSION_TLSv1_2);
590595
curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L); // CURLFOLLOW_ALL
591596
curl_easy_setopt(curl, CURLOPT_USERAGENT, "vcpkg/1.0");
592597

593598
curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 1L);
594-
auto buff = std::make_unique<std::string>();
595-
curl_easy_setopt(curl, CURLOPT_WRITEDATA, buff.get());
599+
std::string buff;
600+
curl_easy_setopt(curl, CURLOPT_WRITEDATA, static_cast<void*>(&buff));
596601
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, &string_append_cb);
597602

598603
long response_code = 0;
@@ -602,10 +607,10 @@ namespace vcpkg
602607
{
603608
curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &response_code);
604609
Debug::println(fmt::format("Metrics upload response code: {}", response_code));
605-
Debug::println("Metrics upload response body: ", *buff);
610+
Debug::println("Metrics upload response body: ", buff);
606611
if (response_code == 200)
607612
{
608-
is_success = parse_metrics_response(*buff);
613+
is_success = parse_metrics_response(buff);
609614
}
610615
}
611616
else

0 commit comments

Comments
 (0)