[12/15] Log every request through an access-log interceptor - #165
Draft
UnknownJoe796 wants to merge 6 commits into
Draft
[12/15] Log every request through an access-log interceptor#165UnknownJoe796 wants to merge 6 commits into
UnknownJoe796 wants to merge 6 commits into
Conversation
KnownDeviceProofEndpoints previously called Uuid.parse() on the client-supplied identifier segment with no error handling; a malformed UUID threw an unhandled IllegalArgumentException, which the framework surfaces as a 500 Internal Server Error rather than a 400 Bad Request. Wrap the parse in a try/catch and surface a BadRequestException with the parse failure reason instead. Reconstructed from stale origin/fixes commit 01b5f4e; clean apply, file was untouched on version-5.3 since the fork point.
Covers the previous commit: a non-Uuid id segment must produce a BadRequestException (400) rather than an unhandled IllegalArgumentException.
Return 400 instead of 500 for malformed known-device identifiers
UnknownJoe796
commented
Jul 29, 2026
| // once, and in production every setting is pre-resolved single-threaded during ready(). An immutable Map | ||
| // (not a ConcurrentHashMap) is used so a null result is stored naturally, with no sentinel. | ||
| @Volatile | ||
| private var goal: Map<ServerSetting<*, *>, Any?> = emptyMap() |
Contributor
Author
There was a problem hiding this comment.
WTF? Why is this getting reverted? Why are PRs stacking changes on each other?
UnknownJoe796
force-pushed
the
v53/docs-migration-v52
branch
from
July 29, 2026 22:54
ec8b005 to
d2fd0af
Compare
UnknownJoe796
force-pushed
the
v53/access-logging
branch
from
July 29, 2026 22:54
62b738b to
5c379a1
Compare
Concurrent first-touch of a setting could resolve it more than once. Transformed results are now published as an immutable snapshot: reads are lock-free (once a setting is resolved, an immutable Map makes it visible for good via a @volatile field), and writes replace the whole map under a single lock (copy-on-write) -- cheap, since each setting resolves at most once, and in production every setting is pre-resolved single-threaded during ready(). A single reentrant lock guards transformation because a setting's getter may resolve its own dependencies by calling get() again on the same thread; per-setting locks could deadlock on mutually-dependent settings instead. (cherry picked from commit ed1d009)
Replaces the ad-hoc logging scattered through request handling with a single AccessLogInterceptor, and refines the request pipeline, session manager and settings around it. Squashes two commits (both titled "One more step"): the first added RequestLogDescriber.kt, the second deleted it in favour of AccessLogInterceptor. Split across two PRs that would be pure churn, so they're combined. The masquerade *denial* log is kept, in the simpler form this commit introduces. The separate AUDIT-prefixed grant/denial logging from an earlier commit is deliberately gone: it was superseded by this interceptor, and shipping it would have meant reviewing an approach already abandoned. Its ServerDefinition.kt/SecurityHeadersInterceptor.kt and ServerSettings.kt changes, and the engine reliability tests' 408->503 follow-up, already shipped earlier in this de-linearized chain -- kept here only to preserve authorship/history for the rest of the change. Based directly on security-headers + engine-conformance + settings-thread- safety, since AccessLogInterceptor's own tests reference SecurityHeadersInterceptor and this commit further edits the shared conformance suite and implementationHelpers.kt.
UnknownJoe796
force-pushed
the
v53/docs-migration-v52
branch
from
July 30, 2026 02:20
d2fd0af to
faf3fc9
Compare
UnknownJoe796
force-pushed
the
v53/access-logging
branch
from
July 30, 2026 02:20
5c379a1 to
241a6d1
Compare
UnknownJoe796
changed the base branch from
v53/docs-migration-v52
to
v53/engine-conformance
July 30, 2026 02:24
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces logging scattered through request handling with a single
AccessLogInterceptor.Read this before merging. Two things here are not a plain replay of history:
RequestLogDescriber.kt; the second deleted it again in favour ofAccessLogInterceptor. Split across two PRs that is pure churn -- one PRproposing a file the next one removes -- so they are combined.
Authentication.kthad a conflict resolved by hand. The masquerade deniallog is kept, in the simpler form this commit introduces
(
"$auth denied masquerade as $masquerade"). The separateAUDIT-prefixedgrant/denial logging from an earlier commit is deliberately gone: it was
superseded by this interceptor, and shipping it would have meant reviewing an
approach already abandoned.
This PR also refines the request pipeline, session manager and settings around
the interceptor, which is why it touches more than the new file.