bryancall commented on PR #13704:
URL: https://github.com/apache/trafficserver/pull/13704#issuecomment-5725573033

   Correcting my own description: I overstated the severity, and the original 
text was up for a few hours, so the correction is worth stating rather than 
quietly editing.
   
   I wrote that release builds had both options broken and that the header 
handle was leaking. Both are wrong.
   
   `mime_hdr_field_find()` opens with `hdrtoken_is_wks(field_name.data())`, 
which is a pointer-range test against the well-known-string heap and never 
looks at the length. `FILTER_HEADERS` holds real WKS pointers, so the 
zero-length lookup still took the WKS-index path and found the field. Range 
detection worked in release. And `TSHandleMLocRelease()` returns `TS_SUCCESS` 
immediately for an `HTTP_HEADER` object, so releasing the header mloc frees 
nothing and nothing was leaking there.
   
   What is actually wrong, and what this change fixes:
   
   - a leaked `MIMEFieldSDKHandle` on every request carrying one of the six 
headers, because `TSMimeHdrFieldFind()` allocates a handle per hit and the 
boolean-context call discarded it. That one is real on any build, and it is the 
field handle, not the header handle.
   - a debug-build abort on `ink_assert(!field_name.empty())`, compiled out in 
release.
   
   I have also been precise in the description about what the new test proves: 
it discriminates on an assertion-enabled build, which is what AuTest and CI 
use, but it would pass against the unfixed plugin in a release build, since 
there the defect is the leak rather than a behaviour change. The runs still 
earn their place because they pin two options that had no coverage at all.
   
   The diff itself is unchanged and still correct. `10.2.x` and `10.1.x` both 
carry the bug; `9.2.x` does not ship this plugin.


-- 
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