ZOOKEEPER-1513. "Unreasonable length" exception while starting a server (Skye W-M via phunt)

git-svn-id: https://svn.apache.org/repos/asf/zookeeper/trunk@1420538 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -276,6 +276,9 @@ BUGFIXES:
   ZOOKEEPER-1596. Zab1_0Test should ensure that the file is closed
   (Enis Soztutar via phunt)
 
+  ZOOKEEPER-1513. "Unreasonable length" exception while starting a
+  server (Skye W-M via phunt)
+
 IMPROVEMENTS:
 
   ZOOKEEPER-1170. Fix compiler (eclipse) warnings: unused imports,

build.xml

@@ -67,6 +67,7 @@
     <property name="test.log.dir" value="${test.java.build.dir}/logs" />
     <property name="test.data.dir" value="${test.java.build.dir}/data" />
     <property name="test.data.invalid.dir" value="${test.data.dir}/invalidsnap" />
+    <property name="test.data.buffersize.dir" value="${test.data.dir}/buffersize" />
     <property name="test.cppunit.dir" value="${test.java.build.dir}/test-cppunit"/>
     <property name="test.tmp.dir" value="${test.java.build.dir}/tmp" />
     <property name="test.output" value="no" />
@@ -1144,6 +1145,7 @@
         <delete dir="${test.log.dir}" />
         <delete dir="${test.tmp.dir}" />
         <delete dir="${test.data.invalid.dir}" />
+        <delete dir="${test.data.buffersize.dir}" />
         <delete dir="${test.data.dir}" />
         <mkdir dir="${test.log.dir}" />
         <mkdir dir="${test.tmp.dir}" />
@@ -1152,6 +1154,10 @@
         <copy todir="${test.data.invalid.dir}">
             <fileset dir="${basedir}/src/java/test/data/invalidsnap"/>
         </copy>
+        <mkdir dir="${test.data.buffersize.dir}" />
+        <copy todir="${test.data.buffersize.dir}">
+            <fileset dir="${basedir}/src/java/test/data/buffersize"/>
+        </copy>
     </target>
 
     <condition property="quicktest">

src/java/main/org/apache/jute/BinaryInputArchive.java

@@ -83,20 +83,15 @@ public class BinaryInputArchive implements InputArchive {
     	return new String(b, "UTF8");
     }
     
-    static public final int maxBuffer = determineMaxBuffer();
-    private static int determineMaxBuffer() {
-        String maxBufferString = System.getProperty("jute.maxbuffer");
-        try {
-            return Integer.parseInt(maxBufferString);
-        } catch(Exception e) {
-            return 0xfffff;
-        }
-        
-    }
+    static public final int maxBuffer = Integer.getInteger("jute.maxbuffer", 0xfffff);
+
     public byte[] readBuffer(String tag) throws IOException {
         int len = readInt(tag);
         if (len == -1) return null;
-        if (len < 0 || len > maxBuffer) {
+        // Since this is a rough sanity check, add some padding to maxBuffer to
+        // make up for extra fields, etc. (otherwise e.g. clients may be able to
+        // write buffers larger than we can read from disk!)
+        if (len < 0 || len > maxBuffer + 1024) {
             throw new IOException("Unreasonable length = " + len);
         }
         byte[] arr = new byte[len];

src/java/test/org/apache/zookeeper/test/BufferSizeTest.java

@@ -0,0 +1,132 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import java.io.File;
+import java.io.IOException;
+
+import org.apache.jute.BinaryInputArchive;
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.ZooKeeper;
+import org.junit.Before;
+import org.junit.Test;
+
+public class BufferSizeTest extends ClientBase {
+    public static final int TEST_MAXBUFFER = 100;
+    private static final File TEST_DATA = new File(
+            System.getProperty("test.data.dir", "build/test/data"),
+            "buffersize");
+    
+    private ZooKeeper zk;
+
+    @Before
+    public void setMaxBuffer() throws IOException, InterruptedException {
+        System.setProperty("jute.maxbuffer", "" + TEST_MAXBUFFER);
+        assertEquals("Can't set jute.maxbuffer!", TEST_MAXBUFFER, BinaryInputArchive.maxBuffer);
+        zk = createClient();
+    }
+    
+    @Test
+    public void testCreatesReqs() throws Exception {
+        testRequests(new ClientOp() {
+            @Override
+            public void execute(byte[] data) throws Exception {
+                zk.create("/create_test", data, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT_SEQUENTIAL);
+            }
+        });
+    }
+    
+    @Test
+    public void testSetReqs() throws Exception {
+        final String path = "/set_test";
+        zk.create(path, new byte[1], Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
+        testRequests(new ClientOp() {
+            @Override
+            public void execute(byte[] data) throws Exception {
+                zk.setData(path, data, -1);
+            }
+        });
+    }
+    
+    /** Issues requests containing data smaller, equal, and greater than TEST_MAXBUFFER. */
+    private void testRequests(ClientOp clientOp) throws Exception {
+        clientOp.execute(new byte[TEST_MAXBUFFER - 60]);
+        try {
+            // This should fail since the buffer size > the data size due to extra fields
+            clientOp.execute(new byte[TEST_MAXBUFFER]);
+            fail("Request exceeding jute.maxbuffer succeeded!");
+        } catch (KeeperException.ConnectionLossException e) {}
+        try {
+            clientOp.execute(new byte[TEST_MAXBUFFER + 10]);
+            fail("Request exceeding jute.maxbuffer succeeded!");
+        } catch (KeeperException.ConnectionLossException e) {}
+    }
+
+    private interface ClientOp {
+        void execute(byte[] data) throws Exception;
+    }
+
+    @Test
+    public void testStartup() throws Exception {
+        final String path = "/test_node";
+        zk.create(path, new byte[TEST_MAXBUFFER - 60], Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
+        zk.setData(path, new byte[TEST_MAXBUFFER - 50], -1);
+
+        stopServer();
+        startServer();
+    }
+    
+    @Test
+    public void testStartupFailureCreate() throws Exception {
+        // Empty snapshot and logfile containing a 5000-byte create
+        testStartupFailure(new File(TEST_DATA, "create"),
+                "Server started despite create exceeding jute.maxbuffer!");
+    }
+    
+    @Test
+    public void testStartupFailureSet() throws Exception {
+        // Empty snapshot and logfile containing a 1-byte create and 5000-byte set
+        testStartupFailure(new File(TEST_DATA, "set"),
+                "Server started despite set exceeding jute.maxbuffer!");
+    }
+    
+    @Test
+    public void testStartupFailureSnapshot() throws Exception {
+        // Snapshot containing 5000-byte znode and logfile containing create txn
+        testStartupFailure(new File(TEST_DATA, "snapshot"),
+                "Server started despite znode exceeding jute.maxbuffer!");
+    }
+    
+    private void testStartupFailure(File testDir, String failureMsg) throws Exception {
+        stopServer();
+        // Point server at testDir
+        tmpDir = testDir;
+        try {
+            startServer();
+            fail(failureMsg);
+        } catch (IOException e) {
+            LOG.info("Successfully caught IOException: " + e);
+        }
+    }
+}