git commit: ApiUtils#writeShortString uses String length instead of byte length; kafka-680; patched by David Arthur; reviewed by Jun Rao

[ju...@apache.org]

Updated Branches:
  refs/heads/0.8 6cf46e7ca -> 2f4bfc645


ApiUtils#writeShortString uses String length instead of byte length; kafka-680; patched by David Arthur; reviewed by Jun Rao


Project: http://git-wip-us.apache.org/repos/asf/kafka/repo
Commit: http://git-wip-us.apache.org/repos/asf/kafka/commit/2f4bfc64
Tree: http://git-wip-us.apache.org/repos/asf/kafka/tree/2f4bfc64
Diff: http://git-wip-us.apache.org/repos/asf/kafka/diff/2f4bfc64

Branch: refs/heads/0.8
Commit: 2f4bfc645a597892c3b0a8dfe722158f3b993874
Parents: 6cf46e7
Author: Jun Rao <ju...@gmail.com>
Authored: Fri Jan 4 13:39:07 2013 -0800
Committer: Jun Rao <ju...@gmail.com>
Committed: Fri Jan 4 13:39:07 2013 -0800

----------------------------------------------------------------------
 core/src/main/scala/kafka/api/ApiUtils.scala       |   13 ++-
 .../test/scala/unit/kafka/api/ApiUtilsTest.scala   |   84 +++++++++++++++
 2 files changed, 92 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kafka/blob/2f4bfc64/core/src/main/scala/kafka/api/ApiUtils.scala
----------------------------------------------------------------------
diff --git a/core/src/main/scala/kafka/api/ApiUtils.scala b/core/src/main/scala/kafka/api/ApiUtils.scala
index ba1d199..a6b0328 100644
--- a/core/src/main/scala/kafka/api/ApiUtils.scala
+++ b/core/src/main/scala/kafka/api/ApiUtils.scala
@@ -31,11 +31,14 @@ object ApiUtils {
   def writeShortString(buffer: ByteBuffer, string: String) {
     if(string == null) {
       buffer.putShort(-1)
-    } else if(string.length > Short.MaxValue) {
-      throw new KafkaException("String exceeds the maximum size of " + Short.MaxValue + ".")
     } else {
-      buffer.putShort(string.length.asInstanceOf[Short])
-      buffer.put(string.getBytes(ProtocolEncoding))
+      val encodedString = string.getBytes(ProtocolEncoding)
+      if(encodedString.length > Short.MaxValue) {
+        throw new KafkaException("String exceeds the maximum size of " + Short.MaxValue + ".")
+      } else {
+        buffer.putShort(encodedString.length.asInstanceOf[Short])
+        buffer.put(encodedString)
+      }
     }
   }
   
@@ -89,4 +92,4 @@ object ApiUtils {
     else value
   }
   
-}
\ No newline at end of file
+}

http://git-wip-us.apache.org/repos/asf/kafka/blob/2f4bfc64/core/src/test/scala/unit/kafka/api/ApiUtilsTest.scala
----------------------------------------------------------------------
diff --git a/core/src/test/scala/unit/kafka/api/ApiUtilsTest.scala b/core/src/test/scala/unit/kafka/api/ApiUtilsTest.scala
new file mode 100644
index 0000000..2554425
--- /dev/null
+++ b/core/src/test/scala/unit/kafka/api/ApiUtilsTest.scala
@@ -0,0 +1,84 @@
+/**
+ * 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 kafka.api
+
+import org.junit._
+import org.scalatest.junit.JUnitSuite
+import junit.framework.Assert._
+import scala.util.Random
+import java.nio.ByteBuffer
+import kafka.common.KafkaException
+import kafka.utils.TestUtils
+
+object ApiUtilsTest {
+  val rnd: Random = new Random()
+}
+
+class ApiUtilsTest extends JUnitSuite {
+
+  @Test
+  def testShortStringNonASCII() {
+    // Random-length strings
+    for(i <- 0 to 100) {
+      // Since we're using UTF-8 encoding, each encoded byte will be one to four bytes long 
+      val s: String = ApiUtilsTest.rnd.nextString(math.abs(ApiUtilsTest.rnd.nextInt()) % (Short.MaxValue / 4))  
+      val bb: ByteBuffer = ByteBuffer.allocate(ApiUtils.shortStringLength(s))
+      ApiUtils.writeShortString(bb, s)
+      bb.rewind()
+      assertEquals(s, ApiUtils.readShortString(bb))
+    }
+  }
+
+  @Test
+  def testShortStringASCII() {
+    // Random-length strings
+    for(i <- 0 to 100) {
+      val s: String = TestUtils.randomString(math.abs(ApiUtilsTest.rnd.nextInt()) % Short.MaxValue)  
+      val bb: ByteBuffer = ByteBuffer.allocate(ApiUtils.shortStringLength(s))
+      ApiUtils.writeShortString(bb, s)
+      bb.rewind()
+      assertEquals(s, ApiUtils.readShortString(bb))
+    }
+
+    // Max size string
+    val s1: String = TestUtils.randomString(Short.MaxValue)
+    val bb: ByteBuffer = ByteBuffer.allocate(ApiUtils.shortStringLength(s1))
+    ApiUtils.writeShortString(bb, s1)
+    bb.rewind()
+    assertEquals(s1, ApiUtils.readShortString(bb))
+
+    // One byte too big
+    val s2: String = TestUtils.randomString(Short.MaxValue + 1) 
+    try {
+      ApiUtils.shortStringLength(s2)
+      fail
+    } catch {
+      case e: KafkaException => {
+        // ok
+      }
+    }
+    try {
+      ApiUtils.writeShortString(bb, s2)
+      fail
+    } catch {
+      case e: KafkaException => {
+        // ok
+      }
+    }
+  }
+}