logger: close three silent no-ops in RedactJSON and MaskJSON

[enzo-wego]

What

RedactJSON and MaskJSON miss three classes of value a caller has explicitly asked them to remove. Every one of them fails silently: the key path is listed, nothing is redacted, and nothing tells you.

1. Non-string values on a nested key path are skipped. getJSONValue reads GetStringBytes, which is "" for a number, a boolean, null, an object and an array, and the if value != "" guard then skips the Set. So {"customer", "latitude"} does nothing to {"customer":{"latitude":12.971598}}. Only RedactJSON's single-key branch called Set unconditionally, so a top-level number was redacted and the same number one level down was not.

2. Key matching is byte for byte, request binding is not. encoding/json binds struct fields case-insensitively, so {"Customer":{"LATITUDE":1.5}} reaches the handler and is persisted exactly like the lower-case body, but a lower-case entry in the key list does not match it in the raw bytes being logged.

3. A repeated key keeps its earlier copies. {"customer":{"latitude":1.5},"customer":{"latitude":2.5}} parses with both members present. Get returns the first, Set replaces only the first, and MarshalTo re-emits both. encoding/json binds the last, so the copy the handler actually accepted is the one that survives into the log.

Why

A service of ours now takes two device-precision coordinates on a request body. They are PII and they are JSON numbers, so point 1 means they cannot be removed through the key list at all, and that service is carrying a local redaction pass to work around it. Raised in review by Yan Yi: the fastjson write path already handles any value type, so the gap is here and the local pass should go.

The next change on the same endpoint adds a postal address and a tax identification number, both strings, one of them with mixed-case spelling in the API contract. Point 2 covers those.

How

RedactJSON and MaskJSON now share one key-path walker, replaceLeaves.

• Value type is no longer consulted for redaction. Numbers, booleans, null, objects and arrays are redacted at every depth, the way the top-level branch already behaved. Masking reads a number or boolean through its literal text and writes back a string; objects, arrays and null carry no text to mask and are left untouched.
• Every member matching a key is visited, not only the first. Members repeating a key collapse into one, masked from the value encoding/json would have bound, the last of them. A key present once keeps its position in the object.
• New CaseInsensitiveKeys() option, off by default. Both functions gained a variadic ...Option, so every existing call site compiles and behaves exactly as before. Pass it when the input is a body bound with encoding/json.
• [] at the last position now fans out for RedactJSON too. It was a no-op there while MaskJSON handled it.

One fix outside those three, on the same lines: the replacement was built as fastjson.MustParse("+ value +"), which panics on any value carrying a quote or a backslash. Masking keeps FirstCharsToShow and LastCharsToShow verbatim, so the value "Joker" masked at 2/2 produced ""J*r"" and panicked inside the logging middleware. Replacements go through json.Marshal now.

Tests

New json_keys_test.go covering each of the three gaps plus the escaping. All existing tests and golden files pass unchanged, with no edit to any of them.

Benchmarks, median of 6 runs at -benchtime=300ms:

BenchmarkMaskJSON      9144 -> 9826 ns/op (+7.5%)    94 -> 92 allocs/op
BenchmarkRedactJSON   10278 -> 10508 ns/op (+2.2%)  101 -> 96 allocs/op

Compatibility

Source-compatible. The only behaviour change for a caller that does not pass the new option is that key paths which used to silently do nothing now do what the caller asked, which can only redact more, never less.

@yanyi-wego, please review. I will tag logger/v0.3.10 after merge and drop the local pass in payments on top of it.

[yanyi-wego]

👍

_{🤖 AI-assisted review via Claude Code}