Small refactoring to make safety clearer (#8083)

Author: trask

api/all/src/main/java/io/opentelemetry/api/common/JsonEncoding.java

@@ -37,7 +37,7 @@ static void append(StringBuilder sb, Value<?> value) {
         appendMap(sb, (List<KeyValue>) value.getValue());
         break;
       case BYTES:
-        appendBytes(sb, (ByteBuffer) value.getValue());
+        appendBytes(sb, (Value<ByteBuffer>) value);
         break;
       case EMPTY:
         sb.append("null");
@@ -97,9 +97,11 @@ private static void appendDouble(StringBuilder sb, double value) {
     }
   }
 
-  private static void appendBytes(StringBuilder sb, ByteBuffer value) {
-    byte[] bytes = new byte[value.remaining()];
-    value.get(bytes);
+  private static void appendBytes(StringBuilder sb, Value<ByteBuffer> value) {
+    ByteBuffer buf = value.getValue();
+    byte[] bytes = new byte[buf.remaining()];
+    // getValue() above returns a new ByteBuffer, so mutating its position here is safe
+    buf.get(bytes);
     sb.append('"').append(Base64.getEncoder().encodeToString(bytes)).append('"');
   }
 

exporters/otlp/common/src/main/java/io/opentelemetry/exporter/internal/otlp/AnyValueMarshaler.java

@@ -37,7 +37,7 @@ public static MarshalerWithSize create(Value<?> value) {
       case KEY_VALUE_LIST:
         return KeyValueListAnyValueMarshaler.create((List<KeyValue>) value.getValue());
       case BYTES:
-        return BytesAnyValueMarshaler.create((ByteBuffer) value.getValue());
+        return BytesAnyValueMarshaler.create((Value<ByteBuffer>) value);
       case EMPTY:
         return EmptyAnyValueMarshaler.INSTANCE;
     }

exporters/otlp/common/src/main/java/io/opentelemetry/exporter/internal/otlp/AnyValueStatelessMarshaler.java

@@ -64,7 +64,7 @@ public void writeTo(Serializer output, Value<?> value, MarshalerContext context)
         return;
       case BYTES:
         BytesAnyValueStatelessMarshaler.INSTANCE.writeTo(
-            output, (ByteBuffer) value.getValue(), context);
+            output, (Value<ByteBuffer>) value, context);
         return;
       case EMPTY:
         // no field to write
@@ -105,7 +105,7 @@ public int getBinarySerializedSize(Value<?> value, MarshalerContext context) {
             context);
       case BYTES:
         return BytesAnyValueStatelessMarshaler.INSTANCE.getBinarySerializedSize(
-            (ByteBuffer) value.getValue(), context);
+            (Value<ByteBuffer>) value, context);
       case EMPTY:
         return 0;
     }

exporters/otlp/common/src/main/java/io/opentelemetry/exporter/internal/otlp/BytesAnyValueMarshaler.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.exporter.internal.otlp;
 
+import io.opentelemetry.api.common.Value;
 import io.opentelemetry.exporter.internal.marshal.CodedOutputStream;
 import io.opentelemetry.exporter.internal.marshal.MarshalerWithSize;
 import io.opentelemetry.exporter.internal.marshal.Serializer;
@@ -21,9 +22,11 @@ private BytesAnyValueMarshaler(byte[] value) {
     this.value = value;
   }
 
-  static MarshalerWithSize create(ByteBuffer value) {
-    byte[] bytes = new byte[value.remaining()];
-    value.get(bytes);
+  static MarshalerWithSize create(Value<ByteBuffer> value) {
+    ByteBuffer buf = value.getValue();
+    byte[] bytes = new byte[buf.remaining()];
+    // getValue() above returns a new ByteBuffer, so mutating its position here is safe
+    buf.get(bytes);
     return new BytesAnyValueMarshaler(bytes);
   }
 

exporters/otlp/common/src/main/java/io/opentelemetry/exporter/internal/otlp/BytesAnyValueStatelessMarshaler.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.exporter.internal.otlp;
 
+import io.opentelemetry.api.common.Value;
 import io.opentelemetry.exporter.internal.marshal.CodedOutputStream;
 import io.opentelemetry.exporter.internal.marshal.MarshalerContext;
 import io.opentelemetry.exporter.internal.marshal.Serializer;
@@ -14,22 +15,24 @@
 import java.nio.ByteBuffer;
 
 /** See {@link BytesAnyValueMarshaler}. */
-final class BytesAnyValueStatelessMarshaler implements StatelessMarshaler<ByteBuffer> {
+final class BytesAnyValueStatelessMarshaler implements StatelessMarshaler<Value<ByteBuffer>> {
   static final BytesAnyValueStatelessMarshaler INSTANCE = new BytesAnyValueStatelessMarshaler();
 
   private BytesAnyValueStatelessMarshaler() {}
 
   @Override
-  public void writeTo(Serializer output, ByteBuffer value, MarshalerContext context)
+  public void writeTo(Serializer output, Value<ByteBuffer> value, MarshalerContext context)
       throws IOException {
     byte[] bytes = context.getData(byte[].class);
     output.writeBytes(AnyValue.BYTES_VALUE, bytes);
   }
 
   @Override
-  public int getBinarySerializedSize(ByteBuffer value, MarshalerContext context) {
-    byte[] bytes = new byte[value.remaining()];
-    value.get(bytes);
+  public int getBinarySerializedSize(Value<ByteBuffer> value, MarshalerContext context) {
+    ByteBuffer buf = value.getValue();
+    byte[] bytes = new byte[buf.remaining()];
+    // getValue() above returns a new ByteBuffer, so mutating its position here is safe
+    buf.get(bytes);
     context.addData(bytes);
     return AnyValue.BYTES_VALUE.getTagSize() + CodedOutputStream.computeByteArraySizeNoTag(bytes);
   }