From 83c055e5120179a719ed1861484135c5dc087a57 Mon Sep 17 00:00:00 2001 From: Gregor Jasny Date: Mon, 18 Dec 2023 23:34:56 +0100 Subject: [PATCH 1/2] fix(util): avoid sign extension in base64 encodesr --- util/include/prometheus/detail/base64.h | 14 +++++++------- util/tests/unit/base64_test.cc | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/util/include/prometheus/detail/base64.h b/util/include/prometheus/detail/base64.h index 4adf6100..21c3e31d 100644 --- a/util/include/prometheus/detail/base64.h +++ b/util/include/prometheus/detail/base64.h @@ -37,9 +37,9 @@ inline std::string base64_encode(const std::string& input) { auto it = input.begin(); for (std::size_t i = 0; i < input.size() / 3; ++i) { - temp = (*it++) << 16; - temp += (*it++) << 8; - temp += (*it++); + temp = static_cast(*it++) << 16; + temp += static_cast(*it++) << 8; + temp += static_cast(*it++); encoded.append(1, kEncodeLookup[(temp & 0x00FC0000) >> 18]); encoded.append(1, kEncodeLookup[(temp & 0x0003F000) >> 12]); encoded.append(1, kEncodeLookup[(temp & 0x00000FC0) >> 6]); @@ -48,14 +48,14 @@ inline std::string base64_encode(const std::string& input) { switch (input.size() % 3) { case 1: - temp = (*it++) << 16; + temp = static_cast(*it++) << 16; encoded.append(1, kEncodeLookup[(temp & 0x00FC0000) >> 18]); encoded.append(1, kEncodeLookup[(temp & 0x0003F000) >> 12]); encoded.append(2, kPadCharacter); break; case 2: - temp = (*it++) << 16; - temp += (*it++) << 8; + temp = static_cast(*it++) << 16; + temp += static_cast(*it++) << 8; encoded.append(1, kEncodeLookup[(temp & 0x00FC0000) >> 18]); encoded.append(1, kEncodeLookup[(temp & 0x0003F000) >> 12]); encoded.append(1, kEncodeLookup[(temp & 0x00000FC0) >> 6]); @@ -118,7 +118,7 @@ inline std::string base64_decode(const std::string& input) { decoded.push_back((temp >> 16) & 0x000000FF); decoded.push_back((temp >> 8) & 0x000000FF); - decoded.push_back((temp)&0x000000FF); + decoded.push_back((temp) & 0x000000FF); } return decoded; diff --git a/util/tests/unit/base64_test.cc b/util/tests/unit/base64_test.cc index 1f05410a..7efba320 100644 --- a/util/tests/unit/base64_test.cc +++ b/util/tests/unit/base64_test.cc @@ -13,6 +13,12 @@ struct TestVector { }; const TestVector testVector[] = { + // RFC 3548 examples + {"\x14\xfb\x9c\x03\xd9\x7e", "FPucA9l+"}, + {"\x14\xfb\x9c\x03\xd9", "FPucA9k="}, + {"\x14\xfb\x9c\x03", "FPucAw=="}, + + // RFC 4648 examples {"", ""}, {"f", "Zg=="}, {"fo", "Zm8="}, @@ -20,6 +26,16 @@ const TestVector testVector[] = { {"foob", "Zm9vYg=="}, {"fooba", "Zm9vYmE="}, {"foobar", "Zm9vYmFy"}, + + // Wikipedia examples + {"sure.", "c3VyZS4="}, + {"sure", "c3VyZQ=="}, + {"sur", "c3Vy"}, + {"su", "c3U="}, + {"leasure.", "bGVhc3VyZS4="}, + {"easure.", "ZWFzdXJlLg=="}, + {"asure.", "YXN1cmUu"}, + {"sure.", "c3VyZS4="}, }; using namespace testing; From 4fb9c232f60f28ef91302b42bdadb639090402ae Mon Sep 17 00:00:00 2001 From: Gregor Jasny Date: Mon, 18 Dec 2023 23:42:08 +0100 Subject: [PATCH 2/2] fix(push): encode push labels Fixes #652 --- core/include/prometheus/labels.h | 3 ++ push/BUILD.bazel | 14 ++++++++ push/CMakeLists.txt | 13 +++++-- push/src/{ => detail}/curl_wrapper.cc | 0 push/src/{ => detail}/curl_wrapper.h | 0 push/src/detail/label_encoder.cc | 42 ++++++++++++++++++++++ push/src/detail/label_encoder.h | 13 +++++++ push/src/gateway.cc | 8 +++-- push/tests/CMakeLists.txt | 1 + push/tests/internal/BUILD.bazel | 13 +++++++ push/tests/internal/CMakeLists.txt | 14 ++++++++ push/tests/internal/label_encoder_test.cc | 43 +++++++++++++++++++++++ util/include/prometheus/detail/base64.h | 12 +++++++ util/tests/unit/base64_test.cc | 8 +++++ 14 files changed, 179 insertions(+), 5 deletions(-) rename push/src/{ => detail}/curl_wrapper.cc (100%) rename push/src/{ => detail}/curl_wrapper.h (100%) create mode 100644 push/src/detail/label_encoder.cc create mode 100644 push/src/detail/label_encoder.h create mode 100644 push/tests/internal/BUILD.bazel create mode 100644 push/tests/internal/CMakeLists.txt create mode 100644 push/tests/internal/label_encoder_test.cc diff --git a/core/include/prometheus/labels.h b/core/include/prometheus/labels.h index 919625b7..d6fce0ff 100644 --- a/core/include/prometheus/labels.h +++ b/core/include/prometheus/labels.h @@ -8,4 +8,7 @@ namespace prometheus { /// \brief Multiple labels and their value. using Labels = std::map; +/// \brief Single label and its value. +using Label = Labels::value_type; + } // namespace prometheus diff --git a/push/BUILD.bazel b/push/BUILD.bazel index c7cb3f91..e71647eb 100644 --- a/push/BUILD.bazel +++ b/push/BUILD.bazel @@ -24,6 +24,20 @@ cc_library( visibility = ["//visibility:public"], deps = [ "//core", + "//util", "@com_github_curl//:curl", ], ) + +cc_library( + name = "push_internal_headers", + hdrs = glob( + ["src/detail/*.h"], + ), + strip_include_prefix = "src", + visibility = ["//push/tests:__subpackages__"], + deps = [ + "//core", + "//push", + ], +) diff --git a/push/CMakeLists.txt b/push/CMakeLists.txt index 6399ce44..bf21e818 100644 --- a/push/CMakeLists.txt +++ b/push/CMakeLists.txt @@ -2,9 +2,12 @@ find_package(CURL REQUIRED) add_library(push - src/curl_wrapper.cc - src/curl_wrapper.h src/gateway.cc + + src/detail/curl_wrapper.cc + src/detail/curl_wrapper.h + src/detail/label_encoder.cc + src/detail/label_encoder.h ) add_library(${PROJECT_NAME}::push ALIAS push) @@ -18,6 +21,7 @@ target_link_libraries(push PUBLIC ${PROJECT_NAME}::core PRIVATE + ${PROJECT_NAME}::util Threads::Threads CURL::libcurl $<$,$>>:rt> @@ -78,5 +82,10 @@ if(GENERATE_PKGCONFIG) endif() if(ENABLE_TESTING) + add_library(push_internal_headers INTERFACE) + add_library(${PROJECT_NAME}::push_internal_headers ALIAS push_internal_headers) + target_include_directories(push_internal_headers INTERFACE src) + target_link_libraries(push_internal_headers INTERFACE ${PROJECT_NAME}::push) + add_subdirectory(tests) endif() diff --git a/push/src/curl_wrapper.cc b/push/src/detail/curl_wrapper.cc similarity index 100% rename from push/src/curl_wrapper.cc rename to push/src/detail/curl_wrapper.cc diff --git a/push/src/curl_wrapper.h b/push/src/detail/curl_wrapper.h similarity index 100% rename from push/src/curl_wrapper.h rename to push/src/detail/curl_wrapper.h diff --git a/push/src/detail/label_encoder.cc b/push/src/detail/label_encoder.cc new file mode 100644 index 00000000..75a6fc11 --- /dev/null +++ b/push/src/detail/label_encoder.cc @@ -0,0 +1,42 @@ +#include "label_encoder.h" + +#include +#include + +#include "prometheus/detail/base64.h" + +namespace prometheus { +namespace detail { + +namespace { +// Does this character need encoding like in RFC 3986 section 2.3? +bool needsEncoding(char c) { + if (c >= 'a' && c <= 'z') { + return false; + } + if (c >= 'A' && c <= 'Z') { + return false; + } + if (c >= '0' && c <= '9') { + return false; + } + if (c == '-' || c == '.' || c == '_' || c == '~') { + return false; + } + return true; +} +} // namespace + +void encodeLabel(std::ostream& os, const Label& label) { + if (label.second.empty()) { + os << "/" << label.first << "@base64/="; + } else if (std::any_of(label.second.begin(), label.second.end(), + needsEncoding)) { + os << "/" << label.first << "@base64/" + << detail::base64url_encode(label.second); + } else { + os << "/" << label.first << "/" << label.second; + } +} +} // namespace detail +} // namespace prometheus diff --git a/push/src/detail/label_encoder.h b/push/src/detail/label_encoder.h new file mode 100644 index 00000000..4082b7ac --- /dev/null +++ b/push/src/detail/label_encoder.h @@ -0,0 +1,13 @@ +#pragma once + +#include + +#include "prometheus/labels.h" + +namespace prometheus { +namespace detail { + +void encodeLabel(std::ostream& os, const Label& label); + +} +} // namespace prometheus \ No newline at end of file diff --git a/push/src/gateway.cc b/push/src/gateway.cc index e14eb0bc..11b7644d 100644 --- a/push/src/gateway.cc +++ b/push/src/gateway.cc @@ -8,7 +8,8 @@ #include #include -#include "curl_wrapper.h" +#include "detail/curl_wrapper.h" +#include "detail/label_encoder.h" #include "prometheus/detail/future_std.h" #include "prometheus/metric_family.h" // IWYU pragma: keep #include "prometheus/text_serializer.h" @@ -24,12 +25,13 @@ Gateway::Gateway(const std::string& host, const std::string& port, curlWrapper_ = detail::make_unique(username, password); std::stringstream jobUriStream; - jobUriStream << host << ':' << port << "/metrics/job/" << jobname; + jobUriStream << host << ':' << port << "/metrics"; + detail::encodeLabel(jobUriStream, {"job", jobname}); jobUri_ = jobUriStream.str(); std::stringstream labelStream; for (auto& label : labels) { - labelStream << "/" << label.first << "/" << label.second; + detail::encodeLabel(labelStream, label); } labels_ = labelStream.str(); } diff --git a/push/tests/CMakeLists.txt b/push/tests/CMakeLists.txt index 26e40e61..52b0f4f6 100644 --- a/push/tests/CMakeLists.txt +++ b/push/tests/CMakeLists.txt @@ -1 +1,2 @@ add_subdirectory(integration) +add_subdirectory(internal) diff --git a/push/tests/internal/BUILD.bazel b/push/tests/internal/BUILD.bazel new file mode 100644 index 00000000..2c16933e --- /dev/null +++ b/push/tests/internal/BUILD.bazel @@ -0,0 +1,13 @@ +cc_test( + name = "internal", + srcs = glob([ + "*.cc", + "*.h", + ]), + copts = ["-Iexternal/googletest/include"], + linkstatic = True, + deps = [ + "//push:push_internal_headers", + "@com_google_googletest//:gtest_main", + ], +) diff --git a/push/tests/internal/CMakeLists.txt b/push/tests/internal/CMakeLists.txt new file mode 100644 index 00000000..f505cf6f --- /dev/null +++ b/push/tests/internal/CMakeLists.txt @@ -0,0 +1,14 @@ +add_executable(prometheus_push_internal_test + label_encoder_test.cc +) + +target_link_libraries(prometheus_push_internal_test + PRIVATE + ${PROJECT_NAME}::push_internal_headers + GTest::gmock_main +) + +add_test( + NAME prometheus_push_internal_test + COMMAND prometheus_push_internal_test +) diff --git a/push/tests/internal/label_encoder_test.cc b/push/tests/internal/label_encoder_test.cc new file mode 100644 index 00000000..0c47f6f7 --- /dev/null +++ b/push/tests/internal/label_encoder_test.cc @@ -0,0 +1,43 @@ +#include "detail/label_encoder.h" + +#include + +#include +#include + +namespace prometheus { +namespace { + +class LabelEncoderTest : public testing::Test { + protected: + std::string Encode(const Label& label) { + std::stringstream ss; + detail::encodeLabel(ss, label); + return ss.str(); + } +}; + +// test cases taken from https://github.com/prometheus/pushgateway#url + +TEST_F(LabelEncoderTest, regular) { + EXPECT_EQ("/foo/bar", Encode(Label{"foo", "bar"})); +} + +TEST_F(LabelEncoderTest, empty) { + EXPECT_EQ("/first_label@base64/=", Encode(Label{"first_label", ""})); +} + +TEST_F(LabelEncoderTest, path) { + EXPECT_EQ("/path@base64/L3Zhci90bXA=", Encode(Label{"path", "/var/tmp"})); +} + +TEST_F(LabelEncoderTest, unicode) { + const char unicodeText[] = + "\xce\xa0\xcf\x81\xce\xbf\xce\xbc\xce\xb7\xce\xb8\xce\xb5\xcf\x8d\xcf" + "\x82"; // Προμηθεύς + EXPECT_EQ("/name@base64/zqDPgc6_zrzOt864zrXPjc-C", + Encode(Label{"name", unicodeText})); +} + +} // namespace +} // namespace prometheus diff --git a/util/include/prometheus/detail/base64.h b/util/include/prometheus/detail/base64.h index 21c3e31d..ebd71103 100644 --- a/util/include/prometheus/detail/base64.h +++ b/util/include/prometheus/detail/base64.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -66,6 +67,17 @@ inline std::string base64_encode(const std::string& input) { return encoded; } +// https://tools.ietf.org/html/rfc4648#section-5 +inline std::string base64url_encode(const std::string& input) { + std::string s = base64_encode(input); + std::transform(begin(s), end(s), begin(s), [](char c) { + if (c == '+') return '-'; + if (c == '/') return '_'; + return c; + }); + return s; +} + inline std::string base64_decode(const std::string& input) { if (input.length() % 4) { throw std::runtime_error("Invalid base64 length!"); diff --git a/util/tests/unit/base64_test.cc b/util/tests/unit/base64_test.cc index 7efba320..89e7b405 100644 --- a/util/tests/unit/base64_test.cc +++ b/util/tests/unit/base64_test.cc @@ -47,6 +47,14 @@ TEST(Base64Test, encodeTest) { } } +TEST(Base64Test, encodeUrlTest) { + const char unicodeText[] = + "\xce\xa0\xcf\x81\xce\xbf\xce\xbc\xce\xb7\xce\xb8\xce\xb5\xcf\x8d\xcf" + "\x82"; // Προμηθεύς + std::string encoded = detail::base64url_encode(unicodeText); + EXPECT_EQ("zqDPgc6_zrzOt864zrXPjc-C", encoded); +} + TEST(Base64Test, decodeTest) { for (const auto& test_case : testVector) { std::string decoded = detail::base64_decode(test_case.encoded);