[PR #75] [CLOSED] Fix crash on enabling previously invalid zone #118

New issue

Closed

opened 2026-02-28 14:16:27 +03:00 by kerem · 0 comments

kerem commented

2026-02-28 14:16:27 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/abh/geodns/pull/75
Author: @abligh
Created: 8/19/2015
Status: ❌ Closed

Base: master ← Head: fix-crash-on-enabling-previously-invalid-zone

📝 Commits (4)

75f2193 Fix race referencing config
9ea0491 Move last modified time out of zone
c99b922 Fix crash on removal of zonefile with invalid JSON (Issue #69)
63f15b8 Fix issue #74 - crash on reenabling previously invalid zone

📊 Changes

8 files changed (+62 additions, -21 deletions)

View changed files

📝 config.go (+23 -1)
📝 geodns.go (+2 -2)
📝 geoip.go (+3 -2)
📝 stathat.go (+7 -7)
📝 zone.go (+15 -6)
📝 zone_test.go (+4 -1)
📝 zones.go (+6 -2)
📝 zones_test.go (+2 -0)

📄 Description

Fix issue #74

This crash is similar to #69 but with a different cause (and different fix)

To replicate:

Alter dns/example.com so it is invalid (e.g. put an x at the top)
Start geodns
Alter dns/example.com so it is valid (e.g. remove the inserted x)
Wait for geodns to reload example.com
Perform any dig query for that zone e.g. to dig A weight.example.com @....

This produces a panic at:

    z.Metrics.Queries.Mark(1)

on line 34 of zone.go, because z.Metrics is {nil, nil, nil, nil}.

This cause of this problem is that when the zone is first loaded, Metrics is initialised to {nil, nil, nil, nil}. SetupMetrics is never called because the zone is invalid, but the old zone is kept around (presumably because in more complex circumstances it could have useful metrics in which should not be zeroed. When the zone is made loadable, old is non-nil, so the previous Metrics struct is copied, but not initialised, so it remains nil. The fix (coming right up) is to initialise any nil members of the Metrics struct whatever the value of old on entry.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/abh/geodns/pull/75 **Author:** [@abligh](https://github.com/abligh) **Created:** 8/19/2015 **Status:** ❌ Closed **Base:** `master` ← **Head:** `fix-crash-on-enabling-previously-invalid-zone` --- ### 📝 Commits (4) - [`75f2193`](https://github.com/abh/geodns/commit/75f21937fb32dd445ddc8d856c5832fa434dc0ec) Fix race referencing config - [`9ea0491`](https://github.com/abh/geodns/commit/9ea0491eb4fc9e73554508f41ecb09bd20928eee) Move last modified time out of zone - [`c99b922`](https://github.com/abh/geodns/commit/c99b92208cd405218b9522a91d1ea45f5fe27ee8) Fix crash on removal of zonefile with invalid JSON (Issue #69) - [`63f15b8`](https://github.com/abh/geodns/commit/63f15b8bd560e7e976b742c2c18eb391b330a267) Fix issue #74 - crash on reenabling previously invalid zone ### 📊 Changes **8 files changed** (+62 additions, -21 deletions) <details> <summary>View changed files</summary> 📝 `config.go` (+23 -1) 📝 `geodns.go` (+2 -2) 📝 `geoip.go` (+3 -2) 📝 `stathat.go` (+7 -7) 📝 `zone.go` (+15 -6) 📝 `zone_test.go` (+4 -1) 📝 `zones.go` (+6 -2) 📝 `zones_test.go` (+2 -0) </details> ### 📄 Description Fix issue #74 This crash is similar to #69 but with a different cause (and different fix) To replicate: - Alter `dns/example.com` so it is invalid (e.g. put an `x` at the top) - Start `geodns` - Alter `dns/example.com` so it is valid (e.g. remove the inserted `x`) - Wait for `geodns` to reload `example.com` - Perform any `dig` query for that zone e.g. to `dig A weight.example.com @....` This produces a panic at: ``` z.Metrics.Queries.Mark(1) ``` on line 34 of `zone.go`, because z.Metrics is `{nil, nil, nil, nil}`. This cause of this problem is that when the zone is first loaded, `Metrics` is initialised to `{nil, nil, nil, nil}`. `SetupMetrics` is never called because the zone is invalid, but the old zone is kept around (presumably because in more complex circumstances it could have useful metrics in which should not be zeroed. When the zone is made loadable, `old` is non-nil, so the previous `Metrics` `struct` is copied, but not initialised, so it remains `nil`. The fix (coming right up) is to initialise any `nil` members of the `Metrics` struct whatever the value of `old` on entry. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>