Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: check for libcurl version and feature AsynchDNS #813

Merged
merged 6 commits into from
Mar 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
**Fixes**:

- Remove OpenSSL as direct dependency for the crashpad backend on Linux. ([#812](https://github.com/getsentry/sentry-native/pull/812), [crashpad#81](https://github.com/getsentry/crashpad/pull/81))
- Check `libcurl` for feature `AsynchDNS` at compile- and runtime. ([#813](https://github.com/getsentry/sentry-native/pull/813))

## 0.6.0

Expand Down
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ endif()

if(SENTRY_TRANSPORT_CURL)
if(NOT CURL_FOUND) # Some other lib might bring libcurl already
find_package(CURL REQUIRED)
find_package(CURL REQUIRED COMPONENTS AsynchDNS)
endif()

if(TARGET CURL::libcurl) # Only available in cmake 3.12+
Expand Down
17 changes: 17 additions & 0 deletions src/sentry_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -529,3 +529,20 @@ sentry__snprintf_c(char *buf, size_t buf_size, const char *fmt, ...)
va_end(args);
return rv;
}

bool
sentry__check_min_version(sentry_version_t actual, sentry_version_t expected)
{
if (actual.major < expected.major) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we are not considering semver breaks here? As we are allowing the major version to be higher than expected.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right; I wanted to encode only the minimum version bound in this function. But checking for an upper bound (in another function) would also make sense when checking versions at runtime (also because symmetry is awesome).

But I wasn't in a hurry to add such a function because we are currently only using it for the libcurl check, and it seems that libcurl has more of a Linux major-bump idea for 8: https://daniel.haxx.se/blog/2021/09/13/heading-towards-curl-eight/

I am open to adding an (open interval) upper-bound check to utils, but I am unsure if we'll use it against libcurl under that premise. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they won’t break anything with a new major, then there is no reason to check for that either :-)

return false;
}
if (actual.major == expected.major && actual.minor < expected.minor) {
return false;
}
if (actual.major == expected.major && actual.minor == expected.minor
&& actual.patch < expected.patch) {
return false;
}

return true;
}
16 changes: 16 additions & 0 deletions src/sentry_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -198,4 +198,20 @@ double sentry__strtod_c(const char *ptr, char **endptr);
*/
int sentry__snprintf_c(char *buf, size_t buf_size, const char *fmt, ...);

/**
* Represents a version of a software artifact.
*/
typedef struct {
unsigned int major;
unsigned int minor;
unsigned int patch;
} sentry_version_t;

/**
* Checks whether `actual` is the same or a later version than `expected`.
* Returns `true` if that is the case.
*/
bool sentry__check_min_version(
sentry_version_t actual, sentry_version_t expected);

#endif
27 changes: 26 additions & 1 deletion src/transports/sentry_transport_curl.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

#include <curl/curl.h>
#include <curl/easy.h>
#include <stdlib.h>
#include <string.h>

typedef struct curl_transport_state_s {
Expand Down Expand Up @@ -67,6 +66,32 @@ sentry__curl_transport_start(
SENTRY_WARNF("`curl_global_init` failed with code `%d`", (int)rv);
return 1;
}

curl_version_info_data *version_data
= curl_version_info(CURLVERSION_NOW);

if (!version_data) {
SENTRY_WARN("Failed to retrieve `curl_version_info`");
return 1;
}

sentry_version_t curl_version = {
.major = (version_data->version_num >> 16) & 0xff,
.minor = (version_data->version_num >> 8) & 0xff,
.patch = version_data->version_num & 0xff,
};

if (!sentry__check_min_version(
curl_version, (sentry_version_t) { 7, 10, 7 })) {
SENTRY_WARNF("`libcurl` is at unsupported version `%u.%u.%u`",
curl_version.major, curl_version.minor, curl_version.patch);
return 1;
}

if ((version_data->features & CURL_VERSION_ASYNCHDNS) == 0) {
SENTRY_WARN("`libcurl` was not compiled with feature `AsynchDNS`");
return 1;
}
}

sentry_bgworker_t *bgworker = (sentry_bgworker_t *)transport_state;
Expand Down
29 changes: 29 additions & 0 deletions tests/unit/test_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -221,3 +221,32 @@ SENTRY_TEST(os)

sentry_value_decref(os);
}

SENTRY_TEST(check_version)
{
TEST_CHECK(sentry__check_min_version(
(sentry_version_t) { .major = 7, .minor = 10, .patch = 7 },
(sentry_version_t) { .major = 7, .minor = 10, .patch = 7 }));
TEST_CHECK(sentry__check_min_version(
(sentry_version_t) { .major = 7, .minor = 11, .patch = 7 },
(sentry_version_t) { .major = 7, .minor = 10, .patch = 7 }));
TEST_CHECK(sentry__check_min_version(
(sentry_version_t) { .major = 7, .minor = 10, .patch = 8 },
(sentry_version_t) { .major = 7, .minor = 10, .patch = 7 }));
TEST_CHECK(sentry__check_min_version(
(sentry_version_t) { .major = 8, .minor = 9, .patch = 7 },
(sentry_version_t) { .major = 7, .minor = 10, .patch = 7 }));
TEST_CHECK(sentry__check_min_version(
(sentry_version_t) { .major = 7, .minor = 11, .patch = 6 },
(sentry_version_t) { .major = 7, .minor = 10, .patch = 7 }));

TEST_CHECK(!sentry__check_min_version(
(sentry_version_t) { .major = 6, .minor = 10, .patch = 7 },
(sentry_version_t) { .major = 7, .minor = 10, .patch = 7 }));
TEST_CHECK(!sentry__check_min_version(
(sentry_version_t) { .major = 7, .minor = 9, .patch = 7 },
(sentry_version_t) { .major = 7, .minor = 10, .patch = 7 }));
TEST_CHECK(!sentry__check_min_version(
(sentry_version_t) { .major = 7, .minor = 10, .patch = 6 },
(sentry_version_t) { .major = 7, .minor = 10, .patch = 7 }));
}
5 changes: 3 additions & 2 deletions tests/unit/tests.inc
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,21 @@ XX(basic_tracing_context)
XX(basic_transaction)
XX(bgworker_flush)
XX(build_id_parser)
XX(check_version)
XX(child_spans)
XX(concurrent_init)
XX(concurrent_uninit)
XX(count_sampled_events)
XX(crashed_last_run)
XX(crash_marker)
XX(crashed_last_run)
XX(custom_logger)
XX(discarding_before_send)
XX(distributed_headers)
XX(drop_unfinished_spans)
XX(dsn_parsing_complete)
XX(dsn_parsing_invalid)
XX(dsn_store_url_without_path)
XX(dsn_store_url_with_path)
XX(dsn_store_url_without_path)
XX(empty_transport)
XX(fuzz_json)
XX(init_failure)
Expand Down