bryancall commented on PR #13711: URL: https://github.com/apache/trafficserver/pull/13711#issuecomment-5781196691
Thanks — three of the four are addressed in ff7ca82. **`i2d_SSL_SESSION()` return type (high).** Correct, and it predates this PR. The function returns an `int` and reports failure as a non-positive value, so assigning it straight into a `size_t` turned a serialization failure into a very large length that then misreported itself through the size check as an oversized session. Now checked as an `int` before widening, with the old `len == 0` branch folded into that test. **Range allowing 0 (medium).** Agreed it is a foot-gun, and tightened further than suggested: the floor is now **4096**, not 1. That is the value the limit was fixed at before it became configurable, so no setting can cache less than ATS always did, and `proxy.config.ssl.origin_session_cache.enabled` remains the way to disable the cache. Both bounds and the reason for each are now in the admin guide. **Empty module docstring (low).** Removed. **`ssl_origin_session_max_size` typed as `int` (medium).** Leaving as is. It mirrors the field directly above it, `ssl_origin_session_cache_size`, which is also an `int` published into a `size_t` static — making only the new one `size_t` would leave two adjacent fields with the same role and different types. With the record now validated to `[4096-65536]` the value cannot be negative or exceed `size_t` range, so the conversion hazard is not reachable. Happy to convert both together if a reviewer would rather see that cleaned up, but it felt out of scope here. -- 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]
