[ZOOKEEPER-2369] Flushing DataOutputStream before calling toByteArray on the underlying ByteArrayOutputStream

• Type: Bug
• Status: Open
• Priority: Minor
• Resolution: Unresolved
• Flags: Patch
• Assignee: emopers
• Reporter: emopers
• Created: 18/Feb/16 09:13
• Updated: 04/Mar/16 00:14

Description

In ./src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java

            ByteArrayOutputStream baos = new ByteArrayOutputStream();
            BinaryOutputArchive bos = BinaryOutputArchive.getArchive(baos);
            bos.writeInt(-1, "len");
            rsp.serialize(bos, "connect");
            if (!cnxn.isOldClient) {
                bos.writeBool(
                        this instanceof ReadOnlyZooKeeperServer, "readOnly");
            }
            baos.close();
            ByteBuffer bb = ByteBuffer.wrap(baos.toByteArray());

BinaryOutputArchive internally uses DataOutputStream as its stream, and when a DataOutputStream instance wraps an underlying ByteArrayOutputStream instance,
it is recommended to flush or close the DataOutputStream before invoking the underlying instances's toByteArray() . Also, it is a good practice to call flush/close explicitly as mentioned for example http://stackoverflow.com/questions/2984538/how-to-use-bytearrayoutputstream-and-dataoutputstream-simultaneously-java.
Moreover, "baos.close()" at second last line is not required as it is no-op according to javadoc

Closing a ByteArrayOutputStream has no effect. The methods in this class can be called after the stream has been closed without generating an IOException.

The patch is to add flush method on "bos" before calling toByteArray on "baos". Similar behavior is also present in the following files:
./src/java/main/org/apache/zookeeper/ClientCnxn.java
./src/java/main/org/apache/zookeeper/server/ZKDatabase.java
./src/java/main/org/apache/zookeeper/server/persistence/Util.java
./src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java

Let me know if this looks good. I can provide patch.