Redact HTTP headers on LoggingFeature

[nunomsantos]

As talked on #5014, this allows configuring LoggingFeature with a list of HTTP headers to have their value redacted when printing to logs. By default, the Authorization header is redacted.

Example of a logged request/response with the Authorization header redacted (default behavior):

1 * Sending client request on thread main
1 > GET http://localhost:9998/echo-headers
1 > Authorization: [redacted]

1 * Client response received on thread main
1 < 200
1 < accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2
1 < authorization: [redacted]
1 < connection: keep-alive
1 < Content-Length: 32
1 < Content-Type: text/plain
1 < host: localhost:9998
1 < user-agent: Jersey/2.36-SNAPSHOT (HttpUrlConnection 1.8.0_92)
This entity must (not) be logged

And an example with two headers configured to be redacted (overriding the default behavior of redacting the Authorization header):

1 * Sending client request on thread main
1 > GET http://localhost:9998/echo-headers
1 > Authorization: basic username:password
1 > X-Also-Redact-This-Header: [redacted]
1 > X-Redact-This-Header: [redacted]

1 * Client response received on thread main
1 < 200
1 < accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2
1 < authorization: basic username:password
1 < connection: keep-alive
1 < Content-Length: 32
1 < Content-Type: text/plain
1 < host: localhost:9998
1 < user-agent: Jersey/2.36-SNAPSHOT (HttpUrlConnection 1.8.0_92)
1 < x-also-redact-this-header: [redacted]
1 < x-redact-this-header: [redacted]
This entity must (not) be logged

[nunomsantos]

Hi, @senivam. Just noticed that I'm lowering the case on test assertions and I think that's not needed with the provided matcher. Please give me some time to check that.

[nunomsantos]

Hi, @senivam. Just noticed that I'm lowering the case on test assertions and I think that's not needed with the provided matcher. Please give me some time to check that.

Confirmed. Change pushed.

[senivam]

LGTM

[jansupol]

Add The headers are semicolon-separated.

[nunomsantos]

Will do. Please give me some time.

[nunomsantos]

Pushed changes to the Javadocs on the 3 variations of the property.

I checked if we could set a property multiple times and then get a collection, but it looked like that's not possible.
Can you confirm that? Please let me know if you think of a better way to do this.

[nunomsantos]

Just crossed my mind that maybe we should do a null check here. Not sure if it's possible for a null header ever to reach this point but could do it just to be on the safe side.
The problem would not be in the constructor usage, but when test(header) is called.

Thoughts, @jansupol @senivam?

[senivam]

I think it's OK. This code final String header = headerEntry.getKey(); is NPE safe and this code

this.headersToRedact = headersToRedact.stream()
                   .filter(Objects::nonNull)
                   .filter(Predicates.not(String::isEmpty))
                   .map(RedactHeaderPredicate::normalize)
                   .collect(Collectors.toSet());

is NPE safe. Nothing else calls that predicate.