rhauch commented on a change in pull request #9950:
URL: https://github.com/apache/kafka/pull/9950#discussion_r580584221



##########
File path: 
connect/transforms/src/main/java/org/apache/kafka/connect/transforms/Cast.java
##########
@@ -364,11 +365,24 @@ private static String castToString(Object value) {
         if (value instanceof java.util.Date) {
             java.util.Date dateValue = (java.util.Date) value;
             return Values.dateFormatFor(dateValue).format(dateValue);
+        } else if (value instanceof ByteBuffer) {
+            ByteBuffer byteBuffer = (ByteBuffer) value;
+            return castByteArrayToString(byteBuffer.array());
+        } else if (value instanceof byte[]) {
+            return castByteArrayToString((byte[]) value);
         } else {
             return value.toString();
         }
     }
 
+    private static String castByteArrayToString(byte[] array) {
+        StringBuilder sbuf = new StringBuilder();
+        for (byte b : array) {
+            sbuf.append(String.format("%02X", b));

Review comment:
       As noted in my previous comment, I agree with @kkonstantine that base64 
would be preferable. Doing that would align better with the existing `Values` 
class used in Connect's header converter mechanism.
   
   If we want to add support for multiple encodings, we would need to have a 
KIP since it would likely mean changing the SMT configuration. 

##########
File path: 
connect/transforms/src/main/java/org/apache/kafka/connect/transforms/Cast.java
##########
@@ -364,11 +365,31 @@ private static String castToString(Object value) {
         if (value instanceof java.util.Date) {
             java.util.Date dateValue = (java.util.Date) value;
             return Values.dateFormatFor(dateValue).format(dateValue);
+        } else if (value instanceof ByteBuffer) {
+            ByteBuffer byteBuffer = (ByteBuffer) value;
+            if (byteBuffer.hasArray()) {
+                return castByteArrayToString(byteBuffer.array());
+            }
+            else {
+                byte[] array = new byte[byteBuffer.remaining()];
+                byteBuffer.get(array);
+                return castByteArrayToString(array);
+            }

Review comment:
       The use of `ByteBuffer.get(...)` here does not account for the fact that 
it may not be positioned at the beginning. Kafka has two `Utils.readBytes(...)` 
methods that we should probably use. This code would then simplify to:
   ```suggestion
               byte[] rawBytes = Utils.readBytes(byteBuffer);
               return castByteArrayToString(rawBytes);
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to