Garret Wilson created THRIFT-4856:
-------------------------------------
Summary: Java protocol implementation ByteBuffer usage makes
assumptions.
Key: THRIFT-4856
URL: https://issues.apache.org/jira/browse/THRIFT-4856
Project: Thrift
Issue Type: Bug
Components: Java - Library
Affects Versions: 0.12.0
Reporter: Garret Wilson
In all the Java concrete implementations of {{TProtocol.writeBinary(ByteBuffer
buf)}}, the code makes assumptions about the state of the byte buffer. In other
words, it only works because relies on assumptions about the implementation of
the code calling it. The code could break at any time.
All the Java implementations of {{TProtocol.writeBinary(ByteBuffer buf)}} look
something like this (example from {{TBinaryProtocol}}):
{code}
public void writeBinary(ByteBuffer bin) throws TException {
int length = bin.limit() - bin.position();
writeI32(length);
trans_.write(bin.array(), bin.position() + bin.arrayOffset(), length);
}
{code}
Note that the code blindly grabs {{bin.array()}}. But the API documentation for
[ByteBuffer.array()|https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/nio/ByteBuffer.html#array()]
clearly states that this is an optional operation, and even says:
{quote}
Invoke the
[hasArray|https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/nio/ByteBuffer.html#hasArray()]
method before invoking this method in order to ensure that this buffer has an
accessible backing array.
{quote}
This is explained on Stack Overflow at [ByteBuffer to byte
array|https://stackoverflow.com/q/28940174/421049], [Gets byte array from a
ByteBuffer in java|https://stackoverflow.com/q/679298/421049], and [Convert
ByteBuffer to byte array java|https://stackoverflow.com/q/28744096/421049],
among others.
To fix this you'll first need to call
[ByteBuffer.hasArray()|https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/nio/ByteBuffer.html#hasArray()]
to see if the byte buffer even has a backing array, and if not, you'll need to
copy the bytes as explained at the Stack Overflow links above.
You might say, "but {{TProtocol.writeBinary(ByteBuffer buf)}} requires that the
caller only send byte buffers that are backed by arrays". Fine, but that's not
what the contract to {{TProtocol.writeBinary(ByteBuffer buf)}} says. In fact …
it doesn't say anything!! Here is the definition in {{TProtocol}}
{code}
public abstract void writeBinary(ByteBuffer buf) throws TException;
{code}
If you want to keep the implementations you have and require that the byte
buffer always be backed by an array, that needs to explicitly be added to the
method contract, e.g.:
{code}
/**
* Writes a Thrift binary value to the underlying transport.
* <p>This method requires that the provided buffer be backed by an array;
* that is, <code>buf.hasArray()</code> returns <code>true</code>.</p>
* @param buf The byte buffer containing the bytes to write.
* @throws TException if there is an error writing the bytes.
*/
public abstract void writeBinary(ByteBuffer buf) throws TException;
{code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
