Skip to content

Commit

Permalink
Fix advanced cache initialization in README (#198)
Browse files Browse the repository at this point in the history
* Fix advanced cache initialization in README

As per the documentation of GetConfigForCert:
> The returned Config MUST be associated with the same Cache as the caller.
A valid Config cannot be constructed with &certmagic.Config{} as the certCache field is unexported.
The only way to construct a Config with a non-nil Cache is to use either NewDefault or New.

* Make it an error for GetConfigForCert to return Config w/ nil cache

This prevents an invalid Config from slipping through and causing a hard to
debug nil pointer dereference at some later point.
  • Loading branch information
s111 committed Jun 5, 2023
1 parent 8728b18 commit d37847a
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 10 deletions.
17 changes: 9 additions & 8 deletions README.md
Expand Up @@ -238,16 +238,17 @@ if err != nil {
For more control (particularly, if you need a different way of managing each certificate), you'll make and use a `Cache` and a `Config` like so:

```go
cache := certmagic.NewCache(certmagic.CacheOptions{
// First make a pointer to a Cache as we need to reference the same Cache in
// GetConfigForCert below.
var cache *certmagic.Cache
cache = certmagic.NewCache(certmagic.CacheOptions{
GetConfigForCert: func(cert certmagic.Certificate) (*certmagic.Config, error) {
// do whatever you need to do to get the right
// configuration for this certificate; keep in
// mind that this config value is used as a
// template, and will be completed with any
// defaults that are set in the Default config
return &certmagic.Config{
// Here we use New to get a valid Config associated with the same cache.
// The provided Config is used as a template and will be completed with
// any defaults that are set in the Default config.
return certmagic.New(cache, &certmagic.config{
// ...
}, nil
}), nil
},
...
})
Expand Down
9 changes: 7 additions & 2 deletions cache.go
Expand Up @@ -145,7 +145,8 @@ type CacheOptions struct {
// used for managing a certificate, or for accessing
// that certificate's asset storage (e.g. for
// OCSP staples, etc). The returned Config MUST
// be associated with the same Cache as the caller.
// be associated with the same Cache as the caller,
// use New to obtain a valid Config.
//
// The reason this is a callback function, dynamically
// returning a Config (instead of attaching a static
Expand Down Expand Up @@ -342,7 +343,11 @@ func (certCache *Cache) getConfig(cert Certificate) (*Config, error) {
if err != nil {
return nil, err
}
if cfg.certCache != nil && cfg.certCache != certCache {
if cfg.certCache == nil {
return nil, fmt.Errorf("config returned for certificate %v has nil cache; expected %p (this one)",
cert.Names, certCache)
}
if cfg.certCache != certCache {
return nil, fmt.Errorf("config returned for certificate %v is not nil and points to different cache; got %p, expected %p (this one)",
cert.Names, cfg.certCache, certCache)
}
Expand Down

0 comments on commit d37847a

Please sign in to comment.