add BodyFn option to logger for request-body transforms - #47
Merged
Conversation
Add logger.BodyFn(func(body string, truncated bool) string) Option, a transform applied to the request body before logging, e.g. to mask credentials. It runs only when body logging is enabled (WithBody); the transform receives the body capped at MaxBodySize plus a truncated flag and fully owns the logged output, so it can emit a marker for a partial body it can't safely process. The logger still collapses the result to a single line to prevent log injection, and the read stays bounded by MaxBodySize.
Two log-output defects on the body-logging path, surfaced while reviewing the BodyFn change but predating it: - the single-line collapse only neutralized LF; a lone CR (and VT, FF, NEL, U+2028, U+2029) survived, so a crafted body could forge extra log records. Collapse every line-break character before the whitespace-run pass. - the rendered line was passed to Logf as the format string, so a '%' in a body or URL was interpreted as a format verb (e.g. %!s(MISSING)). Pass it as an argument instead. Also add the missing nil-BodyFn default-path test plus CR/Unicode-separator and percent-in-body regression tests.
Two review follow-ups on the logger: - getBody no longer calls bodyFn for an empty request body - there is nothing to transform, and a transform returning a marker for empty input would otherwise log content for bodyless requests. - formatApacheCombined closed the request quote after the URL, emitting "METHOD URL" PROTO" instead of the correct "METHOD URL PROTO". Move the space so the protocol stays inside the quoted request field, and fix the test that had asserted the malformed line.
Coverage Report for CI Build 29537328224Coverage remained the same at 96.21%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
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
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.
adds
logger.BodyFn, a hook to transform the request body before it is logged, e.g. mask credentials, shrink or reformat a noisy body, or summarize it. Masking is the motivating case: a service that logs login requests shouldn't write plaintext passwords to its log.this is dynamic, unlike masking a fixed set of known secret strings supplied ahead of time (
lgr.Secret-style): the transform runs on the live request body, and what to change is the caller's decision, not the logger's. The logger only guarantees the result ends up on a single log line.BodyFn
BodyFn(func(body string, truncated bool) string) Option, following the existingIPfn/UserFn/SubjFnprecedentWithBody) and the body is non-empty; a nilbodyFnkeeps current behaviorMaxBodySize) plus atruncatedflag, and owns the logged output. It decides how to render a truncated body, so the middleware imposes no placeholder and the transform doesn't have to assume JSONMaxBodySize, no full-body bufferingthree logging fixes on the same path, found while building this
\n, so a lone\r(plus VT, FF, NEL, U+2028, U+2029) slipped through and a crafted body could forge extra log records. Now every line-break char is collapsedLogfas the format string, so a%in a body or URL printed as%!s(MISSING). Now it is passed as an argumentformatApacheCombinedemitted"METHOD URL" PROTO"instead of"METHOD URL PROTO". The protocol now stays inside the quoted request fieldREADMEupdated, tests cover the new option and each fix.