Merge pull request msgpack#359 from msgpack/test-str8-disable

xerial · xerial · commit 202ca62fb641 · 2016-04-11T11:29:33.000-07:00
Fix packString
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -1,5 +1,8 @@
 # Release Notes
 
+## 0.8.5
+ * Add PackerConfig.withStr8FormatSupport (default: true) for backward compatibility with earier versions of msgpack v0.6, which doesn't have STR8 type.
+
 ## 0.8.4
  * Embed bundle paramters for OSGi
 
diff --git a/msgpack-core/src/main/java/org/msgpack/core/MessagePacker.java b/msgpack-core/src/main/java/org/msgpack/core/MessagePacker.java
@@ -449,6 +449,7 @@ private void prepareEncoder()
         if (encoder == null) {
             this.encoder = MessagePack.UTF8.newEncoder();
         }
+        encoder.reset();
     }
 
     private int encodeStringToBufferAt(int pos, String s)
@@ -466,7 +467,13 @@ private int encodeStringToBufferAt(int pos, String s)
                 throw new MessageStringCodingException(e);
             }
         }
-        if (cr.isUnderflow() || cr.isOverflow()) {
+        if (!cr.isUnderflow() || cr.isOverflow()) {
+            // Underflow should be on to ensure all of the input string is encoded
+            return -1;
+        }
+        // NOTE: This flush method does nothing if we use UTF8 encoder, but other general encoders require this
+        cr = encoder.flush(bb);
+        if (!cr.isUnderflow()) {
             return -1;
         }
         return bb.position() - startPosition;
@@ -499,7 +506,7 @@ else if (s.length() < (1 << 8)) {
             // keep 2-byte header region and write raw string
             int written = encodeStringToBufferAt(position + 2, s);
             if (written >= 0) {
-                if (written < (1 << 8)) {
+                if (str8FormatSupport && written < (1 << 8)) {
                     buffer.putByte(position++, STR8);
                     buffer.putByte(position++, (byte) written);
                     position += written;
diff --git a/msgpack-core/src/test/scala/org/msgpack/core/MessagePackerTest.scala b/msgpack-core/src/test/scala/org/msgpack/core/MessagePackerTest.scala
@@ -282,4 +282,39 @@ class MessagePackerTest extends MessagePackSpec {
       .writePayload(payload)
       .close()
   }
+
+  "pack small string with STR8" in {
+    val packer = new PackerConfig().newBufferPacker()
+    packer.packString("Hello. This is a string longer than 32 characters!")
+    val b = packer.toByteArray
+
+    val unpacker = MessagePack.newDefaultUnpacker(b)
+    val f = unpacker.getNextFormat
+    f shouldBe MessageFormat.STR8
+  }
+
+  "be able to disable STR8 for backward compatibility" in {
+    val config = new PackerConfig()
+            .withStr8FormatSupport(false)
+
+    val packer = config.newBufferPacker()
+    packer.packString("Hello. This is a string longer than 32 characters!")
+    val unpacker = MessagePack.newDefaultUnpacker(packer.toByteArray)
+    val f = unpacker.getNextFormat
+    f shouldBe MessageFormat.STR16
+  }
+
+  "be able to disable STR8 when using CharsetEncoder" in {
+    val config = new PackerConfig()
+            .withStr8FormatSupport(false)
+            .withSmallStringOptimizationThreshold(0) // Disable small string optimization
+
+    val packer = config.newBufferPacker()
+    packer.packString("small string")
+    val unpacker = MessagePack.newDefaultUnpacker(packer.toByteArray)
+    val f = unpacker.getNextFormat
+    f shouldNot be (MessageFormat.STR8)
+    val s = unpacker.unpackString()
+    s shouldBe "small string"
+  }
 }

Original file line number	Diff line number	Diff line change
`@@ -449,6 +449,7 @@ private void prepareEncoder()`
`449`	`449`	`if (encoder == null) {`
`450`	`450`	`this.encoder = MessagePack.UTF8.newEncoder();`
`451`	`451`	`}`
	`452`	`+ encoder.reset();`
`452`	`453`	`}`
`453`	`454`
`454`	`455`	`private int encodeStringToBufferAt(int pos, String s)`
`@@ -466,7 +467,13 @@ private int encodeStringToBufferAt(int pos, String s)`
`466`	`467`	`throw new MessageStringCodingException(e);`
`467`	`468`	`}`
`468`	`469`	`}`
`469`		`- if (cr.isUnderflow() \|\| cr.isOverflow()) {`
	`470`	`+ if (!cr.isUnderflow() \|\| cr.isOverflow()) {`
	`471`	`+ // Underflow should be on to ensure all of the input string is encoded`
	`472`	`+ return -1;`
	`473`	`+ }`
	`474`	`+ // NOTE: This flush method does nothing if we use UTF8 encoder, but other general encoders require this`
	`475`	`+ cr = encoder.flush(bb);`
	`476`	`+ if (!cr.isUnderflow()) {`
`470`	`477`	`return -1;`
`471`	`478`	`}`
`472`	`479`	`return bb.position() - startPosition;`
`@@ -499,7 +506,7 @@ else if (s.length() < (1 << 8)) {`
`499`	`506`	`// keep 2-byte header region and write raw string`
`500`	`507`	`int written = encodeStringToBufferAt(position + 2, s);`
`501`	`508`	`if (written >= 0) {`
`502`		`- if (written < (1 << 8)) {`
	`509`	`+ if (str8FormatSupport && written < (1 << 8)) {`
`503`	`510`	`buffer.putByte(position++, STR8);`
`504`	`511`	`buffer.putByte(position++, (byte) written);`
`505`	`512`	`position += written;`