stale_response: handle TS API failures instead of ignoring them

[bryancall]

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:

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.

[Copilot]

Pull request overview

This PR hardens the stale_response experimental plugin by handling Traffic Server API failure returns instead of proceeding with uninitialized TSMLoc output parameters, preventing invalid handle use, incorrect “success” reporting, and potential bad releases/leaks in error paths.

Changes:

• Initialize TSMLoc outputs to TS_NULL_MLOC and check TSReturnCode results before using output parameters (notably around URL/header access).
• Propagate failures cleanly (e.g., create_request_info()/create_state_info() returning nullptr) and ensure callers bail out safely.
• Fix strip_trailing_parameter() to report success only when the URL query rewrite actually succeeds, and add debug/error logging for new failure paths.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
plugins/experimental/stale_response/stale_response.cc	Adds return-code checks and early-exit cleanup around txn/header access, logging setup, and hook handlers to avoid using uninitialized output parameters.
plugins/experimental/stale_response/CacheUpdate.cc	Initializes URL TSMLoc outputs and checks TSHttpHdrUrlGet / TSUrlParse / TSUrlHttpQuerySet results to avoid invalid handle usage and false “stripped” reporting.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.