diff --git a/examples/http-server/common/httplib.h b/examples/http-server/common/httplib.h index 9ade5fd20..88558dc4f 100644 --- a/examples/http-server/common/httplib.h +++ b/examples/http-server/common/httplib.h @@ -3110,7 +3110,7 @@ inline unsigned int str2tag(const std::string &s) { namespace udl { -inline constexpr unsigned int operator"" _t(const char *s, size_t l) { +inline constexpr unsigned int operator""_t(const char *s, size_t l) { return str2tag_core(s, l, 0); } diff --git a/include/datadog/baggage.h b/include/datadog/baggage.h index 49df4b1df..291fe8761 100644 --- a/include/datadog/baggage.h +++ b/include/datadog/baggage.h @@ -57,8 +57,10 @@ class Baggage { /// instance if no errors are encounters . /// /// @param `reader` The input `DictReader` from which to extract the data. + /// @param `opts` Extraction options governing maximum items and bytes. /// @return A `Baggage` instance or an `Error`. - static Expected extract(const DictReader& reader); + static Expected extract( + const DictReader& reader, const Options& opts = default_options); /// Initializes an empty Baggage with the default maximum capacity. Baggage() = default; diff --git a/src/datadog/baggage.cpp b/src/datadog/baggage.cpp index a0a8f6e7a..9ffc47fb5 100644 --- a/src/datadog/baggage.cpp +++ b/src/datadog/baggage.cpp @@ -41,10 +41,16 @@ constexpr bool is_allowed_value_char(char c) { } Expected, Baggage::Error> -parse_baggage(StringView input) { +parse_baggage(StringView input, const Baggage::Options& opts) { std::unordered_map result; if (input.empty()) return result; + // This API throws an error when limits are exceeded, so we can simply check + // the entire header length against the maximum bytes limit. + if (input.size() > opts.max_bytes) { + return Baggage::Error{Baggage::Error::MAXIMUM_BYTES_REACHED}; + } + enum class state : char { leading_spaces_key, key, @@ -93,8 +99,9 @@ parse_baggage(StringView input) { if (c == '=') { consume_key: size_t count = tmp_end - beg; - if (count < 1) + if (count < 1) { return Baggage::Error{Baggage::Error::MALFORMED_BAGGAGE_HEADER, i}; + } key = StringView{input.data() + beg, count}; internal_state = state::leading_spaces_value; @@ -138,11 +145,15 @@ parse_baggage(StringView input) { if (c == ',') { consume_value: size_t count = tmp_end - beg; - if (count < 1) + if (count < 1) { return Baggage::Error{Baggage::Error::MALFORMED_BAGGAGE_HEADER, tmp_end}; + } value = StringView{input.data() + beg, count}; + if (result.size() >= opts.max_items) { + return Baggage::Error{Baggage::Error::MAXIMUM_CAPACITY_REACHED}; + } result.emplace(std::string(key), std::string(value)); beg = i; tmp_end = i; @@ -158,10 +169,17 @@ parse_baggage(StringView input) { if (internal_state == state::value) { value = StringView{input.data() + beg, end - beg}; + if (result.size() >= opts.max_items) { + return Baggage::Error{Baggage::Error::MAXIMUM_CAPACITY_REACHED}; + } + result.emplace(std::string(key), std::string(value)); } else if (internal_state == state::trailing_spaces_value || internal_state == state::properties) { value = StringView{input.data() + beg, tmp_end - beg}; + if (result.size() >= opts.max_items) { + return Baggage::Error{Baggage::Error::MAXIMUM_CAPACITY_REACHED}; + } result.emplace(std::string(key), std::string(value)); } else { return Baggage::Error{Baggage::Error::MALFORMED_BAGGAGE_HEADER, end}; @@ -264,14 +282,15 @@ Expected Baggage::inject(DictWriter& writer, const Options& opts) const { return res; } -Expected Baggage::extract(const DictReader& headers) { +Expected Baggage::extract(const DictReader& headers, + const Options& opts) { auto found = headers.lookup("baggage"); if (!found) { return Baggage::Error{Error::MISSING_HEADER}; } // TODO(@dmehala): Avoid allocation - auto bv = parse_baggage(*found); + auto bv = parse_baggage(*found, opts); if (auto error = bv.if_error()) { return *error; } diff --git a/src/datadog/extraction_util.cpp b/src/datadog/extraction_util.cpp index 948631080..b5405ece2 100644 --- a/src/datadog/extraction_util.cpp +++ b/src/datadog/extraction_util.cpp @@ -152,7 +152,18 @@ Expected extract_datadog( auto trace_tags = headers.lookup("x-datadog-tags"); if (trace_tags) { - handle_trace_tags(*trace_tags, result, span_tags, logger); + // If the x-datadog-tags header value exceeds 512 bytes, drop it and record + // a propagation error tag. + if (trace_tags->size() > 512) { + logger.log_error([&](auto& stream) { + stream << "Received x-datadog-tags header value is too large. The " + "maximum size is 512 bytes, but the received value is " + << trace_tags->size() << " bytes."; + }); + span_tags[tags::internal::propagation_error] = "extract_max_size"; + } else { + handle_trace_tags(*trace_tags, result, span_tags, logger); + } } return result; diff --git a/src/datadog/tracer.cpp b/src/datadog/tracer.cpp index 43dede3d6..4209ac228 100644 --- a/src/datadog/tracer.cpp +++ b/src/datadog/tracer.cpp @@ -471,7 +471,7 @@ Expected Tracer::extract_baggage( return Baggage::Error{Baggage::Error::DISABLED}; } - auto maybe_baggage = Baggage::extract(reader); + auto maybe_baggage = Baggage::extract(reader, baggage_opts_); if (maybe_baggage) { telemetry::counter::increment(metrics::tracer::trace_context::extracted, {"header_style:baggage"}); diff --git a/src/datadog/w3c_propagation.cpp b/src/datadog/w3c_propagation.cpp index dac6753fb..f6e1eb46e 100644 --- a/src/datadog/w3c_propagation.cpp +++ b/src/datadog/w3c_propagation.cpp @@ -278,7 +278,9 @@ void parse_datadog_tracestate(ExtractedData& result, StringView datadog_value) { // `extract_tracestate` populates the `additional_w3c_tracestate` field of // `ExtractedData`, in addition to those populated by // `parse_datadog_tracestate`. -void extract_tracestate(ExtractedData& result, const DictReader& headers) { +void extract_tracestate( + ExtractedData& result, const DictReader& headers, + std::unordered_map& span_tags) { const auto maybe_tracestate = headers.lookup("tracestate"); if (!maybe_tracestate || maybe_tracestate->empty()) { return; @@ -299,6 +301,13 @@ void extract_tracestate(ExtractedData& result, const DictReader& headers) { result.additional_w3c_tracestate = std::move(other_entries); } + // If the "dd" vendor entry's value exceeds 512 bytes, drop it and record a + // propagation error tag. + if (datadog_value.size() > 512) { + span_tags[tags::internal::propagation_error] = "extract_max_size"; + return; + } + parse_datadog_tracestate(result, datadog_value); } @@ -329,7 +338,7 @@ Expected extract_w3c( } result.datadog_w3c_parent_id = "0000000000000000"; - extract_tracestate(result, headers); + extract_tracestate(result, headers, span_tags); return result; } diff --git a/test/test_baggage.cpp b/test/test_baggage.cpp index 4fb0eaa4d..ed72f1a34 100644 --- a/test/test_baggage.cpp +++ b/test/test_baggage.cpp @@ -160,7 +160,82 @@ BAGGAGE_TEST("extract") { CHECK(maybe_baggage.error().code == test_case.expected_baggage.error().code); } else { - FAIL("mistmatch between what is expected and the result"); + FAIL("mismatch between what is expected and the result"); + } + } + + SECTION("custom items and bytes limits are respected") { + struct TestCase final { + std::string name; + std::string input; + size_t max_bytes; + size_t max_items; + Expected expected_baggage; + }; + + auto test_case = GENERATE(values({ + { + "valid input respects maximum item and bytes limits", + "key1=value1,key2=value2", + 2048, + 2, + Baggage(std::unordered_map{ + {"key1", "value1"}, {"key2", "value2"}}), + }, + { + "max items reached", + "key1=value1,key2=value2", + 2048, + 1, + Baggage::Error{Baggage::Error::MAXIMUM_CAPACITY_REACHED}, + }, + { + "max items limit of zero is enforced", + "key1=value1,key2=value2", + 2048, + 0, + Baggage::Error{Baggage::Error::MAXIMUM_CAPACITY_REACHED}, + }, + { + "max bytes reached", + "key1=value1,key2=value2", + 16, + 1000, + Baggage::Error{Baggage::Error::MAXIMUM_BYTES_REACHED}, + }, + { + "max bytes value of zero is enforced", + "key1=value1,key2=value2", + 0, + 1000, + Baggage::Error{Baggage::Error::MAXIMUM_BYTES_REACHED}, + }, + { + "empty baggage does not breach max items limit or max bytes limit", + "", + 0, + 0, + Baggage(), + }, + })); + + CAPTURE(test_case.name, test_case.input, test_case.max_items, + test_case.max_bytes); + + const std::unordered_map headers{ + {"baggage", test_case.input}}; + MockDictReader reader(headers); + + Baggage::Options opts{test_case.max_bytes, test_case.max_items}; + auto maybe_baggage = Baggage::extract(reader, opts); + if (maybe_baggage.has_value() && test_case.expected_baggage.has_value()) { + CHECK(*maybe_baggage == *test_case.expected_baggage); + } else if (maybe_baggage.if_error() && + test_case.expected_baggage.if_error()) { + CHECK(maybe_baggage.error().code == + test_case.expected_baggage.error().code); + } else { + FAIL("mismatch between what is expected and the result"); } } } diff --git a/test/test_trace_segment.cpp b/test/test_trace_segment.cpp index 8da352610..792ffd7a3 100644 --- a/test/test_trace_segment.cpp +++ b/test/test_trace_segment.cpp @@ -175,8 +175,8 @@ TEST_CASE("TraceSegment finalization of spans") { SECTION("root span") { SECTION( - "'inject_max_size' propagation error if X-Datadog-Tags oversized on " - "inject") { + "'extract_max_size' propagation error if x-datadog-tags oversized on " + "extract") { auto finalized = finalize_config(config); REQUIRE(finalized); Tracer tracer{*finalized}; @@ -200,15 +200,18 @@ TEST_CASE("TraceSegment finalization of spans") { auto span = tracer.extract_span(reader); REQUIRE(span); - // Injecting the oversized X-Datadog-Tags will make `TraceSegment` note - // an error, which it will later tag on the root span. + // The oversized x-datadog-tags is rejected at extraction, so none of + // its `_dd.p.*` entries are propagated. MockDictWriter writer; span->inject(writer); - REQUIRE(writer.items.count("x-datadog-tags") == 0); + const auto injected = writer.items.find("x-datadog-tags"); + if (injected != writer.items.end()) { + REQUIRE(injected->second.find("_dd.p.0=") == std::string::npos); + } } REQUIRE(collector->first_span().tags.at( - tags::internal::propagation_error) == "inject_max_size"); + tags::internal::propagation_error) == "extract_max_size"); } SECTION("sampling priority") { diff --git a/test/test_tracer.cpp b/test/test_tracer.cpp index 26a2b233f..72231115f 100644 --- a/test/test_tracer.cpp +++ b/test/test_tracer.cpp @@ -1248,6 +1248,48 @@ TEST_TRACER("span extraction") { REQUIRE(span_tags.empty()); } + SECTION( + "'extract_max_size' propagation error if tracestate \"dd\" vendor " + "value is oversized on extract") { + std::unordered_map span_tags; + MockLogger logger; + CAPTURE(logger.entries); + CAPTURE(span_tags); + + // Build a "dd" vendor value that exceeds the 512 limit + std::string dd_value = "s:1;p:000000003ade68b1;o:synthetics"; + for (int i = 0; dd_value.size() <= 513; ++i) { + dd_value += ";t.k"; + dd_value += std::to_string(i); + dd_value += ":v"; + dd_value += std::to_string(i); + } + + std::unordered_map headers{ + {"traceparent", + "00-00000000000000000000000000000001-0000000000000001-00"}, + {"tracestate", "dd=" + dd_value + ",vendorx=keepme"}}; + MockDictReader reader{headers}; + + const auto extracted = extract_w3c(reader, span_tags, logger); + REQUIRE(extracted); + + // The oversized "dd" entry was rejected, so + // origin/trace_tags/propagated_tags/etc. were not parsed from it. Other + // vendor entries are preserved for propagation. + REQUIRE(extracted->origin == nullopt); + REQUIRE(extracted->trace_tags.empty()); + REQUIRE(extracted->sampling_priority == 0); + REQUIRE(extracted->additional_w3c_tracestate == "vendorx=keepme"); + REQUIRE(extracted->additional_datadog_w3c_tracestate == nullopt); + REQUIRE(extracted->datadog_w3c_parent_id == "0000000000000000"); + + REQUIRE(logger.entries.empty()); + REQUIRE(span_tags.count(tags::internal::propagation_error) == 1); + REQUIRE(span_tags.at(tags::internal::propagation_error) == + "extract_max_size"); + } + SECTION("W3C Phase 3 support - Preferring tracecontext") { // Tests behavior from system-test // test_headers_tracecontext.py::test_tracestate_w3c_p_extract_datadog_w3c @@ -1456,6 +1498,49 @@ TEST_TRACER("span extraction") { "malformed_tid invalidhex"); } } + + SECTION("exceeds the extract limit") { + std::string header_value = "_dd.p.first=ok"; + for (int i = 0; header_value.size() <= 512; ++i) { + header_value += ",_dd.p.k"; + header_value += std::to_string(i); + header_value += "="; + header_value += std::string(40, 'v'); + } + headers["x-datadog-tags"] = header_value; + + SECTION("is not propagated") { + auto maybe_span = tracer.extract_span(reader); + REQUIRE(maybe_span); + + // The oversized header was rejected, so the trace tags from it must + // not have been propagated. Inject and confirm "_dd.p.first" is + // absent. + MockDictWriter writer; + maybe_span->inject(writer); + + const auto injected = writer.items.find("x-datadog-tags"); + if (injected != writer.items.end()) { + REQUIRE(injected->second.find("_dd.p.first") == std::string::npos); + } + } + + SECTION("is noted in an error tag value") { + { + auto maybe_span = tracer.extract_span(reader); + REQUIRE(maybe_span); + } + // Now that the span is destroyed, it will have been sent to the + // collector. + // We can inspect the `SpanData` in the collector to verify that the + // `tags::internal::propagation_error` ("_dd.propagation_error") tag + // is set to the expected value. + const SpanData& span = collector->first_span(); + REQUIRE(span.tags.count(tags::internal::propagation_error) == 1); + REQUIRE(span.tags.find(tags::internal::propagation_error)->second == + "extract_max_size"); + } + } } }