Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
52 changes: 52 additions & 0 deletions src/main/java/com/stripe/net/HttpClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.io.InputStream;
import java.net.ConnectException;
import java.net.SocketTimeoutException;
import java.security.MessageDigest;
import java.time.Duration;
import java.util.HashMap;
import java.util.Map;
Expand All @@ -26,6 +27,53 @@ public abstract class HttpClient {
/** A value indicating whether the client should sleep between automatic request retries. */
boolean networkRetriesSleep = true;

static String UNAME_HASH = computeUnameHash();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💭 Heads-up · Bug · UNAME_HASH is declared as a non-final, non-volatile package-private static field (static String UNAME_HASH). This has two problems: (1) Without volatile, reads/writes from multiple threads are not guaranteed to see a consistent value — a thread could observe a partially-initialized or stale value. (2) The field is mutable by any code in the same package (as demonstrated by the test directly assigning HttpClient.UNAME_HASH = ""), making it easy to accidentally corrupt shared state. Since the value is computed once at class-load time and should never change, it should be declared static final. The test that mutates it should instead use a test-specific override mechanism (e.g., a package-private setter or reflection) rather than relying on the field being mutable.

No historical precedent in your repo — agent reasoning only. Not blocking. Skim, ignore, or address.

🔗 Backed by: agent reasoning

Suggested change
static String UNAME_HASH = computeUnameHash();
static final String UNAME_HASH = computeUnameHash();

Was this helpful? React with 👍 or 👎 to help VibeOps learn.


private static String computeUnameHash() {
String uname = "";
try {
uname =
(System.getProperty("os.name", "")
+ " "
+ System.getProperty("os.version", "")
+ " "
+ System.getProperty("os.arch", "")
+ " "
+ System.getProperty("java.version", "")
+ " "
+ System.getProperty("java.vendor", "")
+ " "
+ System.getProperty("java.vm.name", "")
+ " "
+ getHostname())
.trim();
} catch (Exception e) {
// fall through with empty string
}
if (uname.isEmpty()) {
return "";
}
try {
MessageDigest md = MessageDigest.getInstance("MD5");
byte[] hashBytes = md.digest(uname.getBytes(java.nio.charset.StandardCharsets.UTF_8));
StringBuilder sb = new StringBuilder();
for (byte b : hashBytes) {
sb.append(String.format("%02x", b));
}
return sb.toString();
} catch (Exception e) {
return "";
}
}

private static String getHostname() {
try {
return java.net.InetAddress.getLocalHost().getHostName();
} catch (Exception e) {
return "";
}
}

/** Initializes a new instance of the {@link HttpClient} class. */
protected HttpClient() {}

Expand Down Expand Up @@ -228,6 +276,10 @@ static String buildXStripeClientUserAgentString(String aiAgent) {
propertyMap.put("ai_agent", aiAgent);
}

if (!UNAME_HASH.isEmpty()) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛑 Blocker · Security · The UNAME_HASH is included in the X-Stripe-Client-User-Agent header unconditionally, even when users set Stripe.enableTelemetry = false to opt out of system information collection. The platform field correctly respects this setting (lines 261-269), but the source field does not. This creates inconsistent privacy behavior where users who explicitly disable telemetry still have their system fingerprint (OS, Java version, hostname hash) transmitted. Fix: wrap the source field insertion with the same telemetry guard: if (Stripe.enableTelemetry && !UNAME_HASH.isEmpty()) { propertyMap.put("source", UNAME_HASH); }

🔗 Backed by: agent reasoning

Suggested change
if (!UNAME_HASH.isEmpty()) {
if (Stripe.enableTelemetry && !UNAME_HASH.isEmpty()) {
propertyMap.put("source", UNAME_HASH);
}

Was this helpful? React with 👍 or 👎 to help VibeOps learn.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛑 Blocker · Security · The source field (MD5 hash including hostname) is added to X-Stripe-Client-User-Agent unconditionally, while the platform field (OS info only) respects Stripe.enableTelemetry. When users set Stripe.enableTelemetry = false to opt out of telemetry, they expect no system information to be transmitted, but the hostname-derived hash still gets sent. Fix: wrap the source field addition in if (Stripe.enableTelemetry) like the platform field at lines 261-268.

🔗 Backed by: agent reasoning

Suggested change
if (!UNAME_HASH.isEmpty()) {
if (Stripe.enableTelemetry && !UNAME_HASH.isEmpty()) {
propertyMap.put("source", UNAME_HASH);
}

Was this helpful? React with 👍 or 👎 to help VibeOps learn.

propertyMap.put("source", UNAME_HASH);
}

return ApiResource.INTERNAL_GSON.toJson(propertyMap);
}

Expand Down
28 changes: 28 additions & 0 deletions src/test/java/com/stripe/net/HttpClientTest.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.stripe.net;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -295,4 +296,31 @@ public void testBuildXStripeClientUserAgentStringNoPlatformWithoutTelemetry() {
Stripe.enableTelemetry = originalTelemetry;
}
}

@Test
public void testBuildXStripeClientUserAgentStringIncludesSource() {
String json = HttpClient.buildXStripeClientUserAgentString("");
com.google.gson.JsonObject parsed =
com.google.gson.JsonParser.parseString(json).getAsJsonObject();
// "source" should be present and be a 32-character lowercase MD5 hex digest
assertTrue(parsed.has("source"), "Expected 'source' field in X-Stripe-Client-User-Agent");
String source = parsed.get("source").getAsString();
assertTrue(
source.matches("[0-9a-f]{32}"),
"Expected 'source' to be a 32-character lowercase hex string, got: " + source);
}

@Test
public void testBuildXStripeClientUserAgentStringOmitsSourceWhenEmpty() throws Exception {
String savedHash = HttpClient.UNAME_HASH;
try {
HttpClient.UNAME_HASH = "";
String userAgentString = HttpClient.buildXStripeClientUserAgentString("");
com.google.gson.JsonObject userAgent =
com.google.gson.JsonParser.parseString(userAgentString).getAsJsonObject();
assertFalse(userAgent.has("source"));
} finally {
HttpClient.UNAME_HASH = savedHash;
}
}
}