bneradt commented on code in PR #13790:
URL: https://github.com/apache/trafficserver/pull/13790#discussion_r4197888826
##########
plugins/header_rewrite/operators.cc:
##########
@@ -532,6 +523,320 @@ OperatorRMDestination::exec(const Resources &res) const
return true;
}
+// OperatorSortDestination
+void
+OperatorSortDestination::initialize(Parser &p)
+{
+ Operator::initialize(p);
+
+ _url_qual = parse_url_qualifier(p.get_arg());
+
+ require_resources(RSRC_CLIENT_REQUEST_HEADERS);
+ require_resources(RSRC_SERVER_REQUEST_HEADERS);
+}
+
+bool
+OperatorSortDestination::exec(const Resources &res) const
+{
+ if (res._rri || (res.bufp && res.hdr_loc)) {
+ TSMBuffer bufp;
+ TSMLoc url_m_loc;
+
+ // Determine which TSMBuffer and TSMLoc to use
+ if (res._rri && !res.changed_url) {
+ bufp = res._rri->requestBufp;
+ url_m_loc = res._rri->requestUrl;
+ } else {
+ bufp = res.bufp;
+ if (TSHttpHdrUrlGet(res.bufp, res.hdr_loc, &url_m_loc) != TS_SUCCESS) {
+ Dbg(pi_dbg_ctl, "TSHttpHdrUrlGet was unable to return the url m_loc");
+ return true;
+ }
+ }
+
+ switch (_url_qual) {
+ case URL_QUAL_QUERY: {
+ int q_len = 0;
+ const char *q_ptr = TSUrlHttpQueryGet(bufp, url_m_loc, &q_len);
+ std::string_view query = q_len > 0 ? std::string_view(q_ptr,
static_cast<size_t>(q_len)) : std::string_view();
+
+ if (is_query_sorted(query)) {
+ Dbg(pi_dbg_ctl, "OperatorSortDestination::exec() QUERY already sorted,
leaving it unchanged");
+ break;
+ }
+
+ std::string sorted = sort_query(query);
+
+ if (TSUrlHttpQuerySet(bufp, url_m_loc, sorted.c_str(), sorted.size()) !=
TS_SUCCESS) {
+ Dbg(pi_dbg_ctl, "OperatorSortDestination::exec() unable to set QUERY");
+ break;
+ }
+ const_cast<Resources &>(res).changed_url = true;
+ res.reset_query_cache();
+ Dbg(pi_dbg_ctl, "OperatorSortDestination::exec() rewrote QUERY to
\"%s\"", sorted.c_str());
+ break;
+ }
+ default:
+ Dbg(pi_dbg_ctl, "Sort destination %i has no handler", _url_qual);
+ break;
+ }
+ } else {
+ Dbg(pi_dbg_ctl, "OperatorSortDestination::exec() unable to continue due to
missing bufp=%p or hdr_loc=%p, rri=%p!", res.bufp,
+ res.hdr_loc, res._rri);
+ }
+ return true;
+}
+
+// OperatorSetKey
+void
+OperatorSetKey::initialize(Parser &p)
+{
+ Operator::initialize(p);
+
+ _url_qual = parse_url_qualifier(p.get_arg());
+ switch (_url_qual) {
+ case URL_QUAL_HOST:
+ case URL_QUAL_PORT:
+ case URL_QUAL_PATH:
+ case URL_QUAL_QUERY:
+ case URL_QUAL_SCHEME:
+ break;
+ default:
+ throw std::runtime_error("set-cache-key accepts HOST, PORT, PATH, QUERY,
or SCHEME, got: " + p.get_arg());
+ }
+
+ _value.set_value(p.get_value(), this);
+ require_resources(RSRC_CLIENT_REQUEST_HEADERS);
+}
+
+void
+OperatorSetKey::initialize_hooks()
+{
+ add_allowed_hook(TS_REMAP_PSEUDO_HOOK);
+ add_allowed_hook(TS_HTTP_POST_REMAP_HOOK);
+}
+
+bool
+OperatorSetKey::exec(const Resources &res) const
+{
+ if (!res.ensure_key_url()) {
+ Dbg(pi_dbg_ctl, "OperatorSetKey::exec() unable to create the cache URL");
+ return true;
+ }
+
+ UrlKeyState &key = res.cache_key;
+ std::string value;
+
+ _value.append_value(value, res);
+
+ // Unlike set-destination, an empty value is applied: it clears the
component.
+ switch (_url_qual) {
+ case URL_QUAL_HOST:
+ TSUrlHostSet(key.bufp, key.url_loc, value.data(), value.size());
+ break;
+ case URL_QUAL_PATH:
+ TSUrlPathSet(key.bufp, key.url_loc, value.data(), value.size());
+ break;
+ case URL_QUAL_QUERY:
+ TSUrlHttpQuerySet(key.bufp, key.url_loc, value.data(), value.size());
+ break;
+ case URL_QUAL_SCHEME:
+ TSUrlSchemeSet(key.bufp, key.url_loc, value.data(), value.size());
+ break;
+ case URL_QUAL_PORT: {
+ int port = 0;
+
+ if (!value.empty()) {
+ auto [end, ec] = std::from_chars(value.data(), value.data() +
value.size(), port);
+
+ if (ec != std::errc() || end != value.data() + value.size() || port < 0
|| port > 0xFFFF) {
+ Dbg(pi_dbg_ctl, "OperatorSetKey::exec() invalid PORT \"%s\",
skipping", value.c_str());
+ return true;
+ }
+ }
+ TSUrlPortSet(key.bufp, key.url_loc, port);
+ break;
+ }
+ default:
+ return true;
+ }
+
+ key.active = true;
+ Dbg(pi_dbg_ctl, "OperatorSetKey::exec() set component %d to \"%s\"",
_url_qual, value.c_str());
+ return true;
+}
+
+// OperatorAddKey
+void
+OperatorAddKey::initialize(Parser &p)
+{
+ Operator::initialize(p);
+
+ _value.set_value(p.get_arg(), this);
+ require_resources(RSRC_CLIENT_REQUEST_HEADERS);
+}
+
+void
+OperatorAddKey::initialize_hooks()
+{
+ add_allowed_hook(TS_REMAP_PSEUDO_HOOK);
+ add_allowed_hook(TS_HTTP_POST_REMAP_HOOK);
+}
+
+bool
+OperatorAddKey::exec(const Resources &res) const
+{
+ if (!res.ensure_key_url()) {
+ Dbg(pi_dbg_ctl, "OperatorAddKey::exec() unable to create the cache URL");
+ return true;
+ }
+
+ std::string value;
+
+ // An empty value still adds a segment, so segment i always holds the same
input.
+ _value.append_value(value, res);
+ Dbg(pi_dbg_ctl, "OperatorAddKey::exec() adding segment \"%s\"",
value.c_str());
+ res.cache_key.key_data.push_back(std::move(value));
Review Comment:
[P2] Include queued segments in CACHE-URL reads
This only updates key_data; ConditionUrl::append_value(CACHE) reads
cache_key.url_loc, and the queued segments are not appended to that URL until
finalize_key_ops() runs after all rules. With a base path p, `add-cache-key
"en"` followed by `set-header X-Key "%{CACHE-URL:PATH}"` emits p even though
the committed key has path p/en. A subsequent CACHE-URL:PATH condition can
consequently skip the intended rule. This contradicts the documented promise
that later CACHE-URL conditions see preceding cache-key edits. Please make
PATH/full-URL reads project the pending segments (while preserving
clear-cache-key semantics), and add replay coverage that reads and conditions
on the key between add-cache-key and the hook's final commit.
--
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]
