Skip to content

Commit

Permalink
feat(request_id): set request id when it's set but empty (#38474)
Browse files Browse the repository at this point in the history
<!--
!!!ATTENTION!!!

If you are fixing *any* crash or *any* potential security issue, *do
not*
open a pull request in this repo. Please report the issue via emailing
envoy-security@googlegroups.com where the issue will be triaged
appropriately.
Thank you in advance for helping to keep Envoy secure.

!!!ATTENTION!!!

For an explanation of how to fill out the fields, please see the
relevant section
in
[PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/main/PULL_REQUESTS.md)
-->

Commit Message: `feat(request_id): set request id when it's set but
empty`
Risk Level: Low
Testing: Yes
Docs Changes: N/A
Release Notes:
* `generate_request_id` will generate a request id on non-present and
empty `x-request-id` header.

Fixes #38445

---------

Signed-off-by: Nathanael DEMACON <quantumsheep@users.noreply.github.com>
Co-authored-by: Nathanael DEMACON <quantumsheep@users.noreply.github.com>
  • Loading branch information
quantumsheep and quantumsheep authored Feb 21, 2025
1 parent f1e276f commit 4c77c6e
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 1 deletion.
5 changes: 5 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ minor_behavior_changes:
- area: cel
change: |
Support extension regex fuctions(e.g. ``re.extract``, ``re.capture`, ``re.captureN``) in CEL.
- area: http
change: |
:ref:`generate_request_id
<envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.generate_request_id>`
will generate a request id on non-present and now on empty ``x-request-id`` header.
bug_fixes:
# *Changes expected to improve the state of the world and are unlikely to have negative effects*
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/request_id/uuid/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ void UUIDRequestIDExtension::set(Http::RequestHeaderMap& request_headers, bool e
const Http::HeaderEntry* request_id_header = request_headers.RequestId();

// No request ID then set new one anyway.
if (request_id_header == nullptr) {
if (request_id_header == nullptr || request_id_header->value().empty()) {
request_headers.setRequestId(random_.uuid());
return;
}
Expand Down
45 changes: 45 additions & 0 deletions test/extensions/request_id/uuid/config_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,51 @@ TEST(UUIDRequestIDExtensionTest, SetRequestID) {
}
}

TEST(UUIDRequestIDExtensionTest, SetRequestIDWhenEmpty) {
testing::StrictMock<Random::MockRandomGenerator> random;
UUIDRequestIDExtension uuid_utils(envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig(),
random);

{
// Request ID not set.

Http::TestRequestHeaderMapImpl request_headers;

// A new request ID will be set.
EXPECT_CALL(random, uuid()).WillOnce(Return("first-request-id"));
uuid_utils.set(request_headers, false, true);
EXPECT_EQ("first-request-id", request_headers.get_(Http::Headers::get().RequestId));
}

{
// Request ID is empty.

Http::TestRequestHeaderMapImpl request_headers{{
"x-request-id",
"",
}};

// A new request ID will be set.
EXPECT_CALL(random, uuid()).WillOnce(Return("first-request-id"));
uuid_utils.set(request_headers, false, true);
EXPECT_EQ("first-request-id", request_headers.get_(Http::Headers::get().RequestId));
}

{
// Request ID is not empty.

Http::TestRequestHeaderMapImpl request_headers{{
"x-request-id",
"some-request-id",
}};

// The request ID will be kept.
EXPECT_CALL(random, uuid()).Times(0);
uuid_utils.set(request_headers, false, true);
EXPECT_EQ("some-request-id", request_headers.get_(Http::Headers::get().RequestId));
}
}

TEST(UUIDRequestIDExtensionTest, ClearExternalTraceReason) {
testing::NiceMock<Random::MockRandomGenerator> random;
UUIDRequestIDExtension uuid_utils(envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig(),
Expand Down

0 comments on commit 4c77c6e

Please sign in to comment.