[ 
https://issues.apache.org/jira/browse/DIRMINA-664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12677237#action_12677237
 ] 

David Rosenstrauch commented on DIRMINA-664:
--------------------------------------------

That's not going to fix it.  It doesn't matter if you declare the constant(s) 
as private.  The fact you're handing out the same (mutable) "constant" buffer 
repeatedly is the problem.  Someone can modify the object, and then it's no 
longer an empty buffer.

Take, for example, the case where someone calls 
IoBuffer.allocate(0).setAutoExpand(true).  As per the code:

     public static IoBuffer allocate(int capacity, boolean direct) {
        if (capacity == 0) {
            return direct ? EMPTY_DIRECT_BUFFER : EMPTY_HEAP_BUFFER;
        }

... it will return either EMPTY_DIRECT_BUFFER or EMPTY_HEAP_BUFFER.  Both of 
those are private fields.  But it doesn't matter.  If I then set that 
"constant" "empty" buffer that was returned to me to be expandable and write to 
it, then any subsequent calls to IoBuffer.allocate(0) will return that buffer - 
which is no longer an empty buffer.

So I don't think you have any choice but to get rid of the static constants.

As far as the expandable functionality, just my IMO, but I'd request that you 
not remove that as it's extremely useful.  It can often be difficult to know 
ahead of time exactly how large of a buffer you're going to need (e.g., if the 
data can vary in length), and stepping through your object graph to compute the 
size needed might require almost as much coding and CPU cycles as it would take 
to write the object graph to the buffer in the first place.  So it's extremely 
useful to have an expandable buffer.

> EMPTY_* IoBuffer constants can be made mutable and cause data errors
> --------------------------------------------------------------------
>
>                 Key: DIRMINA-664
>                 URL: https://issues.apache.org/jira/browse/DIRMINA-664
>             Project: MINA
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 2.0.0-M4
>         Environment: All?
>            Reporter: David Rosenstrauch
>            Priority: Minor
>
> The EMPTY_* constants in the IoBuffer class can be made mutable (by using the 
> setAutoExpand(true) method call) which can result in those constant buffers 
> no longer being empty.  This can obviously cause a multitude of data errors.
> See this JUnit test case for an example:
> import junit.framework.TestCase;
> import org.apache.mina.core.buffer.IoBuffer;
> public class TestIoBuffer extends TestCase {
>       public void testIoBufferAllocate() {
>               IoBuffer buf = IoBuffer.allocate(0).setAutoExpand(true);
>               buf.putInt(1234);
>               buf.flip();
>               buf = IoBuffer.allocate(0);
>               assertEquals(0, buf.remaining());
>       }
>       public void testEmptyIoBuffer() {
>               IoBuffer buf = IoBuffer.EMPTY_BUFFER.setAutoExpand(true);
>               buf.putInt(1234);
>               buf.flip();
>               buf = IoBuffer.EMPTY_BUFFER;
>               assertEquals(0, buf.remaining());
>       }
> }

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to