Skip to content

Commit 186bb85

Browse files
committed
Addresses review comment on ObservabilityUtils.java for key-only query parameter redaction and fixes HttpJsonClientCallsTest test failure.
1 parent 0e37152 commit 186bb85

File tree

3 files changed

+41
-42
lines changed

3 files changed

+41
-42
lines changed

sdk-platform-java/gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonClientCallsTest.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,9 @@ void setUp() throws IOException {
9393
@Test
9494
void testValidUniverseDomain() {
9595
HttpJsonClientCalls.newCall(descriptor, callContext);
96-
Mockito.verify(mockChannel, Mockito.times(1)).newCall(descriptor, callOptions);
96+
HttpJsonCallOptions expectedCallOptions =
97+
callOptions.toBuilder().setTracer(callContext.getTracer()).build();
98+
Mockito.verify(mockChannel, Mockito.times(1)).newCall(descriptor, expectedCallOptions);
9799
}
98100

99101
// This test is when the universe domain does not match

sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityUtils.java

Lines changed: 31 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,15 @@
4242

4343
final class ObservabilityUtils {
4444

45-
private ObservabilityUtils() {
46-
}
45+
private ObservabilityUtils() {}
4746

4847
/** Constant for redacted values. */
4948
private static final String REDACTED_VALUE = "REDACTED";
5049

5150
/**
52-
* A set of lowercase query parameter keys whose values should be
53-
* redacted in URLs for observability. These include direct credentials
54-
* (access keys), cryptographic signatures (to prevent replay attacks
55-
* or leak of authorization), and session identifiers (like upload_id).
51+
* A set of lowercase query parameter keys whose values should be redacted in URLs for
52+
* observability. These include direct credentials (access keys), cryptographic signatures (to
53+
* prevent replay attacks or leak of authorization), and session identifiers (like upload_id).
5654
*/
5755
private static final ImmutableSet<String> SENSITIVE_QUERY_KEYS =
5856
ImmutableSet.of(
@@ -66,27 +64,24 @@ private ObservabilityUtils() {
6664
"api_key"); // API Keys
6765

6866
/**
69-
* Sanitizes an HTTP URL by redacting sensitive query parameters and
70-
* credentials in the user-info component. If the provided URL cannot
71-
* be parsed (e.g. invalid syntax), it returns the original string.
67+
* Sanitizes an HTTP URL by redacting sensitive query parameters and credentials in the user-info
68+
* component. If the provided URL cannot be parsed (e.g. invalid syntax), it returns the original
69+
* string.
7270
*
73-
* <p>This sanitization process conforms to the recommendations in footnote 3
74-
* of the OpenTelemetry semantic conventions for HTTP URL attributes:
71+
* <p>This sanitization process conforms to the recommendations in footnote 3 of the OpenTelemetry
72+
* semantic conventions for HTTP URL attributes:
7573
* https://opentelemetry.io/docs/specs/semconv/registry/attributes/url/
7674
*
7775
* <ul>
7876
* <li><i>"url.full MUST NOT contain credentials passed via URL in form of
79-
* https://user:pass@example.com/. In such case username and password
80-
* SHOULD be redacted and attribute's value SHOULD be
81-
* https://REDACTED:REDACTED@example.com/."</i>
82-
* - Handled by stripping the raw user info component.
83-
* <li><i>"url.full SHOULD capture the absolute URL when it is available
84-
* (or can be reconstructed)."</i>
85-
* - Handled by parsing and rebuilding the generic URI.
86-
* <li><i>"When a query string value is redacted, the query string key
87-
* SHOULD still be preserved, e.g.
88-
* https://www.example.com/path?color=blue&sig=REDACTED."</i>
89-
* - Handled by the redactSensitiveQueryValues method.
77+
* https://user:pass@example.com/. In such case username and password SHOULD be redacted and
78+
* attribute's value SHOULD be https://REDACTED:REDACTED@example.com/."</i> - Handled by
79+
* stripping the raw user info component.
80+
* <li><i>"url.full SHOULD capture the absolute URL when it is available (or can be
81+
* reconstructed)."</i> - Handled by parsing and rebuilding the generic URI.
82+
* <li><i>"When a query string value is redacted, the query string key SHOULD still be
83+
* preserved, e.g. https://www.example.com/path?color=blue&sig=REDACTED."</i> - Handled by
84+
* the redactSensitiveQueryValues method.
9085
* </ul>
9186
*
9287
* @param url the raw URL string
@@ -96,9 +91,7 @@ static String sanitizeUrlFull(final String url) {
9691
try {
9792
java.net.URI uri = new java.net.URI(url);
9893
String sanitizedUserInfo =
99-
uri.getRawUserInfo() != null
100-
? REDACTED_VALUE + ":" + REDACTED_VALUE
101-
: null;
94+
uri.getRawUserInfo() != null ? REDACTED_VALUE + ":" + REDACTED_VALUE : null;
10295
String sanitizedQuery = redactSensitiveQueryValues(uri.getRawQuery());
10396
java.net.URI sanitizedUri =
10497
new java.net.URI(
@@ -118,16 +111,14 @@ static String sanitizeUrlFull(final String url) {
118111
/**
119112
* Redacts the values of sensitive keys within a raw URI query string.
120113
*
121-
* <p>This logic splits the query string by the {@code &} delimiter
122-
* without full URL decoding, ensures only values belonging to predefined
123-
* sensitive keys are replaced with {@code REDACTED_VALUE}.
124-
* The check is strictly case-insensitive.
114+
* <p>This logic splits the query string by the {@code &} delimiter without full URL decoding,
115+
* ensures only values belonging to predefined sensitive keys are replaced with {@code
116+
* REDACTED_VALUE}. The check is strictly case-insensitive.
125117
*
126-
* <p>Note regarding Footnote 3: The OpenTelemetry spec recommends
127-
* case-sensitive matching for query parameters. However, we intentionally
128-
* utilize case-insensitive matching (by lowercasing all query keys)
129-
* to prevent credentials bypassing validation when sent with mixed
130-
* casings (e.g., Sig=..., API_KEY=...).
118+
* <p>Note regarding Footnote 3: The OpenTelemetry spec recommends case-sensitive matching for
119+
* query parameters. However, we intentionally utilize case-insensitive matching (by lowercasing
120+
* all query keys) to prevent credentials bypassing validation when sent with mixed casings (e.g.,
121+
* Sig=..., API_KEY=...).
131122
*
132123
* @param rawQuery the raw query string from a java.net.URI
133124
* @return a reconstructed query sequence with sensitive values redacted
@@ -142,13 +133,13 @@ private static String redactSensitiveQueryValues(final String rawQuery) {
142133
.map(
143134
param -> {
144135
int equalsIndex = param.indexOf('=');
145-
String key = equalsIndex >= 0
146-
? param.substring(0, equalsIndex)
147-
: param;
136+
if (equalsIndex < 0) {
137+
return param;
138+
}
139+
String key = param.substring(0, equalsIndex);
148140
// Case-insensitive match utilizing the fact that all
149141
// predefined keys are in lowercase
150-
if (SENSITIVE_QUERY_KEYS.contains(
151-
key.toLowerCase(java.util.Locale.US))) {
142+
if (SENSITIVE_QUERY_KEYS.contains(key.toLowerCase())) {
152143
return key + "=" + REDACTED_VALUE;
153144
}
154145
return param;
@@ -172,8 +163,7 @@ static String extractStatus(@Nullable final Throwable error) {
172163
} else if (error instanceof CancellationException) {
173164
statusString = StatusCode.Code.CANCELLED.toString();
174165
} else if (error instanceof ApiException) {
175-
statusString =
176-
((ApiException) error).getStatusCode().getCode().toString();
166+
statusString = ((ApiException) error).getStatusCode().getCode().toString();
177167
} else {
178168
statusString = StatusCode.Code.UNKNOWN.toString();
179169
}

sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/ObservabilityUtilsTest.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,13 @@ void testSanitizeUrlFull_redactsSensitiveQueryParameters_caseInsensitive() {
136136
"https://example.com/some/path?upload_Id=REDACTED&AWSAccessKeyId=REDACTED&foo=bar&API_KEY=REDACTED");
137137
}
138138

139+
@Test
140+
void testSanitizeUrlFull_handlesKeyOnlyParameters() {
141+
String url = "https://example.com/some/path?api_key&foo=bar";
142+
String sanitized = ObservabilityUtils.sanitizeUrlFull(url);
143+
assertThat(sanitized).isEqualTo("https://example.com/some/path?api_key&foo=bar");
144+
}
145+
139146
@Test
140147
void testSanitizeUrlFull_handlesMalformedUrl() {
141148
String url = "invalid::url:";

0 commit comments

Comments
 (0)