header_rewrite: %{CIDR:,N} zeroes the IPv4 mask instead of keeping the default
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 78/100
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.
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:
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.
- Dominant language
- C++
- Stars
- 2k
- Forks
- 877
- Avg merge
- 4d 18h
- Merged PRs (30d)
- 52
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from apache/trafficserver
-
hrw4u
Difficulty 1/5 Under an hour Newbie friendliness 88/100
apache/trafficserver#13701 · 1 assignee ·
-
hrw4u
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
apache/trafficserver#13619 ·
-
Bug HTTP Support
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
apache/trafficserver#13118 ·
-
Cache Tests
Difficulty 4/5 3-5 days Newbie friendliness 45/100
apache/trafficserver#13698 · 1 assignee ·
-
Bug Config Reload Configuration HTTP/2
apache/trafficserver#13694 · 1 assignee ·
All issues in apache/trafficserver
Similar issues
-
Difficulty 1/5 Under an hour Newbie friendliness 90/100
AXERA-TECH/ax-llm#77 ·
-
Difficulty 1/5 Under an hour Newbie friendliness 90/100
games-on-whales/wolf#509 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
-
bug-unconfirmed
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
NVIDIA/cuda-samples#453 ·