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]

Reply via email to