The response was built by concatenating the request path into a JSON
string. The path is percent-decoded, so it can contain quotes and
backslashes, which produced malformed JSON and let a caller inject
arbitrary keys into the object.
Requests to the token and pushed authorization endpoints carry the
client secret or a signed client assertion. Following a redirect would
forward those credentials to a host the provider never advertised.
The metadata document was fetched with http.Get, which has no deadline,
so a hung connection blocked startup indefinitely. The response status
was never checked either, leaving an error page to surface as a JSON
decoding error.
The fetch now runs with a 10s deadline derived from the context passed
in from main, goes through the shared transport so it is traced and
pooled, and rejects any non-200 response.
Registration errors were discarded, leaving the collector silently
absent from the metrics endpoint while the vectors still recorded.
Duplicate registrations remain ignored; they are expected when several
instances are constructed in the same process.
Satisfies errcheck. Errors that carry no actionable information are
ignored explicitly: writes to an already-committed response, and closing
a fully read response body or file.
Test fixture setup asserts with require.NoError instead, since a failure
there means the fixture itself is broken.
Introduce SecondsNotApplicable for *_in_seconds fields whose feature is
disabled (no inactivity timeout, auto-refresh off), replacing scattered
magic -1 values and the post-construction mutation in Verbose(). JSON
output is unchanged.
Consolidate the refresh schedule into a single NextRefresh function (leeway +
inactivity half-life, floored at the cooldown). ShouldRefresh now defers to it,
so the explicit cooldown gate is redundant and removed. Add doc comments naming
the mechanisms. No behavior change.
Refreshes are activity-based and synchronous-on-expiry, so the proactive
leeway only hides refresh latency and provides retry headroom; it is not
what prevents serving expired tokens. Shrink it from 5m to 30s to align
with identity provider guidance against refreshing long before expiry,
while keeping decent end-user UX.
The previous if/else only checked for WarnLevel, causing any other
level (e.g. InfoLevel in 24fb9b1) to fall through to Errorf. Use Logf to
dispatch on the actual level passed by callers.
Move metrics and probe listeners from main.go into server.go and
manage all servers with errgroup. This replaces log.Fatalf goroutines
with proper error propagation and ties all server lifetimes together
so a failure in any listener triggers graceful shutdown of the others.
Without IdleTimeout, clients holding keep-alive connections open indefinitely
caused server-side goroutines (and their ~16KB of buffers) to accumulate
linearly until OOM.
Trong Huu Nguyen
@x10an14-nav
Kyrre HavikandGitHub
