Skip to content

Conversation

@vuori
Copy link
Contributor

@vuori vuori commented Nov 3, 2025

Handling of the Accept-Encoding field in the stats_over_http plugin used strstr() on the result of TSMimeHdrFieldValueStringGet leading to read of uninitialized memory. As a workaround, make a null-terminated copy of the header string.

As a matter of opinion, I think the compression support in stats_over_http should be completely removed as it significantly complicates the code for questionable benefit. Ideally users who care about this could stack the compress plugin, though I'm not clear on whether this works with intercept?

Handling of the Accept-Encoding field used strstr() on the result
of TSMimeHdrFieldValueStringGet leading to read of uninitialized
memory. As a workaround, make a null-terminated copy of the
returned string.

// Ensure str is null-terminated for strstr()
if (len > 0) {
str = TSstrndup(mime_str, len);
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of this, could you create a std::string_view from {mime_str, Len} and use std::string_view::find to search for the string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ugghhh I'll think about it. Not very enthusiastic at all about all this code that just duplicates compress, the only reason I'm touching this is to make a thing that insists on sending Accept-Encoding: identity work reliably.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the std::string_view suggestion too. It would be nice to avoid the cost of the allocation and the need to free.

Thank you for working on this.

@bryancall bryancall added this to the 10.2.0 milestone Nov 3, 2025
@bryancall bryancall added the Bug label Nov 3, 2025
@ezelkow1
Copy link
Member

ezelkow1 commented Nov 3, 2025

[approve ci]

@cmcfarlen cmcfarlen self-requested a review November 10, 2025 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants