From 4173042e343c4bae8aa3bd26a6fce2f983bd2683 Mon Sep 17 00:00:00 2001 From: Robert Thomas Date: Fri, 4 Sep 2026 01:41:07 +0100 Subject: [PATCH 1/4] fix: use cached bootstrap registry files instead of downloading Lookup() only parsed a cached Service Registry file into memory when the cache reported ShouldReload, a state only DiskCache returns. Two bugs followed from that: A caller-populated cache was ignored. MemoryCache reports Good straight after Save(), so a registry embedded with //go:embed left registries[type] nil and triggered a download that also overwrote the embedded copy. An expired file was never refreshed. Once parsed, registries[type] was non-nil and Expired was ignored, so a long lived Client served the same file forever, contrary to the documented 24 hour refresh. Load from the cache whenever nothing is parsed yet and the cached copy is usable, force a download once it expires, and fall back to the expired copy if that download fails. Registry files change rarely, so a stale copy beats a failed lookup, and this makes the "returned even if its State() is Expired" contract of both caches reachable. Fixes #54 --- bootstrap/client.go | 43 ++++++++++++++--- bootstrap/client_test.go | 101 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 6 deletions(-) diff --git a/bootstrap/client.go b/bootstrap/client.go index 7fd7fa2..ffda363 100644 --- a/bootstrap/client.go +++ b/bootstrap/client.go @@ -81,6 +81,19 @@ // // dsr2 := b.DNS() // Loads dns.json from disk cache. // +// You can pre-populate the cache with your own copy of a Service Registry file +// (e.g. one embedded with //go:embed), to avoid downloading it: +// +// c := cache.NewMemoryCache() +// c.Save(bootstrap.DNS.Filename(), embeddedDNSJSON) +// +// b := &bootstrap.Client{Cache: c} // Lookup() uses the cached file. +// +// The file is still refreshed once it expires (see SetTimeout), falling back to +// the cached copy if the download fails. Note this only works with the default +// BaseURL: files cached for a custom bootstrap service are stored under a +// different, unexported filename. +// // This package also implements the experimental Service Provider registry. Due // to the experimental nature, no Service Registry file exists on data.iana.org // yet, additionally the filename isn't known. The current filename used is @@ -250,8 +263,10 @@ func (c *Client) download(ctx context.Context, registry RegistryType) ([]byte, R return json, s, nil } +// freshenFromCache attempts to refresh the specified registry from the cache +// if it is outdated or missing in memory. func (c *Client) freshenFromCache(registry RegistryType) { - if c.Cache.State(c.filenameFor(registry)) == cache.ShouldReload { + if c.shouldLoadFromCache(registry, c.Cache.State(c.filenameFor(registry))) { // Best-effort refresh; on failure the existing in-memory registry is kept. _ = c.reloadFromCache(registry) } @@ -274,6 +289,13 @@ func (c *Client) reloadFromCache(registry RegistryType) error { return nil } +// shouldLoadFromCache reports whether the cached registry file should be parsed +// into memory: either the cache holds a newer copy, or nothing is in memory yet +// and the cache holds an unexpired one (e.g., a caller pre-populated the cache). +func (c *Client) shouldLoadFromCache(registry RegistryType, state cache.FileState) bool { + return state == cache.ShouldReload || (state == cache.Good && c.registries[registry] == nil) +} + func newRegistry(registry RegistryType, json []byte) (Registry, error) { var s Registry var err error @@ -312,8 +334,10 @@ func (c *Client) Lookup(question *Question) (*Answer, error) { state := c.Cache.State(c.filenameFor(registry)) c.Verbose(fmt.Sprintf(" bootstrap: Cache state: %s: %s", c.filenameFor(registry), state)) - var forceDownload bool - if state == cache.ShouldReload { + // An expired file is refreshed even if it's already parsed into memory. + forceDownload := state == cache.Expired + + if c.shouldLoadFromCache(registry, state) { if err := c.reloadFromCache(registry); err != nil { forceDownload = true @@ -324,9 +348,16 @@ func (c *Client) Lookup(question *Question) (*Answer, error) { if c.registries[registry] == nil || forceDownload { c.Verbose(fmt.Sprintf(" bootstrap: Downloading %s", registry.Filename())) - err := c.DownloadWithContext(question.Context(), registry) - if err != nil { - return nil, err + if err := c.DownloadWithContext(question.Context(), registry); err != nil { + // Service Registry files change rarely, so an expired copy still + // answers most queries. Prefer one to failing the lookup. + if c.registries[registry] == nil { + if cacheErr := c.reloadFromCache(registry); cacheErr != nil { + return nil, err + } + } + + c.Verbose(fmt.Sprintf(" bootstrap: Download failed (%s), using expired Service Registry file", err)) } } else { c.Verbose(" bootstrap: Using cached Service Registry file") diff --git a/bootstrap/client_test.go b/bootstrap/client_test.go index d86a6bd..4e2a787 100644 --- a/bootstrap/client_test.go +++ b/bootstrap/client_test.go @@ -6,7 +6,10 @@ package bootstrap import ( "testing" + "time" + "github.com/jarcoal/httpmock" + "github.com/openrdap/rdap/bootstrap/cache" "github.com/openrdap/rdap/test" ) @@ -105,6 +108,104 @@ func TestLookups(t *testing.T) { } } +// A pre-populated cache should be used instead of downloading. The HTTP +// responders here all return 404, so a download attempt fails the test. +func TestLookupUsesPrePopulatedCache(t *testing.T) { + test.Start(test.BootstrapHTTPError) + defer test.Finish() + + memCache := cache.NewMemoryCache() + if err := memCache.Save("dns.json", test.LoadFile("bootstrap/dns.json")); err != nil { + t.Fatalf("Save() error: %s", err) + } + + c := &Client{Cache: memCache} + + answer, err := c.Lookup(&Question{RegistryType: DNS, Query: "example.br"}) + if err != nil { + t.Fatalf("Lookup() error: %s", err) + } + + if len(answer.URLs) != 1 || answer.URLs[0].String() != "https://rdap.registro.br/" { + t.Errorf("Lookup() got %v, want [https://rdap.registro.br/]", answer.URLs) + } + + if c.DNS() == nil { + t.Error("DNS() returned nil after loading the registry from the cache") + } +} + +// An expired cached file is used when the refresh download fails, rather than +// failing the lookup. +func TestLookupFallsBackToExpiredCache(t *testing.T) { + test.Start(test.BootstrapHTTPError) + defer test.Finish() + + memCache := cache.NewMemoryCache() + if err := memCache.Save("dns.json", test.LoadFile("bootstrap/dns.json")); err != nil { + t.Fatalf("Save() error: %s", err) + } + + memCache.SetTimeout(-1 * time.Second) + + if got := memCache.State("dns.json"); got != cache.Expired { + t.Fatalf("cache state = %v, want Expired", got) + } + + c := &Client{Cache: memCache} + + answer, err := c.Lookup(&Question{RegistryType: DNS, Query: "example.br"}) + if err != nil { + t.Fatalf("Lookup() error: %s", err) + } + + if len(answer.URLs) != 1 || answer.URLs[0].String() != "https://rdap.registro.br/" { + t.Errorf("Lookup() got %v, want [https://rdap.registro.br/]", answer.URLs) + } +} + +// An expired file is re-downloaded even though it's already parsed into memory. +func TestLookupRefreshesExpiredCache(t *testing.T) { + test.Start(test.Bootstrap) + defer test.Finish() + + c := &Client{} + + if _, err := c.Lookup(&Question{RegistryType: DNS, Query: "example.br"}); err != nil { + t.Fatalf("Lookup() error: %s", err) + } + + downloads := httpmock.GetTotalCallCount() + + c.Cache.SetTimeout(-1 * time.Second) + + if _, err := c.Lookup(&Question{RegistryType: DNS, Query: "example.br"}); err != nil { + t.Fatalf("Lookup() error: %s", err) + } + + if got := httpmock.GetTotalCallCount() - downloads; got != 1 { + t.Errorf("expired registry triggered %d downloads, want 1", got) + } +} + +// An unparseable cached file plus a failed download reports the download error, +// rather than panicking on the unpopulated registry. +func TestLookupWithCorruptCacheAndDownloadError(t *testing.T) { + test.Start(test.BootstrapHTTPError) + defer test.Finish() + + memCache := cache.NewMemoryCache() + if err := memCache.Save("dns.json", []byte("{{{ not json")); err != nil { + t.Fatalf("Save() error: %s", err) + } + + c := &Client{Cache: memCache} + + if _, err := c.Lookup(&Question{RegistryType: DNS, Query: "example.br"}); err == nil { + t.Error("Lookup() unexpectedly succeeded") + } +} + func TestLookupWithDownloadError(t *testing.T) { test.Start(test.BootstrapHTTPError) defer test.Finish() From 483fd307352bfb10dc7ca6a83a1ebe9e83864fd3 Mon Sep 17 00:00:00 2001 From: Robert Thomas Date: Fri, 4 Sep 2026 01:41:07 +0100 Subject: [PATCH 2/4] chore: bump go directive and update dependencies --- go.mod | 6 +++--- go.sum | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/go.mod b/go.mod index 6273533..b5e06ba 100644 --- a/go.mod +++ b/go.mod @@ -1,13 +1,13 @@ module github.com/openrdap/rdap -go 1.25.0 +go 1.26.0 require ( github.com/alecthomas/kingpin/v2 v2.4.0 github.com/davecgh/go-spew v1.1.1 - github.com/jarcoal/httpmock v1.4.1 + github.com/jarcoal/httpmock v1.4.2 github.com/mitchellh/go-homedir v1.1.0 - golang.org/x/crypto v0.54.0 + golang.org/x/crypto v0.56.0 ) require ( diff --git a/go.sum b/go.sum index 1f759a8..f8a9bd4 100644 --- a/go.sum +++ b/go.sum @@ -5,8 +5,8 @@ github.com/alecthomas/units v0.0.0-20240927000941-0f3dac36c52b/go.mod h1:fvzegU4 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/jarcoal/httpmock v1.4.1 h1:0Ju+VCFuARfFlhVXFc2HxlcQkfB+Xq12/EotHko+x2A= -github.com/jarcoal/httpmock v1.4.1/go.mod h1:ftW1xULwo+j0R0JJkJIIi7UKigZUXCLLanykgjwBXL0= +github.com/jarcoal/httpmock v1.4.2 h1:dKwiP/9zITCPfBLsDn3kchbSOu16JrnxtVEmL0fPRcI= +github.com/jarcoal/httpmock v1.4.2/go.mod h1:ftW1xULwo+j0R0JJkJIIi7UKigZUXCLLanykgjwBXL0= github.com/maxatome/go-testdeep v1.14.0 h1:rRlLv1+kI8eOI3OaBXZwb3O7xY3exRzdW5QyX48g9wI= github.com/maxatome/go-testdeep v1.14.0/go.mod h1:lPZc/HAcJMP92l7yI6TRz1aZN5URwUBUAfUNvrclaNM= github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= @@ -24,8 +24,8 @@ github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsT github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/xhit/go-str2duration/v2 v2.1.0 h1:lxklc02Drh6ynqX+DdPyp5pCKLUQpRT8bp8Ydu2Bstc= github.com/xhit/go-str2duration/v2 v2.1.0/go.mod h1:ohY8p+0f07DiV6Em5LKB0s2YpLtXVyJfNt1+BlmyAsU= -golang.org/x/crypto v0.54.0 h1:YLIA59K4fiNzHzjnZt2tUJQjQtUWfWbeHBqKtk3eScw= -golang.org/x/crypto v0.54.0/go.mod h1:KWL8ny2AZdGR2cWmzeHrp2azQPGogOv+HeQaVEXC2dk= +golang.org/x/crypto v0.56.0 h1:GUh5Ii4J5jtcseSMiRqr1jXCNHoxjeV9Fmekc2oLy6Y= +golang.org/x/crypto v0.56.0/go.mod h1:OMW5y6CY9l38uPLmxU6l6pwcXp1obtLo3e6gT7gQR2I= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= From f73230a6132eeeb0496930cb85c65bb3051b98cf Mon Sep 17 00:00:00 2001 From: Robert Thomas Date: Fri, 4 Sep 2026 01:43:53 +0100 Subject: [PATCH 3/4] ci: test against Go 1.26 only, matching the go.mod floor --- .github/workflows/tests.yml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 2c7ee3f..4ff6ebb 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -20,10 +20,9 @@ jobs: strategy: fail-fast: false matrix: - # 1.25.x is the floor declared in go.mod; 1.26.x is the latest release. - # Testing both ensures the declared minimum stays buildable while the - # latest toolchain is exercised. - go-version: ['1.25.x', '1.26.x'] + # 1.26.x is both the floor declared in go.mod and the latest release. + # Add older versions here if go.mod ever lowers its floor. + go-version: ['1.26.x'] steps: - name: Checkout From 334576e869a3cc7cc0993f07022428f1778ed467 Mon Sep 17 00:00:00 2001 From: Robert Thomas Date: Fri, 4 Sep 2026 01:54:58 +0100 Subject: [PATCH 4/4] test: assert download counts in bootstrap cache tests Both tests only checked the answer, so they passed for the wrong reasons. TestLookupUsesPrePopulatedCache would stay green if Lookup downloaded anyway, because the 404 hits the expired-copy fallback and keeps the already-loaded registry. TestLookupFallsBackToExpiredCache would stay green if the refresh attempt were skipped entirely. Assert a httpmock call delta of 0 and 1 respectively. --- bootstrap/client_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/bootstrap/client_test.go b/bootstrap/client_test.go index 4e2a787..46d17ce 100644 --- a/bootstrap/client_test.go +++ b/bootstrap/client_test.go @@ -121,11 +121,17 @@ func TestLookupUsesPrePopulatedCache(t *testing.T) { c := &Client{Cache: memCache} + downloads := httpmock.GetTotalCallCount() + answer, err := c.Lookup(&Question{RegistryType: DNS, Query: "example.br"}) if err != nil { t.Fatalf("Lookup() error: %s", err) } + if got := httpmock.GetTotalCallCount() - downloads; got != 0 { + t.Errorf("pre-populated cache triggered %d downloads, want 0", got) + } + if len(answer.URLs) != 1 || answer.URLs[0].String() != "https://rdap.registro.br/" { t.Errorf("Lookup() got %v, want [https://rdap.registro.br/]", answer.URLs) } @@ -154,11 +160,17 @@ func TestLookupFallsBackToExpiredCache(t *testing.T) { c := &Client{Cache: memCache} + downloads := httpmock.GetTotalCallCount() + answer, err := c.Lookup(&Question{RegistryType: DNS, Query: "example.br"}) if err != nil { t.Fatalf("Lookup() error: %s", err) } + if got := httpmock.GetTotalCallCount() - downloads; got != 1 { + t.Errorf("expired cache triggered %d refresh attempts, want 1", got) + } + if len(answer.URLs) != 1 || answer.URLs[0].String() != "https://rdap.registro.br/" { t.Errorf("Lookup() got %v, want [https://rdap.registro.br/]", answer.URLs) }