Skip to content

Commit c2f9023

Browse files
brandboatshowuon
brandboat
and
showuon
authored
MINOR: Improve some requests/responses toString method (#20776)
Improve some requests/responses toString method to log only the required info, including the request.Builder toString methods. 1. AlterConfigsRequest 2. AlterUserScramCredentialsRequest 3. ExpireDelegationTokenRequest 4. IncrementalAlterConfigsRequest 5. RenewDelegationTokenRequest 6. SaslAuthenticateRequest 7. createDelegationTokenResponse 8. describeDelegationTokenResponse 9. SaslAuthenticateResponse Reviewers: TaiJuWu <[email protected]>, Chia-Ping Tsai <[email protected]> Co-authored-by: Luke Chen <[email protected]>
1 parent e118c08 commit c2f9023

File tree

11 files changed

+170
-34
lines changed

11 files changed

+170
-34
lines changed

‎clients/src/main/java/org/apache/kafka/common/requests/AlterConfigsRequest.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,11 @@ public Builder(Map<ConfigResource, Config> configs, boolean validateOnly) {
8888
public AlterConfigsRequest build(short version) {
8989
return new AlterConfigsRequest(data, version);
9090
}
91+
92+
@Override
93+
public String toString() {
94+
return maskData(data);
95+
}
9196
}
9297

9398
private final AlterConfigsRequestData data;
@@ -135,4 +140,20 @@ public AbstractResponse getErrorResponse(int throttleTimeMs, Throwable e) {
135140
public static AlterConfigsRequest parse(ByteBuffer buffer, short version) {
136141
return new AlterConfigsRequest(new AlterConfigsRequestData(new ByteBufferAccessor(buffer), version), version);
137142
}
143+
144+
// It is not safe to print all config values
145+
private static String maskData(AlterConfigsRequestData data) {
146+
AlterConfigsRequestData tempData = data.duplicate();
147+
tempData.resources().forEach(resource -> {
148+
resource.configs().forEach(config -> {
149+
config.setValue("REDACTED");
150+
});
151+
});
152+
return tempData.toString();
153+
}
154+
155+
@Override
156+
public String toString() {
157+
return maskData(data);
158+
}
138159
}

‎clients/src/main/java/org/apache/kafka/common/requests/AlterUserScramCredentialsRequest.java‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,10 @@
1717
package org.apache.kafka.common.requests;
1818

1919
import org.apache.kafka.common.message.AlterUserScramCredentialsRequestData;
20-
import org.apache.kafka.common.message.AlterUserScramCredentialsRequestDataJsonConverter;
2120
import org.apache.kafka.common.message.AlterUserScramCredentialsResponseData;
2221
import org.apache.kafka.common.protocol.ApiKeys;
2322
import org.apache.kafka.common.protocol.ByteBufferAccessor;
2423

25-
import com.fasterxml.jackson.databind.JsonNode;
26-
import com.fasterxml.jackson.databind.node.ObjectNode;
27-
2824
import java.nio.ByteBuffer;
2925
import java.util.List;
3026
import java.util.Set;
@@ -48,7 +44,7 @@ public AlterUserScramCredentialsRequest build(short version) {
4844

4945
@Override
5046
public String toString() {
51-
return data.toString();
47+
return maskData(data);
5248
}
5349
}
5450

@@ -87,15 +83,18 @@ public AbstractResponse getErrorResponse(int throttleTimeMs, Throwable e) {
8783
return new AlterUserScramCredentialsResponse(new AlterUserScramCredentialsResponseData().setResults(results));
8884
}
8985

86+
private static String maskData(AlterUserScramCredentialsRequestData data) {
87+
AlterUserScramCredentialsRequestData tempData = data.duplicate();
88+
tempData.upsertions().forEach(upsertion -> {
89+
upsertion.setSalt(new byte[0]);
90+
upsertion.setSaltedPassword(new byte[0]);
91+
});
92+
return tempData.toString();
93+
}
94+
9095
// Do not print salt or saltedPassword
9196
@Override
9297
public String toString() {
93-
JsonNode json = AlterUserScramCredentialsRequestDataJsonConverter.write(data, version()).deepCopy();
94-
95-
for (JsonNode upsertion : json.get("upsertions")) {
96-
((ObjectNode) upsertion).put("salt", "");
97-
((ObjectNode) upsertion).put("saltedPassword", "");
98-
}
99-
return AlterUserScramCredentialsRequestDataJsonConverter.read(json, version()).toString();
98+
return maskData(data);
10099
}
101100
}

‎clients/src/main/java/org/apache/kafka/common/requests/CreateDelegationTokenResponse.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,4 +103,13 @@ public boolean hasError() {
103103
public boolean shouldClientThrottle(short version) {
104104
return version >= 1;
105105
}
106+
107+
// Do not print tokenId and Hmac, overwrite a temp copy of the data with empty content
108+
@Override
109+
public String toString() {
110+
CreateDelegationTokenResponseData tempData = data.duplicate();
111+
tempData.setTokenId("REDACTED");
112+
tempData.setHmac(new byte[0]);
113+
return tempData.toString();
114+
}
106115
}

‎clients/src/main/java/org/apache/kafka/common/requests/DescribeDelegationTokenResponse.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,4 +128,15 @@ public boolean hasError() {
128128
public boolean shouldClientThrottle(short version) {
129129
return version >= 1;
130130
}
131+
132+
// Do not print tokenId and Hmac, overwrite a temp copy of the data with empty content
133+
@Override
134+
public String toString() {
135+
DescribeDelegationTokenResponseData tempData = data.duplicate();
136+
tempData.tokens().forEach(token -> {
137+
token.setTokenId("REDACTED");
138+
token.setHmac(new byte[0]);
139+
});
140+
return tempData.toString();
141+
}
131142
}

‎clients/src/main/java/org/apache/kafka/common/requests/ExpireDelegationTokenRequest.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,19 @@ public ExpireDelegationTokenRequest build(short version) {
7474

7575
@Override
7676
public String toString() {
77-
return data.toString();
77+
return maskData(data);
7878
}
7979
}
80+
81+
private static String maskData(ExpireDelegationTokenRequestData data) {
82+
ExpireDelegationTokenRequestData tempData = data.duplicate();
83+
tempData.setHmac(new byte[0]);
84+
return tempData.toString();
85+
}
86+
87+
// Do not print Hmac, overwrite a temp copy of the data with empty content
88+
@Override
89+
public String toString() {
90+
return maskData(data);
91+
}
8092
}

‎clients/src/main/java/org/apache/kafka/common/requests/IncrementalAlterConfigsRequest.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,11 @@
2121
import org.apache.kafka.common.config.ConfigResource;
2222
import org.apache.kafka.common.message.IncrementalAlterConfigsRequestData;
2323
import org.apache.kafka.common.message.IncrementalAlterConfigsRequestData.AlterConfigsResource;
24-
import org.apache.kafka.common.message.IncrementalAlterConfigsRequestDataJsonConverter;
2524
import org.apache.kafka.common.message.IncrementalAlterConfigsResponseData;
2625
import org.apache.kafka.common.message.IncrementalAlterConfigsResponseData.AlterConfigsResourceResponse;
2726
import org.apache.kafka.common.protocol.ApiKeys;
2827
import org.apache.kafka.common.protocol.ByteBufferAccessor;
2928

30-
import com.fasterxml.jackson.databind.JsonNode;
31-
import com.fasterxml.jackson.databind.node.ObjectNode;
32-
3329
import java.nio.ByteBuffer;
3430
import java.util.Collection;
3531
import java.util.Map;
@@ -77,7 +73,7 @@ public IncrementalAlterConfigsRequest build(short version) {
7773

7874
@Override
7975
public String toString() {
80-
return data.toString();
76+
return maskData(data);
8177
}
8278
}
8379

@@ -113,14 +109,18 @@ public AbstractResponse getErrorResponse(final int throttleTimeMs, final Throwab
113109
}
114110

115111
// It is not safe to print all config values
112+
private static String maskData(IncrementalAlterConfigsRequestData data) {
113+
IncrementalAlterConfigsRequestData tempData = data.duplicate();
114+
tempData.resources().forEach(resource -> {
115+
resource.configs().forEach(config -> {
116+
config.setValue("REDACTED");
117+
});
118+
});
119+
return tempData.toString();
120+
}
121+
116122
@Override
117123
public String toString() {
118-
JsonNode json = IncrementalAlterConfigsRequestDataJsonConverter.write(data, version()).deepCopy();
119-
for (JsonNode resource : json.get("resources")) {
120-
for (JsonNode config : resource.get("configs")) {
121-
((ObjectNode) config).put("value", "REDACTED");
122-
}
123-
}
124-
return IncrementalAlterConfigsRequestDataJsonConverter.read(json, version()).toString();
124+
return maskData(data);
125125
}
126126
}

‎clients/src/main/java/org/apache/kafka/common/requests/RenewDelegationTokenRequest.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,19 @@ public RenewDelegationTokenRequest build(short version) {
6666

6767
@Override
6868
public String toString() {
69-
return data.toString();
69+
return maskData(data);
7070
}
7171
}
72+
73+
private static String maskData(RenewDelegationTokenRequestData data) {
74+
RenewDelegationTokenRequestData tempData = data.duplicate();
75+
tempData.setHmac(new byte[0]);
76+
return tempData.toString();
77+
}
78+
79+
// Do not print Hmac, overwrite a temp copy of the data with empty content
80+
@Override
81+
public String toString() {
82+
return maskData(data);
83+
}
7284
}

‎clients/src/main/java/org/apache/kafka/common/requests/SaslAuthenticateRequest.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,4 +78,12 @@ public static SaslAuthenticateRequest parse(ByteBuffer buffer, short version) {
7878
return new SaslAuthenticateRequest(new SaslAuthenticateRequestData(new ByteBufferAccessor(buffer), version),
7979
version);
8080
}
81+
82+
// Do not print authBytes, overwrite a temp copy of the data with empty bytes
83+
@Override
84+
public String toString() {
85+
SaslAuthenticateRequestData tempData = data.duplicate();
86+
tempData.setAuthBytes(new byte[0]);
87+
return tempData.toString();
88+
}
8189
}

‎clients/src/main/java/org/apache/kafka/common/requests/SaslAuthenticateResponse.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,12 @@ public SaslAuthenticateResponseData data() {
8080
public static SaslAuthenticateResponse parse(ByteBuffer buffer, short version) {
8181
return new SaslAuthenticateResponse(new SaslAuthenticateResponseData(new ByteBufferAccessor(buffer), version));
8282
}
83+
84+
// Do not print authBytes, overwrite a temp copy of the data with empty bytes
85+
@Override
86+
public String toString() {
87+
SaslAuthenticateResponseData tempData = data.duplicate();
88+
tempData.setAuthBytes(new byte[0]);
89+
return tempData.toString();
90+
}
8391
}

‎clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java‎

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3317,15 +3317,27 @@ private DescribeConfigsResponse createDescribeConfigsResponse(short version) {
33173317
}
33183318

33193319
private AlterConfigsRequest createAlterConfigsRequest(short version) {
3320-
Map<ConfigResource, AlterConfigsRequest.Config> configs = new HashMap<>();
3320+
Map<ConfigResource, AlterConfigsRequest.Config> configs = new LinkedHashMap<>();
33213321
List<AlterConfigsRequest.ConfigEntry> configEntries = asList(
33223322
new AlterConfigsRequest.ConfigEntry("config_name", "config_value"),
33233323
new AlterConfigsRequest.ConfigEntry("another_name", "another value")
33243324
);
33253325
configs.put(new ConfigResource(ConfigResource.Type.BROKER, "0"), new AlterConfigsRequest.Config(configEntries));
33263326
configs.put(new ConfigResource(ConfigResource.Type.TOPIC, "topic"),
33273327
new AlterConfigsRequest.Config(emptyList()));
3328-
return new AlterConfigsRequest.Builder(configs, false).build(version);
3328+
AlterConfigsRequest alterConfigsRequest = new AlterConfigsRequest.Builder(configs, false).build(version);
3329+
assertEquals(
3330+
"AlterConfigsRequestData(resources=[" +
3331+
"AlterConfigsResource(resourceType=" + ConfigResource.Type.BROKER.id() + ", " +
3332+
"resourceName='0', " +
3333+
"configs=[AlterableConfig(name='config_name', value='REDACTED'), " +
3334+
"AlterableConfig(name='another_name', value='REDACTED')]), " +
3335+
"AlterConfigsResource(resourceType=" + ConfigResource.Type.TOPIC.id() + ", " +
3336+
"resourceName='topic', configs=[])], " +
3337+
"validateOnly=false)",
3338+
alterConfigsRequest.toString()
3339+
);
3340+
return alterConfigsRequest;
33293341
}
33303342

33313343
private AlterConfigsResponse createAlterConfigsResponse() {
@@ -3428,7 +3440,12 @@ private CreateDelegationTokenResponse createCreateTokenResponse() {
34283440
.setMaxTimestampMs(System.currentTimeMillis())
34293441
.setTokenId("token1")
34303442
.setHmac("test".getBytes());
3431-
return new CreateDelegationTokenResponse(data);
3443+
CreateDelegationTokenResponse response = new CreateDelegationTokenResponse(data);
3444+
3445+
String responseStr = response.toString();
3446+
assertTrue(responseStr.contains("tokenId='REDACTED'"));
3447+
assertTrue(responseStr.contains("hmac=[]"));
3448+
return response;
34323449
}
34333450

34343451
private RenewDelegationTokenRequest createRenewTokenRequest(short version) {
@@ -3484,7 +3501,14 @@ private DescribeDelegationTokenResponse createDescribeTokenResponse(short versio
34843501
tokenList.add(new DelegationToken(tokenInfo1, "test".getBytes()));
34853502
tokenList.add(new DelegationToken(tokenInfo2, "test".getBytes()));
34863503

3487-
return new DescribeDelegationTokenResponse(version, 20, Errors.NONE, tokenList);
3504+
DescribeDelegationTokenResponse response = new DescribeDelegationTokenResponse(version, 20, Errors.NONE, tokenList);
3505+
3506+
String responseStr = response.toString();
3507+
String[] parts = responseStr.split(",");
3508+
// The 2 token info should both be redacted
3509+
assertEquals(2, Arrays.stream(parts).filter(s -> s.trim().contains("tokenId='REDACTED'")).count());
3510+
assertEquals(2, Arrays.stream(parts).filter(s -> s.trim().contains("hmac=[]")).count());
3511+
return response;
34883512
}
34893513

34903514
private ElectLeadersRequest createElectLeadersRequestNullPartitions() {
@@ -4101,4 +4125,26 @@ public void testInvalidTaggedFieldsWithSaslAuthenticateRequest() {
41014125
parseRequest(SASL_AUTHENTICATE, SASL_AUTHENTICATE.latestVersion(), accessor.buffer())).getMessage();
41024126
assertEquals("Error reading byte array of 32767 byte(s): only 3 byte(s) available", msg);
41034127
}
4128+
4129+
@Test
4130+
public void testSaslAuthenticateRequestResponseToStringMasksSensitiveData() {
4131+
byte[] sensitiveAuthBytes = "sensitive-auth-token-123".getBytes(StandardCharsets.UTF_8);
4132+
SaslAuthenticateRequestData requestData = new SaslAuthenticateRequestData().setAuthBytes(sensitiveAuthBytes);
4133+
SaslAuthenticateRequest request = new SaslAuthenticateRequest(requestData, (short) 2);
4134+
4135+
String requestString = request.toString();
4136+
4137+
// Verify that the authBytes field is present but empty in the output
4138+
assertTrue(requestString.contains("authBytes=[]"),
4139+
"authBytes field should be empty in toString() output");
4140+
4141+
SaslAuthenticateResponseData responseData = new SaslAuthenticateResponseData().setAuthBytes(sensitiveAuthBytes);
4142+
SaslAuthenticateResponse response = new SaslAuthenticateResponse(responseData);
4143+
4144+
String responseString = response.toString();
4145+
4146+
// Verify that the authBytes field is present but empty in the output
4147+
assertTrue(responseString.contains("authBytes=[]"),
4148+
"authBytes field should be empty in toString() output");
4149+
}
41044150
}

0 commit comments

Comments
 (0)