bryancall opened a new pull request, #13592: URL: https://github.com/apache/trafficserver/pull/13592
Several call sites in the `stale_response` plugin discarded a `TSReturnCode` and then used an output parameter that the failing call never wrote. ### The bug `TSHttpHdrUrlGet` returns `TS_ERROR` without touching `*locp` when the header is not a request. The plugin did this in three functions: ```c TSMLoc url_loc; // uninitialized TSHttpHdrUrlGet(hdr_url_buf, hdr_url_loc, &url_loc); // may not write url_loc reqUrl.populate(hdr_url_buf, url_loc); // stack garbage used as a TSMLoc ``` On the failure path `url_loc` keeps stack garbage and is passed to `UrlComponents::populate()`, then to `TSUrlHttpQuerySet()` or `TSUrlParse()`, and finally to `TSHandleMLocRelease()`. `create_request_info()` had the same shape with `TSHttpTxnClientReqGet` and `TSHttpHdrClone`. That one is worse: `req_info` comes from `TSmalloc()` and is not zeroed, so a garbage `TSMLoc` is stored in a structure that lives for the whole transaction and is later handed to `TSMimeHdrFieldFind`, `TSHttpHdrPrint`, and `TSHandleMLocRelease`. `strip_trailing_parameter()` also set `stripped = true` before calling `TSUrlHttpQuerySet()` and ignored its result, so it reported success even when the URL was never rewritten. ### What changed Check these calls, log through the plugin's existing debug tags, and bail the way neighboring code in the same file already does (`get_pristine_url()` and `intercept_check_request()` were already checking the identical call). `create_request_info()` now returns `nullptr` on failure and its callers handle that. `TSHttpTxnEffectiveUrlStringGet` is null-checked before being passed to `TSstrndup`. ### Known limitation, not addressed here On a `TSUrlParse` failure in `add_trailing_parameter()`, the URL has already been cleared by the API before parsing begins, so that path continues with an emptied URL. This change makes the failure visible in the logs rather than silent, but does not repair the URL. Doing that properly means snapshotting the original and restoring it, which is a behavior change that belongs in its own patch. ### Testing The plugin builds clean. The changes were reviewed specifically for new leaks and double-releases on the added error paths. -- 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]
