Skip to content

Conversation

zwoop
Copy link
Contributor

@zwoop zwoop commented Oct 1, 2025

This also fixes a small bug in run-plugin.

I think this may be a candidate for v10.1.x, but not 100% sure if all these things are in 10.1.x ?

[Issue discovered by Fina.]

@zwoop zwoop added this to the 10.2.0 milestone Oct 1, 2025
@zwoop zwoop requested review from cmcfarlen and Copilot October 1, 2025 22:14
@zwoop zwoop self-assigned this Oct 1, 2025
@zwoop zwoop added the header_rewrite header_rewrite plugin label Oct 1, 2025
@zwoop zwoop added this to ATS v10.1.x Oct 1, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Fix logic around incomplete rules in header_rewrite when encountering ELIF/ELSE blocks and correct a small argument handling bug in run-plugin initialization.

  • Centralize rule-completion validation with a new helper to enforce constraints on ELIF/ELSE/OPER clauses without operators
  • Adjust run-plugin argv population to iterate over token count rather than argc
  • Minor parser/comment cleanups and a new ConditionGroup::has_conditions() helper

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
plugins/header_rewrite/parser.cc Comment cleanup and small whitespace tweak; no functional change.
plugins/header_rewrite/operators.cc Fix loop bound to use tokens.size() for argv population in run-plugin.
plugins/header_rewrite/header_rewrite.cc Introduce validate_rule_completion and use it during parsing; adjust when rules are validated/transferred.
plugins/header_rewrite/conditions.h Add has_conditions() to expose whether a group has any conditions.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

This also fixes a small bug in run-plugin
@zwoop zwoop merged commit 64d4769 into apache:master Oct 7, 2025
15 checks passed
@zwoop zwoop deleted the HrwElifFixes branch October 7, 2025 16:00
@github-project-automation github-project-automation bot moved this to For v10.1.1 in ATS v10.1.x Oct 7, 2025
@cmcfarlen cmcfarlen removed this from ATS v10.1.x Oct 7, 2025
zwoop added a commit to zwoop/trafficserver that referenced this pull request Oct 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

header_rewrite header_rewrite plugin

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants