Copilot commented on code in PR #12569:
URL: https://github.com/apache/trafficserver/pull/12569#discussion_r2456093407
##########
plugins/experimental/url_sig/url_sig.cc:
##########
@@ -836,10 +832,10 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp,
TSRemapRequestInfo *rri)
switch (cfg->err_status) {
case TS_HTTP_STATUS_MOVED_TEMPORARILY:
- Dbg(dbg_ctl, "Redirecting to %s", cfg->err_url);
+ Dbg(dbg_ctl, "Redirecting to %s", cfg->err_url.c_str());
char *start, *end;
- start = cfg->err_url;
- end = start + strlen(cfg->err_url);
+ start = const_cast<char *>(cfg->err_url.c_str());
+ end = start + cfg->err_url.size();
if (TSUrlParse(rri->requestBufp, rri->requestUrl, (const char **)&start,
end) != TS_PARSE_DONE) {
err_log("url", 3, "Error inn TSUrlParse!");
Review Comment:
Corrected spelling of 'inn' to 'in'.
```suggestion
err_log("url", 3, "Error in TSUrlParse!");
```
##########
plugins/experimental/url_sig/url_sig.cc:
##########
@@ -836,10 +832,10 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp,
TSRemapRequestInfo *rri)
switch (cfg->err_status) {
case TS_HTTP_STATUS_MOVED_TEMPORARILY:
- Dbg(dbg_ctl, "Redirecting to %s", cfg->err_url);
+ Dbg(dbg_ctl, "Redirecting to %s", cfg->err_url.c_str());
char *start, *end;
- start = cfg->err_url;
- end = start + strlen(cfg->err_url);
+ start = const_cast<char *>(cfg->err_url.c_str());
Review Comment:
Using const_cast to remove const from std::string::c_str() is unsafe if
TSUrlParse modifies the string. This violates std::string's internal buffer
guarantees. If TSUrlParse requires a mutable buffer, copy err_url to a mutable
buffer first.
##########
plugins/experimental/url_sig/url_sig.cc:
##########
@@ -582,7 +577,8 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo
*rri)
// check for path params.
if (query == nullptr || strstr(query, "E=") == nullptr) {
- char *const parsed = urlParse(url, cfg->sig_anchor, new_path, 8192,
path_params, 8192);
+ char *const parsed = urlParse(url, cfg->sig_anchor.empty() ? nullptr :
const_cast<char *>(cfg->sig_anchor.c_str()), new_path,
Review Comment:
The const_cast here suggests urlParse may modify the sig_anchor string,
which is unsafe when passing std::string::c_str(). Consider refactoring
urlParse to accept const char* if it doesn't modify the string, or document
this requirement clearly.
```suggestion
// Create a mutable copy of sig_anchor if not empty
std::string sig_anchor_copy;
char *sig_anchor_ptr = nullptr;
if (!cfg->sig_anchor.empty()) {
sig_anchor_copy = cfg->sig_anchor;
sig_anchor_ptr = &sig_anchor_copy[0];
}
char *const parsed = urlParse(url, cfg->sig_anchor.empty() ? nullptr :
sig_anchor_ptr, new_path,
```
--
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]
