header_rewrite: %{CIDR:,N} zeroes the IPv4 mask instead of keeping the default
- 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
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