tyrus
  1. tyrus
  2. TYRUS-216

Incorrect use of ByteBuffer.array() in sendBinary(ByteBuffer)

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.2
    • Component/s: None
    • Labels:
      None

      Description

      In org.glassfish.tyrus.core.TyrusRemoteEndpoint#sendBinary(ByteBuffer byteBuffer)

      The use of ByteBuffer.array() is incorrect use of a ByteBuffer.

      That will obtain the entire underlying array for the byte buffer, regardless of the position and limit that the ByteBuffer currently has.

      Example:

      final Charset UTF8 = Charset.forName("UTF-8");
      ByteBuffer buf = ByteBuffer.allocate(100);
      buf.put("Hello World".getBytes(UTF8));

      The ByteBuffer at this point has the following structure ...

      HeapByteBuffer@29578426[p=11,l=100,c=100,r=89]=

      {Hello World<<<�����������������...���������������>>>}

      The key to read this:
      p = position
      l = limit
      c = capacity
      r = remaining
      <<<_>>> what is in the brackets is what is active and visible from the point of view of the ByteBuffer at this moment.

      Which means that there is 89 bytes of remaining content to write in this buffer.
      This ByteBuffer hasn't been flipped yet and been made available for writing.

      If we change the example to be:

      final Charset UTF8 = Charset.forName("UTF-8");
      ByteBuffer buf = ByteBuffer.allocate(100);
      buf.put("Hello World".getBytes(UTF8));
      buf.flip();

      We can see that this ByteBuffer has been flipped, and at this point has the following structure ...

      HeapByteBuffer@29578426[p=0,l=11,c=100,r=11]=

      {<<<Hello World>>>�����������������...���������������}

      This has 11 bytes to write, with the position and limit set to only include the "Hello World" bytes.

      With the use of ByteBuffer.array() you will ALWAYS get the 100 bytes of the allocated buffer, even though only 11 bytes should be written.

        Activity

        joakimerdfelt created issue -
        Hide
        joakimerdfelt added a comment - - edited

        Proper access of the underlying, and active, array on a ByteBuffer.

        public byte[] getActiveArray(ByteBuffer buffer)
        {
          byte[] ret = new byte[buffer.remaining()];
          if (buffer.hasArray())
          {
              byte[] array = buffer.array();
              System.arraycopy(array, buffer.arrayOffset() + buffer.position(), ret, 0, ret.length);
          }
          else
          {
              buffer.slice().get(ret);
          }
          return ret;
        }
        
        Show
        joakimerdfelt added a comment - - edited Proper access of the underlying, and active, array on a ByteBuffer. public byte[] getActiveArray(ByteBuffer buffer) { byte[] ret = new byte[buffer.remaining()]; if (buffer.hasArray()) { byte[] array = buffer.array(); System.arraycopy(array, buffer.arrayOffset() + buffer.position(), ret, 0, ret.length); } else { buffer.slice().get(ret); } return ret; }
        Hide
        joakimerdfelt added a comment -

        Also, not all ByteBuffer's have arrays. (note the use of .hasArray() in prior comment)

        Lets say we hand off a Read Only ByteBuffer or a direct ByteBuffer to the .sendBinary(ByteBuffer buffer), the existing implementation will fail.

            final Charset UTF8 = Charset.forName("UTF-8");
            ByteBuffer buf = ByteBuffer.allocate(100);
            buf.put("Hello World".getBytes(UTF8));
            buf.flip();
            ByteBuffer ro = buf.asReadOnlyBuffer();
            
            endpoint.sendBinary(ro); <-- throws java.nio.ReadOnlyBufferException
        

        also

            final Charset UTF8 = Charset.forName("UTF-8");
            ByteBuffer buf = ByteBuffer.allocateDirect(100);
            buf.put("Hello World".getBytes(UTF8));
            buf.flip();
        
            endpoint.sendBinary(buf); <-- throws java.lang.UnsupportedOperationException
                                            at java.nio.ByteBuffer.array(ByteBuffer.java:959)
        
        Show
        joakimerdfelt added a comment - Also, not all ByteBuffer's have arrays. (note the use of .hasArray() in prior comment) Lets say we hand off a Read Only ByteBuffer or a direct ByteBuffer to the .sendBinary(ByteBuffer buffer), the existing implementation will fail. final Charset UTF8 = Charset.forName("UTF-8"); ByteBuffer buf = ByteBuffer.allocate(100); buf.put("Hello World".getBytes(UTF8)); buf.flip(); ByteBuffer ro = buf.asReadOnlyBuffer(); endpoint.sendBinary(ro); <-- throws java.nio.ReadOnlyBufferException also final Charset UTF8 = Charset.forName("UTF-8"); ByteBuffer buf = ByteBuffer.allocateDirect(100); buf.put("Hello World".getBytes(UTF8)); buf.flip(); endpoint.sendBinary(buf); <-- throws java.lang.UnsupportedOperationException at java.nio.ByteBuffer.array(ByteBuffer.java:959)
        stepan.kopriva made changes -
        Field Original Value New Value
        Assignee stepan.kopriva [ stepan.kopriva ]
        stepan.kopriva made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 1.2 [ 16550 ]
        Resolution Fixed [ 1 ]

          People

          • Assignee:
            stepan.kopriva
            Reporter:
            joakimerdfelt
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: