Split from #195, which covers the missing rebuild trigger. This one is independent: it would remain even if a rebuild were triggered correctly.
The defect
RpzParseFuncFactory (sources.go:388) only ever writes:
s.Names[name] = tapir.TapirName{Name: name, Action: action}It never deletes and never clears. s.Names is reset only in the startup paths (sources.go:295, 307, 356).
The parse function is invoked for every RR of every transfer, including refreshes — FetchFromUpstream carries RRParseFunc onto the new ZoneData (tapir/zone_utils.go:88), and it is called per RR at tapir/dns_utils.go:145. So a re-transfer accumulates into the existing map.
A name removed from an upstream RPZ therefore stays in pop's list indefinitely, and stays blocked.
Measured
Upstream feed carrying two denylisted names; one removed; five refresh cycles; then a forced rebuild via /debug gen-output:
BEFORE removal: 2 denylisted names
AFTER removal + forced rebuild: 2 denylisted names; doomed.example still present = true
The forced rebuild is significant: it rules out the missing trigger in #195 as the cause. The name is gone upstream and still in pop's list.
Interaction with #195
Fixing #195 alone would give propagation of additions but never removals. That is worse than the current behaviour, because additions arriving makes the source look like it is tracking its upstream.
Suggested fix
Accumulate each transfer into a per-transfer map, then diff it against s.Names and apply the difference — added entries inserted, removed entries deleted — rather than replacing s.Names wholesale.
Applying a delta rather than replacing matters for what comes next: with inbound IXFR the delta arrives directly from the wire, carrying both adds and deletes, and can be applied to s.Names by the same code. A wholesale replace would not generalise to that.
The same diff is what #195 needs in order to publish incrementally.
Reproducer
Available in the test rig; not yet committed as a permanent test.
Split from #195, which covers the missing rebuild trigger. This one is independent: it would remain even if a rebuild were triggered correctly.
The defect
RpzParseFuncFactory(sources.go:388) only ever writes:It never deletes and never clears.
s.Namesis reset only in the startup paths (sources.go:295,307,356).The parse function is invoked for every RR of every transfer, including refreshes —
FetchFromUpstreamcarriesRRParseFunconto the new ZoneData (tapir/zone_utils.go:88), and it is called per RR attapir/dns_utils.go:145. So a re-transfer accumulates into the existing map.A name removed from an upstream RPZ therefore stays in pop's list indefinitely, and stays blocked.
Measured
Upstream feed carrying two denylisted names; one removed; five refresh cycles; then a forced rebuild via
/debug gen-output:The forced rebuild is significant: it rules out the missing trigger in #195 as the cause. The name is gone upstream and still in pop's list.
Interaction with #195
Fixing #195 alone would give propagation of additions but never removals. That is worse than the current behaviour, because additions arriving makes the source look like it is tracking its upstream.
Suggested fix
Accumulate each transfer into a per-transfer map, then diff it against
s.Namesand apply the difference — added entries inserted, removed entries deleted — rather than replacings.Nameswholesale.Applying a delta rather than replacing matters for what comes next: with inbound IXFR the delta arrives directly from the wire, carrying both adds and deletes, and can be applied to
s.Namesby the same code. A wholesale replace would not generalise to that.The same diff is what #195 needs in order to publish incrementally.
Reproducer
Available in the test rig; not yet committed as a permanent test.