Copilot commented on code in PR #13587:
URL: https://github.com/apache/trafficserver/pull/13587#discussion_r3858239823


##########
tools/hrw4u/src/hrw_symbols.py:
##########
@@ -408,6 +409,16 @@ def negate_expression(self, term: str) -> str:
 
     def percent_to_ident_or_func(self, percent: str, section: SectionType | 
None) -> tuple[str, bool]:
         """Convert percent block to identifier or function call."""
+        stripped, mods = split_percent_mods(percent)
+        expr, is_func = self._percent_to_ident_or_func(stripped, section)
+
+        # An unresolved block comes back verbatim, so re-attaching mods would 
duplicate them.
+        if mods and expr != stripped:
+            expr = f"{expr} with {','.join(mods)}"
+

Review Comment:
   percent_to_ident_or_func() strips trailing [MODS] via split_percent_mods(), 
but only re-attaches modifiers when the %{} block was successfully mapped to a 
DSL symbol/function (expr != stripped). If the block is left as a raw %{} 
expression (e.g., section-label tags or any unrecognized tag), modifiers are 
silently dropped during reverse conversion.



##########
plugins/header_rewrite/condition.cc:
##########
@@ -75,6 +75,27 @@ parse_matcher_op(std::string &arg)
   }
 }
 
+void
+Condition::normalize(std::string &s, size_t start)
+{
+  auto pos = s.find('%', start);
+
+  // Percent-decoding is the only normalization so far, hence the cheap 
bail-out.
+  if (pos == std::string::npos) {
+    return;
+  }
+
+  size_t len     = s.size() - pos;
+  size_t written = 0;
+
+  // Decoding only shrinks, so it's safe in place; the +1 is room for the NUL 
it appends.
+  if (TSStringPercentDecode(s.data() + pos, len, s.data() + pos, len + 1, 
&written) == TS_SUCCESS) {
+    s.resize(pos + written);
+  } else {
+    Dbg(pi_dbg_ctl, "Failed to percent-decode, leaving the value untouched");
+  }

Review Comment:
   Condition::normalize() passes dst_size=len+1 to TSStringPercentDecode(), 
which always writes a NUL terminator at dst[data_written]. With a std::string 
buffer this can write past the string’s current size (UB) unless the string is 
temporarily extended to provide that extra byte. This is especially relevant 
for invalid %-escapes where the decoded length may equal the input length (so 
the NUL would land at pos+len).



-- 
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]

Reply via email to