AbstractTcpListener, incorrect length computations about ice4j HOT 6 CLOSED

This is just wrong, len-off does not represent anything useful. Please elaborate on why you think the correction is needed if you disagree.

from ice4j.

The current constraints are inaccurate, which can be illustrated with, for instance, len == 0 and off == 0.

len - off represents how many bytes of buf can be read from position off without IndexOutOfBoundsException. I assume here that 0 <= off <= len.

In the first case (from line 130), buf[j] is read with with: off <= j < off + googleTurnSslTcp.length; thus the following constraint must be ensured: off + googleTurnSslTcp.length <= buf.length == len, which is len - off >= googleTurnSslTcp.length.

In the second case (from line 158) buf[j] is read with: off + 6 <= j < off + 6 + magicCookie.length == off + 10; thus the following constraint must be ensured: off + 10 <= buf.length == len, which is len - off >= 10.

If one of those two constraints is violated, an IndexOutOfBoundsException will be thrown.

Note that len - off >= 10 is preferred over len >= 10 + off, because the 10 + off may wrap around and thus be negative, while len - off is alway positive and smaller than len.

from ice4j.

@bgrozev You may consider re-opining this issue.

from ice4j.

@bgrozev You may consider re-opining this issue.

Feel free to re-open these yourself if you think it's necessary. We do appreciate the feedback!

from ice4j.

len - off represents how many bytes of buf can be read from position off without IndexOutOfBoundsException

This is not true. The number of bytes that can be read without an IndexOutOfBoundsException depends only the "length" field of the byte[]. The number of bytes represented by a (byte[], int off, int len) triplet or a DatagramPacket or a RawPacket in libjitsi are given simply by "len" (or DatagramPacket.getLength() or RawPacket.getLength()).

You can verify this for example with the following code:

byte[] buf = new byte[]{0, 1, 2, 3, 4, 5, 6};
 DatagramPacket p = new DatagramPacket(buf, 2, 3);
 DatagramSocket s = new DatagramSocket(12345);
 p.setAddress(InetAddress.getByName("1.2.3.4"));
 p.setPort(12345);
 s.send(p);

The packet represents a buffer with 3 bytes, starting at offset 2, and you can observe a UDP packet being sent with payload of 0x020304.

from ice4j.

Now I think you are right.

But https://docs.oracle.com/javase/7/docs/api/java/net/DatagramPacket.html is still unclear to me.

public DatagramPacket(byte[] buf, int offset, int length) says:

The length argument must be less than or equal to buf.length.

but offset is not even considered, which I feel wrong.

public void setLength(int length) says:

The length must be lesser or equal to the offset plus the length of the packet's buffer.

but the stated length <= offset + buf.length seems wrong to me; instead, length + offset <= buf.length (according to your argument) makes definitively more sense to me.

However, according to http://developer.classpath.org/doc/java/net/DatagramPacket-source.html:

 380:   public synchronized void setLength(int length)
 381:   {
 382:     if (length < 0)
 383:       throw new IllegalArgumentException("Invalid length: " + length);
 384:     if (offset + length > buffer.length)
 385:       throw new IllegalArgumentException("Potential buffer overflow - offset: "
 386:                                          + offset + " length: " + length);
...

the constraint is really offset + length <= buffer.length, as you claim.
Thus this ticket is invalid, indeed.

from ice4j.