You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "bryancall (via GitHub)" <gi...@apache.org> on 2026/03/16 20:19:47 UTC
[PR] Fix uninitialized fields in header_rewrite and ja4_fingerprint plugins [trafficserver]
bryancall opened a new pull request, #12977:
URL: https://github.com/apache/trafficserver/pull/12977
## Summary
Initialize members that Coverity flagged as uninitialized:
- **OperatorSetHttpCntl::_cntl_qual**: init to `TS_HTTP_CNTL_LOGGING_MODE` (CID 1497355)
- **OperatorSetPluginCntl::_name**: init to `PluginCtrl::TIMEZONE` (CID 1644243)
- **OperatorSetPluginCntl::_value**: init to `0`
- **TLSClientHelloSummary::protocol**: init to `Protocol::TLS` (CID 1587255)
These are all trivial default-initializer additions — no behavioral changes.
## Test Plan
- [x] Builds cleanly with ASAN (dev-asan preset)
- [x] All 3 header_rewrite unit tests pass (test_header_rewrite, verify_global_header_rewrite, verify_remap_header_rewrite)
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix uninitialized fields in header_rewrite and ja4_fingerprint plugins [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall merged PR #12977:
URL: https://github.com/apache/trafficserver/pull/12977
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix uninitialized fields in header_rewrite and ja4_fingerprint plugins [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #12977:
URL: https://github.com/apache/trafficserver/pull/12977#discussion_r2943825958
##########
plugins/header_rewrite/operators.h:
##########
@@ -487,8 +487,8 @@ class OperatorSetPluginCntl : public Operator
}
private:
- PluginCtrl _name;
- int _value;
+ PluginCtrl _name{PluginCtrl::TIMEZONE}; // always overwritten by initialize()
+ int _value{0};
Review Comment:
The comment says _name is "always overwritten by initialize()", but initialize() does not overwrite it when the control name is unknown (it only logs an error). With the new default initializer (TIMEZONE), this can mask parse failures and lead to unintended behavior; consider using an explicit invalid default (or a separate validity flag) and updating/removing the misleading comment.
##########
plugins/header_rewrite/operators.cc:
##########
@@ -1232,6 +1232,8 @@ OperatorSetPluginCntl::initialize(Parser &p)
} else {
TSError("[%s] Unknown value for INBOUND_IP_SOURCE control: %s", PLUGIN_NAME, value.c_str());
}
+ } else {
+ TSError("[%s] Unknown plugin control name: %s", PLUGIN_NAME, name.c_str());
}
Review Comment:
OperatorSetPluginCntl::initialize() logs an error on unknown control name/value but leaves the operator in a state that will still execute using the default-initialized _name/_value. That means an invalid config can silently set TIMEZONE to 0 (TIMEZONE_LOCAL) or set an unintended INBOUND_IP_SOURCE. Consider marking the operator invalid on parse errors (e.g., add an INVALID enum / _valid flag) and make exec() a no-op (or otherwise prevent execution) when initialization did not successfully parse a supported control/value.
##########
plugins/experimental/ja4_fingerprint/ja4.h:
##########
@@ -53,8 +53,8 @@ class TLSClientHelloSummary
public:
using difference_type = std::iterator_traits<std::vector<std::uint16_t>::iterator>::difference_type;
- Protocol protocol;
- std::uint16_t TLS_version{0}; // 0 is not the default, this is only to not have it un-initialized.
+ Protocol protocol{Protocol::TLS}; // always overwritten by caller
Review Comment:
The new default initializer sets protocol to Protocol::TLS but the comment says it is "always overwritten by caller". If callers rely on the default (or forget to set it), this comment becomes misleading; either remove/adjust the comment or enforce explicit initialization (e.g., via a constructor parameter) if a default is not intended.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix uninitialized fields in header_rewrite and ja4_fingerprint plugins [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #12977:
URL: https://github.com/apache/trafficserver/pull/12977#discussion_r2942882711
##########
plugins/header_rewrite/operators.h:
##########
@@ -487,8 +487,8 @@ class OperatorSetPluginCntl : public Operator
}
private:
- PluginCtrl _name;
- int _value;
+ PluginCtrl _name = PluginCtrl::TIMEZONE;
+ int _value = 0;
Review Comment:
`OperatorSetPluginCntl::initialize()` (operators.cc:1211+) has no `else`/error path for an unknown control name. With `_name`/`_value` now default-initialized (TIMEZONE/0), a config typo will silently apply TIMEZONE=LOCAL rather than failing fast. Consider adding an explicit INVALID enum value (or a boolean) set during `initialize()`, log a `TSError` for unknown names, and have `exec()` no-op/return false when invalid so misconfigurations don’t change transaction state.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org