Skip to content

There is a problem with the implementation of the tool function in Extensions.kt #2083

@IcarusL

Description

@IcarusL

Problem

The current ByteBuffer.toByteArray() implementation is unsafe for sliced or duplicated ByteBuffers:

fun ByteBuffer.toByteArray(): ByteArray {
    return if (this.hasArray() && !isDirect) {
        this.array()
    } else {
        this.rewind()
        val byteArray = ByteArray(this.remaining())
        this.get(byteArray)
        byteArray
    }
}

fun ByteBuffer.clone(): ByteBuffer = ByteBuffer.wrap(toByteArray())

For heap buffers, array() returns the original backing array, not the visible range of the current ByteBuffer.

This is a problem because slice() and duplicate() are shallow views. They share the same underlying array, and slice() only changes the internal offset/position/limit.

Example:

val buffer = ByteBuffer.allocate(10)
for (i in 0 until buffer.limit()) {
    buffer.put(i.toByte())
}

val slice = buffer.slice(3, 6)

println("slice[0]: ${slice.get(0)}")
// slice[0]: 3

val array = slice.array()

println("slice array size: ${array.size}, slice array index 0: ${array[0]}")
// slice array size: 10, slice array index 0: 0

println("slice array offset: ${slice.arrayOffset()}")
// slice array offset: 3

array[3] = 100

println("buffer[3] = ${buffer.get(3)}, slice[0] = ${slice.get(0)}")
// buffer[3] = 100, slice[0] = 100

This shows that slice.array() returns the original shared array. The sliced data starts at arrayOffset(), not at index 0.

Because of this, the current toByteArray() does not always return the current buffer content. It may return:

  • the full original backing array
  • data outside the current slice range
  • a shared mutable array instead of a deep copy

As a result, the current clone() implementation is also misleading. It looks like a deep copy, but for heap buffers it may just wrap the same shared backing array again.

Expected behavior

toByteArray() should always return a real copy of the requested ByteBuffer range.

It should not:

  • return the backing array directly
  • depend on arrayOffset()
  • modify the original buffer position/limit
  • share mutable data with the original buffer

Suggested fix

Use a duplicated buffer and copy the required range explicitly:

fun ByteBuffer.toByteArray(
    position: Int = 0,
    size: Int = limit()
): ByteArray {
    val duplicate = duplicate()
    duplicate.position(position)
    duplicate.limit(position + size)

    val byteArray = ByteArray(duplicate.remaining())
    duplicate.get(byteArray)

    return byteArray
}

This makes the behavior clear:

  • position is the start position to copy from
  • size is the number of bytes to copy
  • the returned ByteArray is always a deep copy
  • the original ByteBuffer state is not changed

Related issue in removeInfo

There is also a range handling issue here:

fun ByteBuffer.removeInfo(info: MediaFrame.Info): ByteBuffer {
    try {
        position(info.offset)
        limit(info.size)
        // According to the parameter semantics,
        // this should be limit(info.offset + info.size)
    } catch (_: Exception) { }
    return slice()
}

If info.offset is the start offset and info.size is the data length, then limit(info.size) is incorrect.

The correct range should be:

position(info.offset)
limit(info.offset + info.size)

For example:

offset = 10
size = 100

Expected range:

[10, 110)

Current range:

[10, 100)

This drops valid data. In some cases, when offset > size, it may even create an invalid position/limit range.

Summary

The current implementation mixes up three different concepts:

  • the backing array of a ByteBuffer
  • the visible range of a ByteBuffer
  • the offset + size range from MediaFrame.Info

This can produce incorrect frame data, especially when the input buffer is sliced or when MediaFrame.Info.offset is not zero.

We should update the buffer copy logic to always perform an explicit deep copy of the intended range, and update removeInfo() to use offset + size as the limit.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions