Skip to content

Add default set of headers to be white listed for logging #5893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
samvaity merged 3 commits into from
Oct 17, 2019
Merged

Add default set of headers to be white listed for logging #5893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
47bccc0
add default headers to whitelist
samvaity Oct 16, 2019
7df7893
review comments
samvaity Oct 17, 2019
dc90cb6
add case insensitive
samvaity Oct 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 35 additions & 4 deletions sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLogOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@

package com.azure.core.http.policy;

import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;

Expand All @@ -14,11 +16,35 @@ public class HttpLogOptions {
private HttpLogDetailLevel logLevel;
private Set<String> allowedHeaderNames;
private Set<String> allowedQueryParamNames;
private static final List<String> DEFAULT_HEADERS_WHITELIST = Arrays.asList(
"x-ms-client-request-id",
"x-ms-return-client-request-id",
"traceparent",
"Accept",
"Cache-Control",
"Connection",
"Content-Length",
"Content-Type",
"Date",
"ETag",
"Expires",
"If-Match",
"If-Modified-Since",
"If-None-Match",
"If-Unmodified-Since",
"Last-Modified",
"Pragma",
"Request-Id",
"Retry-After",
"Server",
"Transfer-Encoding",
"User-Agent"
);

public HttpLogOptions() {
logLevel = HttpLogDetailLevel.NONE;
allowedHeaderNames = new HashSet<>();
allowedQueryParamNames = new HashSet<>();
allowedQueryParamNames = new HashSet<>(DEFAULT_HEADERS_WHITELIST);
}

/**
Expand Down Expand Up @@ -54,18 +80,23 @@ public Set<String> getAllowedHeaderNames() {

/**
* Sets the given whitelisted headers that should be logged.
* <p>
* This method sets the provided header names to be the whitelisted header names which will be logged for all http
* requests and responses, overwriting any previously configured headers, including the default set.
* Additionally, user can use {@link HttpLogOptions#addAllowedHeaderName(String)}
* or {@link HttpLogOptions#getAllowedHeaderNames()} to add or remove more headers names to the existing set of
* allowed header names.
*
* @param allowedHeaderNames The list of whitelisted header names from the user.
* @return The updated HttpLogOptions object.
* @throws NullPointerException If {@code allowedHeaderNames} is {@code null}.
*/
public HttpLogOptions setAllowedHeaderNames(final Set<String> allowedHeaderNames) {
this.allowedHeaderNames = allowedHeaderNames;
this.allowedHeaderNames = allowedHeaderNames == null ? new HashSet<>() : allowedHeaderNames;
return this;
}

/**
* Sets the given whitelisted header that should be logged.
* Sets the given whitelisted header to the default header set that should be logged.
*
* @param allowedHeaderName The whitelisted header name from the user.
* @return The updated HttpLogOptions object.
Expand Down
6 changes: 3 additions & 3 deletions sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLoggingPolicy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,11 @@ private Mono<Void> logRequest(final ClientLogger logger, final HttpRequest reque

private void formatAllowableHeaders(Set<String> allowedHeaderNames, HttpHeaders requestResponseHeaders,
ClientLogger logger) {
if (allowedHeaderNames != null && !allowedHeaderNames.isEmpty()) {
if (!allowedHeaderNames.isEmpty()) {
StringBuilder sb = new StringBuilder();
for (HttpHeader header : requestResponseHeaders) {
sb.append(header.getName()).append(":");

@JonathanGiles JonathanGiles Oct 17, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be case insensitive

@samvaity samvaity Oct 17, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated here - https://github.com/Azure/azure-sdk-for-java/pull/5893/files#diff-1bf1978289f15f56cf8db11581def1a9R129

if (allowedHeaderNames.contains(header.getName())) {
if (allowedHeaderNames.stream().anyMatch(header.getName()::equalsIgnoreCase)) {
sb.append(header.getValue());
} else {
sb.append(REDACTED_PLACEHOLDER);
Expand All @@ -145,7 +145,7 @@ private void formatAllowableQueryParams(Set<String> allowedQueryParamNames, Stri
for (String queryParam : queryParams) {
String[] queryPair = queryParam.split("=", 2);
if (queryPair.length == 2) {
if (allowedQueryParamNames.contains(queryPair[0])) {
if (allowedQueryParamNames.stream().anyMatch(queryPair[0]::equalsIgnoreCase)) {
sb.append(queryParam);
} else {
sb.append(queryPair[0]).append("=").append(REDACTED_PLACEHOLDER);
Expand Down