Copilot commented on code in PR #13133:
URL: https://github.com/apache/trafficserver/pull/13133#discussion_r3184847930
##########
src/iocore/cache/P_CacheDoc.h:
##########
@@ -82,6 +82,10 @@ Doc::prefix_len() const
inline uint32_t
Doc::data_len() const
{
+ if (this->len <= sizeof(self_type) + this->hlen) {
+ return 0;
+ }
+
return this->len - sizeof(self_type) - this->hlen;
Review Comment:
This only hardens the in-process cache `Doc::data_len()` implementation.
`src/traffic_cache_tool/CacheDefs.cc:149-152` still computes `len - sizeof(Doc)
- hlen` without a guard, so malformed cache entries can still underflow when
they are inspected with `traffic_cache_tool`, and the two `Doc` implementations
now have different semantics.
##########
src/iocore/cache/P_CacheDoc.h:
##########
@@ -82,6 +82,10 @@ Doc::prefix_len() const
inline uint32_t
Doc::data_len() const
{
+ if (this->len <= sizeof(self_type) + this->hlen) {
+ return 0;
+ }
+
return this->len - sizeof(self_type) - this->hlen;
Review Comment:
The new underflow guard changes behavior for malformed length fields, but
there is no regression test covering the new boundary cases (`len ==
sizeof(Doc) + hlen` and `len < sizeof(Doc) + hlen`). This code path already has
unit coverage in `src/iocore/cache/unit_tests/test_Stripe.cc`, so adding one
malformed-document test here would help prevent future reintroductions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]