Skip to content

Commit 6bab9f3

Browse files
authored
fix: improves debug logging of HTTP requests (#150)
<!-- CURSOR_SUMMARY --> > [!NOTE] > **Low Risk** > Low risk: changes are limited to debug logging and a new unit test, with no impact on request execution or data parsing aside from log formatting. > > **Overview** > **Prevents leaking SDK credentials in debug logs** by sanitizing the polling request string before logging in `DefaultFeatureRequestor` (redacts the `Authorization` header value and uses deferred log evaluation). > > Adds a regression test ensuring the "Making request" debug message contains `REDACTED` and does not include the SDK key, while confirming the actual HTTP request still sends the real `Authorization` header. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 3b786dc. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 820c5e4 commit 6bab9f3

2 files changed

Lines changed: 46 additions & 1 deletion

File tree

‎lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/DefaultFeatureRequestor.java‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import com.google.common.annotations.VisibleForTesting;
1313
import com.google.gson.stream.JsonReader;
1414
import com.launchdarkly.logging.LDLogger;
15+
import com.launchdarkly.logging.LogValues;
1516
import com.launchdarkly.sdk.internal.http.HttpConsts;
1617
import com.launchdarkly.sdk.internal.http.HttpErrors.HttpErrorException;
1718
import com.launchdarkly.sdk.internal.http.HttpHelpers;
@@ -32,6 +33,22 @@
3233
final class DefaultFeatureRequestor implements FeatureRequestor {
3334
private static final long MAX_HTTP_CACHE_SIZE_BYTES = 10 * 1024 * 1024; // 10 MB
3435

36+
private static final String REDACTED = "REDACTED";
37+
38+
private static String sanitizedRequestStringForLogging(Request request) {
39+
Request.Builder builder = request.newBuilder();
40+
redactHeaderValueForLog(builder, request, "Authorization");
41+
return builder.build().toString();
42+
}
43+
44+
private static void redactHeaderValueForLog(Request.Builder builder, Request request, String name) {
45+
if (request.headers(name).isEmpty()) {
46+
return;
47+
}
48+
builder.removeHeader(name);
49+
builder.addHeader(name, REDACTED);
50+
}
51+
3552
private final OkHttpClient httpClient;
3653
@VisibleForTesting
3754
final URI pollingUri;
@@ -88,7 +105,7 @@ public FullDataSet<ItemDescriptor> getAllData(boolean returnDataEvenIfCached)
88105
.get()
89106
.build();
90107

91-
logger.debug("Making request: " + request);
108+
logger.debug("Making request: {}", LogValues.defer(() -> sanitizedRequestStringForLogging(request)));
92109

93110
try (Response response = httpClient.newCall(request).execute()) {
94111
boolean wasCached = response.networkResponse() == null || response.networkResponse().code() == 304;

‎lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/DefaultFeatureRequestorTest.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.launchdarkly.sdk.server;
22

3+
import com.launchdarkly.logging.LDLogLevel;
4+
import com.launchdarkly.logging.LogCapture;
35
import com.launchdarkly.sdk.internal.http.HttpErrors.HttpErrorException;
46
import com.launchdarkly.sdk.internal.http.HttpProperties;
57
import com.launchdarkly.sdk.server.DataModel.FeatureFlag;
@@ -27,7 +29,9 @@
2729
import static com.launchdarkly.sdk.server.TestComponents.clientContext;
2830
import static com.launchdarkly.sdk.server.TestUtil.assertDataSetEquals;
2931
import static org.hamcrest.MatcherAssert.assertThat;
32+
import static org.hamcrest.Matchers.containsString;
3033
import static org.hamcrest.Matchers.equalTo;
34+
import static org.hamcrest.Matchers.not;
3135
import static org.junit.Assert.assertEquals;
3236
import static org.junit.Assert.assertNotNull;
3337
import static org.junit.Assert.assertNull;
@@ -244,4 +248,28 @@ private void verifyHeaders(RequestInfo req) {
244248
assertThat(req.getHeader(kv.getKey()), equalTo(kv.getValue()));
245249
}
246250
}
251+
252+
@Test
253+
public void makingRequestDebugLogRedactsAuthorizationValue() throws Exception {
254+
Handler resp = Handlers.bodyJson(allDataJson);
255+
try (HttpServer server = HttpServer.start(resp)) {
256+
try (DefaultFeatureRequestor r = makeRequestor(server)) {
257+
r.getAllData(true);
258+
259+
String makingRequestLog = null;
260+
for (LogCapture.Message m : logCapture.getMessages()) {
261+
if (m.getLevel() == LDLogLevel.DEBUG && m.getText().contains("Making request:")) {
262+
makingRequestLog = m.getText();
263+
break;
264+
}
265+
}
266+
assertNotNull("expected Making request debug log", makingRequestLog);
267+
assertThat(makingRequestLog, containsString("REDACTED"));
268+
assertThat(makingRequestLog, not(containsString(sdkKey)));
269+
270+
RequestInfo req = server.getRecorder().requireRequest();
271+
assertEquals(sdkKey, req.getHeader("Authorization"));
272+
}
273+
}
274+
}
247275
}

0 commit comments

Comments
 (0)