moonchen commented on code in PR #13303:
URL: https://github.com/apache/trafficserver/pull/13303#discussion_r3444438854
##########
plugins/header_rewrite/operators.cc:
##########
@@ -100,10 +100,11 @@ createRequestString(const std::string_view &value, char
(&req_buf)[MAX_SIZE], in
if (TSUrlCreate(url_buf, &url_loc) == TS_SUCCESS && TSUrlParse(url_buf,
url_loc, &start, end) == TS_PARSE_DONE) {
const char *host = TSUrlHostGet(url_buf, url_loc, &host_len);
- const char *url = TSUrlStringGet(url_buf, url_loc, &url_len);
+ char *url = TSUrlStringGet(url_buf, url_loc, &url_len);
*req_buf_size = snprintf(req_buf, MAX_SIZE, "GET %.*s HTTP/1.1\r\nHost:
%.*s\r\n\r\n", url_len, url, host_len, host);
Review Comment:
On the nullptr concern: this is a false positive. `TSUrlStringGet()` returns
`ats_malloc(len + 1)`, which `ink_abort()`s on allocation failure rather than
returning null, so `url` is never null here. `TSUrlHostGet()` can only yield
null when the host length is 0, and `snprintf("%.*s", 0, ...)` with a zero
precision does not dereference the pointer, so there is no UB on that path.
On the truncation concern: this one is valid and is fixed in 0e0692545d.
`createRequestString()` now checks the `snprintf()` return value and returns
`TS_ERROR` instead of passing an over-large length to `TSFetchUrl()` when the
formatted request would not fit in `req_buf`.
--
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]
