Conversation
| return crl, shasum[:], err | ||
| } | ||
|
|
||
| func normalizedUrlString(u url.URL) string { |
There was a problem hiding this comment.
Might be nice to have a couple of unit tests for this function.
There was a problem hiding this comment.
Ah yeah, this is actually a very convoluted way to do this and I'm not sure it's correct. The real issue was that we were comparing strings from CRL DP extensions against the normalized URLs that we get from the String() method on url.URL. We can use the same String() function to normalize the CRL DP extension contents.
| defer mi.mutex.Unlock() | ||
| mi.modTime = fi.ModTime() | ||
|
|
||
| // Reset maps to prevent duplicates on reload |
There was a problem hiding this comment.
I'm not understanding this. Is the reload you're referring to when Load() calls LoadFromDisk() once, and then again if the dataset is old enough? Shouldn't InsertIssuer() already handle any duplicates?
There was a problem hiding this comment.
Yes that's the reload I'm referring to. The hash map used in InsertIssuer() maps SPKI hashes to lists of certificates; it doesn't currently deduplicate the list although I guess it could.
There was a problem hiding this comment.
Oh, I see. I feel like InsertIssuer() would be more robust in general if it deduplicated certificates at that point, but this should be fine for now.
jschanck
force-pushed
the
rootprogram-fixup
branch
from
d2b2669 to
101a721
Compare
| defer mi.mutex.Unlock() | ||
| mi.modTime = fi.ModTime() | ||
|
|
||
| // Reset maps to prevent duplicates on reload |
There was a problem hiding this comment.
Oh, I see. I feel like InsertIssuer() would be more robust in general if it deduplicated certificates at that point, but this should be fine for now.
2 participants
A few minor changes after observing a few runs with #379 on staging: