From 47bccc02d4acd6d744725cabe523041b9236ab7a Mon Sep 17 00:00:00 2001 From: Sameeksha Vaity Date: Wed, 16 Oct 2019 16:39:29 -0700 Subject: [PATCH 1/3] add default headers to whitelist --- .../core/http/policy/HttpLogOptions.java | 37 +++++++++++++++++-- .../core/http/policy/HttpLoggingPolicy.java | 20 +++++----- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLogOptions.java b/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLogOptions.java index b3e1e84b1a27..b057173c6452 100644 --- a/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLogOptions.java +++ b/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLogOptions.java @@ -3,6 +3,7 @@ package com.azure.core.http.policy; +import java.util.Arrays; import java.util.HashSet; import java.util.Objects; import java.util.Set; @@ -17,7 +18,30 @@ public class HttpLogOptions { public HttpLogOptions() { logLevel = HttpLogDetailLevel.NONE; - allowedHeaderNames = new HashSet<>(); + allowedHeaderNames = new HashSet<>(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" + )); allowedQueryParamNames = new HashSet<>(); } @@ -54,18 +78,23 @@ public Set getAllowedHeaderNames() { /** * Sets the given whitelisted headers that should be logged. + *

+ * If a set of allowedHeaderNames is provided it will override the default set of header names to be whitelisted. + * Additionally, use {@link HttpLogOptions#addAllowedHeaderName(String)} or {@link HttpLogOptions#getAllowedHeaderNames()} + * to add more headers names to the existing set of default allowed header names. + *

+ * If a set of allowedHeaderNames is not provided, the default header names will be used to be whitelisted. * * @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 allowedHeaderNames) { - this.allowedHeaderNames = allowedHeaderNames; + this.allowedHeaderNames = allowedHeaderNames == null ? this.allowedHeaderNames : 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. diff --git a/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLoggingPolicy.java b/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLoggingPolicy.java index b087b8011446..53997c55e56d 100644 --- a/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLoggingPolicy.java +++ b/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLoggingPolicy.java @@ -122,19 +122,17 @@ private Mono logRequest(final ClientLogger logger, final HttpRequest reque private void formatAllowableHeaders(Set allowedHeaderNames, HttpHeaders requestResponseHeaders, ClientLogger logger) { - if (allowedHeaderNames != null && !allowedHeaderNames.isEmpty()) { - StringBuilder sb = new StringBuilder(); - for (HttpHeader header : requestResponseHeaders) { - sb.append(header.getName()).append(":"); - if (allowedHeaderNames.contains(header.getName())) { - sb.append(header.getValue()); - } else { - sb.append(REDACTED_PLACEHOLDER); - } - sb.append(System.getProperty("line.separator")); + StringBuilder sb = new StringBuilder(); + for (HttpHeader header : requestResponseHeaders) { + sb.append(header.getName()).append(":"); + if (allowedHeaderNames.contains(header.getName())) { + sb.append(header.getValue()); + } else { + sb.append(REDACTED_PLACEHOLDER); } - logger.info(sb.toString()); + sb.append(System.getProperty("line.separator")); } + logger.info(sb.toString()); } private void formatAllowableQueryParams(Set allowedQueryParamNames, String queryString, From 7df78936b2aa89483c9d7a3cb0ef00fc2e3e4916 Mon Sep 17 00:00:00 2001 From: Sameeksha Vaity Date: Wed, 16 Oct 2019 20:22:06 -0700 Subject: [PATCH 2/3] review comments --- .../core/http/policy/HttpLogOptions.java | 12 +++++------ .../core/http/policy/HttpLoggingPolicy.java | 20 ++++++++++--------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLogOptions.java b/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLogOptions.java index b057173c6452..fbc18f756b1f 100644 --- a/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLogOptions.java +++ b/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLogOptions.java @@ -79,17 +79,17 @@ public Set getAllowedHeaderNames() { /** * Sets the given whitelisted headers that should be logged. *

- * If a set of allowedHeaderNames is provided it will override the default set of header names to be whitelisted. - * Additionally, use {@link HttpLogOptions#addAllowedHeaderName(String)} or {@link HttpLogOptions#getAllowedHeaderNames()} - * to add more headers names to the existing set of default allowed header names. - *

- * If a set of allowedHeaderNames is not provided, the default header names will be used to be whitelisted. + * 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. */ public HttpLogOptions setAllowedHeaderNames(final Set allowedHeaderNames) { - this.allowedHeaderNames = allowedHeaderNames == null ? this.allowedHeaderNames : allowedHeaderNames; + this.allowedHeaderNames = allowedHeaderNames == null ? new HashSet<>() : allowedHeaderNames; return this; } diff --git a/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLoggingPolicy.java b/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLoggingPolicy.java index 53997c55e56d..96d143a9aed2 100644 --- a/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLoggingPolicy.java +++ b/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLoggingPolicy.java @@ -122,17 +122,19 @@ private Mono logRequest(final ClientLogger logger, final HttpRequest reque private void formatAllowableHeaders(Set allowedHeaderNames, HttpHeaders requestResponseHeaders, ClientLogger logger) { - StringBuilder sb = new StringBuilder(); - for (HttpHeader header : requestResponseHeaders) { - sb.append(header.getName()).append(":"); - if (allowedHeaderNames.contains(header.getName())) { - sb.append(header.getValue()); - } else { - sb.append(REDACTED_PLACEHOLDER); + if (!allowedHeaderNames.isEmpty()) { + StringBuilder sb = new StringBuilder(); + for (HttpHeader header : requestResponseHeaders) { + sb.append(header.getName()).append(":"); + if (allowedHeaderNames.contains(header.getName())) { + sb.append(header.getValue()); + } else { + sb.append(REDACTED_PLACEHOLDER); + } + sb.append(System.getProperty("line.separator")); } - sb.append(System.getProperty("line.separator")); + logger.info(sb.toString()); } - logger.info(sb.toString()); } private void formatAllowableQueryParams(Set allowedQueryParamNames, String queryString, From dc90cb69a979be51429c93aa479a0e21b3efad34 Mon Sep 17 00:00:00 2001 From: Sameeksha Vaity Date: Wed, 16 Oct 2019 21:00:42 -0700 Subject: [PATCH 3/3] add case insensitive --- .../core/http/policy/HttpLogOptions.java | 52 ++++++++++--------- .../core/http/policy/HttpLoggingPolicy.java | 4 +- 2 files changed, 29 insertions(+), 27 deletions(-) diff --git a/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLogOptions.java b/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLogOptions.java index fbc18f756b1f..bfac6a05a7ce 100644 --- a/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLogOptions.java +++ b/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLogOptions.java @@ -5,6 +5,7 @@ import java.util.Arrays; import java.util.HashSet; +import java.util.List; import java.util.Objects; import java.util.Set; @@ -15,34 +16,35 @@ public class HttpLogOptions { private HttpLogDetailLevel logLevel; private Set allowedHeaderNames; private Set allowedQueryParamNames; + private static final List 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<>(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" - )); - allowedQueryParamNames = new HashSet<>(); + allowedHeaderNames = new HashSet<>(); + allowedQueryParamNames = new HashSet<>(DEFAULT_HEADERS_WHITELIST); } /** diff --git a/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLoggingPolicy.java b/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLoggingPolicy.java index 96d143a9aed2..06a065a5eaf6 100644 --- a/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLoggingPolicy.java +++ b/sdk/core/azure-core/src/main/java/com/azure/core/http/policy/HttpLoggingPolicy.java @@ -126,7 +126,7 @@ private void formatAllowableHeaders(Set allowedHeaderNames, HttpHeaders StringBuilder sb = new StringBuilder(); for (HttpHeader header : requestResponseHeaders) { sb.append(header.getName()).append(":"); - if (allowedHeaderNames.contains(header.getName())) { + if (allowedHeaderNames.stream().anyMatch(header.getName()::equalsIgnoreCase)) { sb.append(header.getValue()); } else { sb.append(REDACTED_PLACEHOLDER); @@ -145,7 +145,7 @@ private void formatAllowableQueryParams(Set 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);