You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "bneradt (via GitHub)" <gi...@apache.org> on 2025/11/18 22:22:26 UTC
[PR] cookie_remap: set_sendto_headers [trafficserver]
bneradt opened a new pull request, #12672:
URL: https://github.com/apache/trafficserver/pull/12672
This adds a new set_sendto_headers configuration option that allows setting arbitrary HTTP headers (including the Host header) on requests to sendto destinations when cookie rules match. Header values support dynamic substitution of regex capture groups ($1, $2, etc.) and path variables ($path, $cr_req_url, etc.). When the Host header is set via this option, pristine_host_hdr is automatically disabled for that transaction.
--
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] cookie_remap: set_sendto_headers [trafficserver]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #12672: cookie_remap: set_sendto_headers
URL: https://github.com/apache/trafficserver/pull/12672
--
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] cookie_remap: set_sendto_headers [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #12672:
URL: https://github.com/apache/trafficserver/pull/12672#discussion_r2582738664
##########
plugins/experimental/cookie_remap/cookie_remap.cc:
##########
@@ -1268,12 +1344,101 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri)
TSError("can't parse substituted URL string");
goto error;
} else {
+ bool host_header_was_set = false;
+
+ // Set custom headers if configured and we took the sendto path.
+ if (!op->getSendtoHeaders().empty() && used_sendto) {
+ for (auto const &header_pair : op->getSendtoHeaders()) {
+ std::string header_name = header_pair.first;
+ std::string header_value = header_pair.second;
+
+ // Apply regex substitution to header value if we have regex matches ($1, $2, etc.)
+ if (regex_ccount > 0 && !regex_match_strings.empty() && header_value.find('$') != std::string::npos) {
+ std::string::size_type pos = 0;
+ std::string::size_type ppos = 0;
+ std::string substituted_value;
+ substituted_value.reserve(header_value.size() * 2);
+
+ while (pos < header_value.length()) {
+ pos = header_value.find('$', ppos);
+ if (pos == std::string::npos) {
+ break;
+ }
+ // Check if there's a digit after the $
+ if (pos + 1 < header_value.length() && isdigit(header_value[pos + 1])) {
+ int const ix = header_value[pos + 1] - '0';
+ if (ix <= regex_ccount && ix < static_cast<int>(regex_match_strings.size())) {
+ // Append everything before the $
+ substituted_value += header_value.substr(ppos, pos - ppos);
+ // Append the regex match string
+ substituted_value += regex_match_strings[ix];
+ // Move past the $N
+ ppos = pos + 2;
+ }
+ }
+ pos++;
+ }
+ // Append any remaining text
+ if (ppos < header_value.length()) {
+ substituted_value += header_value.substr(ppos);
+ }
+ header_value = substituted_value;
+ }
Review Comment:
The regex substitution logic for header values has a bug where unmatched `$` characters are silently dropped. If a `$` in the header value is not followed by a valid digit or doesn't match a regex capture group, the character is skipped rather than preserved in the output.
For example, if `header_value = "test$invalid"` and there are no regex matches, the output will be `"test"` instead of `"test$invalid"`.
The issue is that when the condition at line 1368-1377 is not met (e.g., `$` not followed by a digit, or invalid capture group index), the loop increments `pos++` at line 1379 but never appends the `$` character to `substituted_value`. The `ppos` pointer is only updated when a successful substitution occurs, so non-substituted `$` characters between `ppos` and `pos` are lost.
The fix should either:
1. Append the `$` character when it's not part of a valid substitution pattern (similar to how the original sendto URL substitution works at lines 745-765), or
2. Only increment `ppos` when a substitution is made, and handle the `$` character explicitly when it's not part of a valid pattern.
##########
plugins/experimental/cookie_remap/cookie_remap.cc:
##########
@@ -1268,12 +1344,101 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri)
TSError("can't parse substituted URL string");
goto error;
} else {
+ bool host_header_was_set = false;
+
+ // Set custom headers if configured and we took the sendto path.
+ if (!op->getSendtoHeaders().empty() && used_sendto) {
+ for (auto const &header_pair : op->getSendtoHeaders()) {
+ std::string header_name = header_pair.first;
+ std::string header_value = header_pair.second;
+
+ // Apply regex substitution to header value if we have regex matches ($1, $2, etc.)
+ if (regex_ccount > 0 && !regex_match_strings.empty() && header_value.find('$') != std::string::npos) {
+ std::string::size_type pos = 0;
+ std::string::size_type ppos = 0;
+ std::string substituted_value;
+ substituted_value.reserve(header_value.size() * 2);
+
+ while (pos < header_value.length()) {
+ pos = header_value.find('$', ppos);
+ if (pos == std::string::npos) {
+ break;
+ }
+ // Check if there's a digit after the $
+ if (pos + 1 < header_value.length() && isdigit(header_value[pos + 1])) {
+ int const ix = header_value[pos + 1] - '0';
+ if (ix <= regex_ccount && ix < static_cast<int>(regex_match_strings.size())) {
+ // Append everything before the $
+ substituted_value += header_value.substr(ppos, pos - ppos);
+ // Append the regex match string
+ substituted_value += regex_match_strings[ix];
+ // Move past the $N
+ ppos = pos + 2;
+ }
+ }
+ pos++;
+ }
+ // Append any remaining text
+ if (ppos < header_value.length()) {
+ substituted_value += header_value.substr(ppos);
+ }
+ header_value = substituted_value;
+ }
+
+ // Apply cr_substitutions for variables like $path, $cr_req_url, etc.
+ cr_substitutions(header_value, req_url);
+
+ Dbg(dbg_ctl, "Setting header: %s to value: %s", header_name.c_str(), header_value.c_str());
+
+ // Find or create the header
+ TSMLoc field_loc = TSMimeHdrFieldFind(rri->requestBufp, rri->requestHdrp, header_name.c_str(), header_name.length());
+
+ if (field_loc == TS_NULL_MLOC) {
+ // Header doesn't exist, create it
+ if (TS_SUCCESS == TSMimeHdrFieldCreateNamed(rri->requestBufp, rri->requestHdrp, header_name.c_str(),
+ header_name.length(), &field_loc)) {
+ if (TS_SUCCESS == TSMimeHdrFieldValueStringSet(rri->requestBufp, rri->requestHdrp, field_loc, -1,
+ header_value.c_str(), header_value.length())) {
+ TSMimeHdrFieldAppend(rri->requestBufp, rri->requestHdrp, field_loc);
+ Dbg(dbg_ctl, "Created and set header: %s", header_name.c_str());
+ }
+ TSHandleMLocRelease(rri->requestBufp, rri->requestHdrp, field_loc);
+ }
+ } else {
+ // Header exists, update it
+ TSMLoc tmp = TS_NULL_MLOC;
+ bool first = true;
+
+ while (field_loc != TS_NULL_MLOC) {
+ tmp = TSMimeHdrFieldNextDup(rri->requestBufp, rri->requestHdrp, field_loc);
+ if (first) {
+ first = false;
+ if (TS_SUCCESS == TSMimeHdrFieldValueStringSet(rri->requestBufp, rri->requestHdrp, field_loc, -1,
+ header_value.c_str(), header_value.length())) {
+ Dbg(dbg_ctl, "Updated header: %s", header_name.c_str());
+ }
+ } else {
+ // Remove duplicate headers
+ TSMimeHdrFieldDestroy(rri->requestBufp, rri->requestHdrp, field_loc);
+ }
+ TSHandleMLocRelease(rri->requestBufp, rri->requestHdrp, field_loc);
+ field_loc = tmp;
+ }
+ }
+
+ // Check if we're setting the Host header (case-insensitive)
+ if (strcasecmp(header_name.c_str(), "Host") == 0) {
Review Comment:
Missing include for `strcasecmp`. The `strcasecmp` function requires `<cstring>` to be included. While this may compile on some systems due to transitive includes, it's not guaranteed by the C++ standard and should be explicitly included.
Other files in the same plugin (cookiejar.cc, hash.cc, strip.cc) and other plugins in the codebase (url_sig.cc) that use `strcasecmp` include `<cstring>`.
Add `#include <cstring>` to the includes at the top of the file (around line 34).
##########
doc/admin-guide/plugins/cookie_remap.en.rst:
##########
@@ -233,6 +234,110 @@ This option only affects the successful match (``sendto``) path. The ``else``
path will continue to use the configured pristine host header setting (typically
enabled in production environments).
+.. _set-sendto-headers:
+
+set_sendto_headers: [optional]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Sets arbitrary HTTP request headers when a rule matches and takes the ``sendto``
+path. This provides a flexible way to modify request headers, including the
+Host header, for redirected requests.
+
+Headers are only set when:
+
+* The operation matches successfully.
+* The ``sendto`` path is taken (not the ``else`` path).
+
+**Format:**
+
+The value must be a YAML sequence (list) where each item is a single-key map
+representing a header name and its value:
+
+.. code-block:: yaml
+
+ set_sendto_headers:
+ - Header-Name: header-value
+ - Another-Header: another-value
+
+**Header Value Substitution:**
+
+The special variables mentioned below for regex capture groups, path variables,
+URL variables, unmatched path, and URL encoding can be used in the header value.
+
+* **Regex capture groups**: ``$1``, ``$2``, ... ``$9`` (from regex operations).
+* **Path variables**: ``$path``, ``$ppath`` (pre-remapped path).
+* **URL variables**: ``$cr_req_url``, ``$cr_req_purl`` (pre-remapped URL).
+* **Unmatched path**: ``$unmatched_path``, ``$unmatched_ppath``.
+* **URL encoding**: ``$cr_urlencode(...)``.
+
+**Special Behavior for Host Header:**
+
+When ``set_sendto_headers`` includes a ``Host`` header (case-insensitive), the
+pristine host header is automatically disabled for that transaction. This allows
+the Host header to be updated to the specified value. You do not need to also
+set ``disable_pristine_host_hdr: true`` in this case.
+
+**Interaction with disable_pristine_host_hdr:**
+
+* If ``set_sendto_headers`` sets the Host header, pristine host header is
+ automatically disabled.
+* If ``set_sendto_headers`` does NOT set Host but ``disable_pristine_host_hdr``
+ is ``true``, pristine host header is still disabled
Review Comment:
Missing period at the end of the sentence. The sentence should end with a period for consistency with the other bullet points.
```suggestion
is ``true``, pristine host header is still disabled.
```
--
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] cookie_remap: set_sendto_headers [trafficserver]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12672:
URL: https://github.com/apache/trafficserver/pull/12672#issuecomment-4071880317
This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.
--
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