apache / apache/trafficserver

header_rewrite: %{CIDR:,N} zeroes the IPv4 mask instead of keeping the default

Open Beginner friendly
#13,620 0 comments 0 reactions 0 assignees View on GitHub
Bug header_rewrite
Dominant language
C++
Stars
2k
Forks
874
Avg merge
6d 15h
Merged PRs (30d)
46

Description

`ConditionCidr::set_qualifier()` parses the whole qualifier with a single `strtol`, so an empty field and an explicit `0` are indistinguishable. The documented v6-only form therefore masks IPv4 away entirely.

Docs (`doc/admin-guide/plugins/header_rewrite.en.rst`):

```
%{CIDR} Defaults to 24,48 (as above)
%{CIDR:16} IPv4 CIDR mask is 16 bits, IPv6 mask is 48
cond %{CIDR:,8} ="fd00::" #note the IPv6 Mask is in the second position
```

For `%{CIDR:,8}`, `strtol()` consumes no digits, returns 0 with `endp` at the comma, `0 <= 32` passes, and `_v4_cidr` becomes 0 rather than staying at its default of 24. Masking a client address through the `cidr.h` helpers:

```
%{CIDR:,8} (v4=0, today) client 10.2.3.4 -> 0.0.0.0
%{CIDR:24,8} (v4=24, docs) client 10.2.3.4 -> 10.2.3.0
```

`%{CIDR:24,}` has the same problem on the other side (v6 becomes 0 instead of 48), and `%{CIDR:abc}` silently means v4=0 instead of raising a `TSError`.

Suggested fix in `set_qualifier()` — treat "no digits consumed" as "keep the default", and only allow it when the field really was empty:

```cpp
cidr = strtol(q.c_str(), &endp, 10);
if (endp == q.c_str()) {
// No v4 digits: only the empty-field form "%{CIDR:,N}" is valid; keep the default.
ok = (*endp == ',' || *endp == '/' || *endp == ':');
cidr = _v4_cidr;
}
```

plus the same treatment after the separator. That also turns `%{CIDR:abc}` into an error rather than a silent /0.

Noticed while reviewing #13205, which pads the missing field with the documented default (24) — that padding is correct against the docs, but disagrees with what the plugin does today. Related: `tools/hrw4u` validates `cidr()` as `range(1, 32)` / `range(1, 128)` while the plugin accepts 0 in both positions, so an explicit `%{CIDR:0,0}` cannot be expressed in hrw4u at all.

Contributor guide

Open the contributing guide

Research direction

Start at ConditionCidr::set_qualifier() and read the documented forms in doc/admin-guide/plugins/header_rewrite.en.rst, along with the cidr.h helpers. Verify that omitted fields retain defaults, explicit zero remains supported, and invalid input such as %{CIDR:abc} raises a TSError rather than becoming /0.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.