Bug: IBBInputStream does not fullfil its contract

public abstract class **InputStream**

 ...
public abstract int **read**()                  throws IOException

Reads the next byte of data from the input stream. The value byte is returned as an int in the range 0 to 255.

But the IBBInputStream read method returns:

// return byte and increment buffer pointer

return (int) buffer[bufferPointer++];

Simple fix:

return (buffer[bufferPointer++] &0xFF);

1 Like

Results of this violation (a Socks5ByteStream session works well)

ERROR 05:30:21,724 [pool-2-thread-1] (ByteStreamTransport.java:79) could not accept request for IBB bytestream connection from *: null

java.io.EOFException

at java.io.DataInputStream.readUnsignedByte(Unknown Source)

at *.initClientEncryption(ByteStreamConnection.java:517)

at *.start(ByteStreamConnection.java:181)

at *.connectionRequest(Router.java:126)

at *.incomingBytestreamRequest(ByteStreamTransport.java:69)

at org.jivesoftware.smackx.bytestreams.ibb.InitiationListener.processRequest(Initi ationListener.java:99)

at org.jivesoftware.smackx.bytestreams.ibb.InitiationListener.access$000(Initiatio nListener.java:41)

at org.jivesoftware.smackx.bytestreams.ibb.InitiationListener$1.run(InitiationList ener.java:67)

at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)

at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)

at java.lang.Thread.run(Unknown Source)

I took a quick look at the source: buffer is declared as byte array, so what’s the gain in masking the higher bytes with & 0xff?

The InBandBytestream class doesn’t appear in the stacktrace you posted. Not sure if I understand the problem correct. Maybe you could elaborate the bug a little bit more for me.

The return value is of type integer. The cast in this code line is not even necessary because the JVM will already do this.

Problem: when converting a byte to an int in a JVM, the sign bit will taken into account.

So byte value 0xFF will become 0xFFFFFFFF

In short, this method will return a negative integer value for all byte values greater 0x7F and so violating the contract that states that is returns an integer value between 0 and 255 OR -1 if the end of the stream had reached.

The InBandBytestream does not appear in the stacktrace because it returns a negative value, but it seems that the method DataInputStream.readUnsignedByte checks for x < 0 instead of x == - 1 which is also a valid check.

Logged as SMACK-394

Thanks for the explanation.