Skip to content

Commit 23d6764

Browse files
showuonshowuon
authored
MINOR: Improve some requests/responses toString method (#20759)
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: Chia-Ping Tsai <[email protected]>, Manikumar Reddy <[email protected]>
1 parent 82a1935 commit 23d6764

File tree

11 files changed

+171
-34
lines changed

11 files changed

+171
-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
@@ -87,6 +87,11 @@ public Builder(Map<ConfigResource, Config> configs, boolean validateOnly) {
8787
public AlterConfigsRequest build(short version) {
8888
return new AlterConfigsRequest(data, version);
8989
}
90+
91+
@Override
92+
public String toString() {
93+
return maskData(data);
94+
}
9095
}
9196

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

‎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.Readable;
2423

25-
import com.fasterxml.jackson.databind.JsonNode;
26-
import com.fasterxml.jackson.databind.node.ObjectNode;
27-
2824
import java.util.List;
2925
import java.util.Set;
3026
import java.util.stream.Collectors;
@@ -47,7 +43,7 @@ public AlterUserScramCredentialsRequest build(short version) {
4743

4844
@Override
4945
public String toString() {
50-
return data.toString();
46+
return maskData(data);
5147
}
5248
}
5349

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

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

‎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
@@ -127,4 +127,15 @@ public boolean hasError() {
127127
public boolean shouldClientThrottle(short version) {
128128
return version >= 1;
129129
}
130+
131+
// Do not print tokenId and Hmac, overwrite a temp copy of the data with empty content
132+
@Override
133+
public String toString() {
134+
DescribeDelegationTokenResponseData tempData = data.duplicate();
135+
tempData.tokens().forEach(token -> {
136+
token.setTokenId("REDACTED");
137+
token.setHmac(new byte[0]);
138+
});
139+
return tempData.toString();
140+
}
130141
}

‎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.Readable;
2928

30-
import com.fasterxml.jackson.databind.JsonNode;
31-
import com.fasterxml.jackson.databind.node.ObjectNode;
32-
3329
import java.util.Collection;
3430
import java.util.Map;
3531

@@ -76,7 +72,7 @@ public IncrementalAlterConfigsRequest build(short version) {
7672

7773
@Override
7874
public String toString() {
79-
return data.toString();
75+
return maskData(data);
8076
}
8177
}
8278

@@ -112,14 +108,18 @@ public AbstractResponse getErrorResponse(final int throttleTimeMs, final Throwab
112108
}
113109

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

‎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
@@ -64,7 +64,19 @@ public RenewDelegationTokenRequest build(short version) {
6464

6565
@Override
6666
public String toString() {
67-
return data.toString();
67+
return maskData(data);
6868
}
6969
}
70+
71+
private static String maskData(RenewDelegationTokenRequestData data) {
72+
RenewDelegationTokenRequestData tempData = data.duplicate();
73+
tempData.setHmac(new byte[0]);
74+
return tempData.toString();
75+
}
76+
77+
// Do not print Hmac, overwrite a temp copy of the data with empty content
78+
@Override
79+
public String toString() {
80+
return maskData(data);
81+
}
7082
}

‎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
@@ -76,4 +76,12 @@ public static SaslAuthenticateRequest parse(Readable readable, short version) {
7676
return new SaslAuthenticateRequest(new SaslAuthenticateRequestData(readable, version),
7777
version);
7878
}
79+
80+
// Do not print authBytes, overwrite a temp copy of the data with empty bytes
81+
@Override
82+
public String toString() {
83+
SaslAuthenticateRequestData tempData = data.duplicate();
84+
tempData.setAuthBytes(new byte[0]);
85+
return tempData.toString();
86+
}
7987
}

‎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
@@ -79,4 +79,12 @@ public SaslAuthenticateResponseData data() {
7979
public static SaslAuthenticateResponse parse(Readable readable, short version) {
8080
return new SaslAuthenticateResponse(new SaslAuthenticateResponseData(readable, version));
8181
}
82+
83+
// Do not print authBytes, overwrite a temp copy of the data with empty bytes
84+
@Override
85+
public String toString() {
86+
SaslAuthenticateResponseData tempData = data.duplicate();
87+
tempData.setAuthBytes(new byte[0]);
88+
return tempData.toString();
89+
}
8290
}

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

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,7 @@
297297
import static java.util.Collections.emptyList;
298298
import static java.util.Collections.singletonList;
299299
import static org.apache.kafka.common.protocol.ApiKeys.API_VERSIONS;
300+
import static org.apache.kafka.common.protocol.ApiKeys.CREATE_DELEGATION_TOKEN;
300301
import static org.apache.kafka.common.protocol.ApiKeys.CREATE_PARTITIONS;
301302
import static org.apache.kafka.common.protocol.ApiKeys.CREATE_TOPICS;
302303
import static org.apache.kafka.common.protocol.ApiKeys.DELETE_ACLS;
@@ -3073,15 +3074,27 @@ private DescribeConfigsResponse createDescribeConfigsResponse(short version) {
30733074
}
30743075

30753076
private AlterConfigsRequest createAlterConfigsRequest(short version) {
3076-
Map<ConfigResource, AlterConfigsRequest.Config> configs = new HashMap<>();
3077+
Map<ConfigResource, AlterConfigsRequest.Config> configs = new LinkedHashMap<>();
30773078
List<AlterConfigsRequest.ConfigEntry> configEntries = asList(
30783079
new AlterConfigsRequest.ConfigEntry("config_name", "config_value"),
30793080
new AlterConfigsRequest.ConfigEntry("another_name", "another value")
30803081
);
30813082
configs.put(new ConfigResource(ConfigResource.Type.BROKER, "0"), new AlterConfigsRequest.Config(configEntries));
30823083
configs.put(new ConfigResource(ConfigResource.Type.TOPIC, "topic"),
30833084
new AlterConfigsRequest.Config(emptyList()));
3084-
return new AlterConfigsRequest.Builder(configs, false).build(version);
3085+
AlterConfigsRequest alterConfigsRequest = new AlterConfigsRequest.Builder(configs, false).build(version);
3086+
assertEquals(
3087+
"AlterConfigsRequestData(resources=[" +
3088+
"AlterConfigsResource(resourceType=" + ConfigResource.Type.BROKER.id() + ", " +
3089+
"resourceName='0', " +
3090+
"configs=[AlterableConfig(name='config_name', value='REDACTED'), " +
3091+
"AlterableConfig(name='another_name', value='REDACTED')]), " +
3092+
"AlterConfigsResource(resourceType=" + ConfigResource.Type.TOPIC.id() + ", " +
3093+
"resourceName='topic', configs=[])], " +
3094+
"validateOnly=false)",
3095+
alterConfigsRequest.toString()
3096+
);
3097+
return alterConfigsRequest;
30853098
}
30863099

30873100
private AlterConfigsResponse createAlterConfigsResponse() {
@@ -3184,7 +3197,12 @@ private CreateDelegationTokenResponse createCreateTokenResponse() {
31843197
.setMaxTimestampMs(System.currentTimeMillis())
31853198
.setTokenId("token1")
31863199
.setHmac("test".getBytes());
3187-
return new CreateDelegationTokenResponse(data);
3200+
var response = new CreateDelegationTokenResponse(data);
3201+
3202+
String responseStr = response.toString();
3203+
assertTrue(responseStr.contains("tokenId='REDACTED'"));
3204+
assertTrue(responseStr.contains("hmac=[]"));
3205+
return response;
31883206
}
31893207

31903208
private RenewDelegationTokenRequest createRenewTokenRequest(short version) {
@@ -3240,7 +3258,14 @@ private DescribeDelegationTokenResponse createDescribeTokenResponse(short versio
32403258
tokenList.add(new DelegationToken(tokenInfo1, "test".getBytes()));
32413259
tokenList.add(new DelegationToken(tokenInfo2, "test".getBytes()));
32423260

3243-
return new DescribeDelegationTokenResponse(version, 20, Errors.NONE, tokenList);
3261+
var response = new DescribeDelegationTokenResponse(version, 20, Errors.NONE, tokenList);
3262+
3263+
String responseStr = response.toString();
3264+
String[] parts = responseStr.split(",");
3265+
// The 2 token info should both be redacted
3266+
assertEquals(2, Arrays.stream(parts).filter(s -> s.trim().contains("tokenId='REDACTED'")).count());
3267+
assertEquals(2, Arrays.stream(parts).filter(s -> s.trim().contains("hmac=[]")).count());
3268+
return response;
32443269
}
32453270

32463271
private ElectLeadersRequest createElectLeadersRequestNullPartitions() {
@@ -3960,6 +3985,28 @@ public void testInvalidTaggedFieldsWithSaslAuthenticateRequest() {
39603985
assertEquals("Error reading byte array of 32767 byte(s): only 3 byte(s) available", msg);
39613986
}
39623987

3988+
@Test
3989+
public void testSaslAuthenticateRequestResponseToStringMasksSensitiveData() {
3990+
byte[] sensitiveAuthBytes = "sensitive-auth-token-123".getBytes(StandardCharsets.UTF_8);
3991+
SaslAuthenticateRequestData requestData = new SaslAuthenticateRequestData().setAuthBytes(sensitiveAuthBytes);
3992+
SaslAuthenticateRequest request = new SaslAuthenticateRequest(requestData, (short) 2);
3993+
3994+
String requestString = request.toString();
3995+
3996+
// Verify that the authBytes field is present but empty in the output
3997+
assertTrue(requestString.contains("authBytes=[]"),
3998+
"authBytes field should be empty in toString() output");
3999+
4000+
SaslAuthenticateResponseData responseData = new SaslAuthenticateResponseData().setAuthBytes(sensitiveAuthBytes);
4001+
SaslAuthenticateResponse response = new SaslAuthenticateResponse(responseData);
4002+
4003+
String responseString = response.toString();
4004+
4005+
// Verify that the authBytes field is present but empty in the output
4006+
assertTrue(responseString.contains("authBytes=[]"),
4007+
"authBytes field should be empty in toString() output");
4008+
}
4009+
39634010
@Test
39644011
public void testListConfigResourcesRequestV0FailsWithConfigResourceTypeOtherThanClientMetrics() {
39654012
// One type which is not CLIENT_METRICS

0 commit comments

Comments
 (0)