Fix a CharsetEncoder bug.

When malformed or unmappable characters span input
buffers, we'd end up setting a negative position on the
buffer.

Also fix up a few test cases which were wrong in
several ways.
- One test was simply checking for the wrong sort
  of exception (unmappable vs malformed)
- Another test was expecting encode() to throw an
  error (and ignoring flush) but the API allows flush
  to throw an error instead of encode.

bug: 10729779

Change-Id: I6560b749ca2445651d61ca651f8a5e388cf1c1b0

2 files changed, 27 insertions, 12 deletions

diff --git a/harmony-tests/src/test/java/org/apache/harmony/tests/java/nio/charset/ASCIICharsetEncoderTest.java b/harmony-tests/src/test/java/org/apache/harmony/tests/java/nio/charset/ASCIICharsetEncoderTest.java
index 3be239c..11b030a 100644
--- a/harmony-tests/src/test/java/org/apache/harmony/tests/java/nio/charset/ASCIICharsetEncoderTest.java
+++ b/harmony-tests/src/test/java/org/apache/harmony/tests/java/nio/charset/ASCIICharsetEncoderTest.java
@@ -16,6 +16,7 @@
 
 package org.apache.harmony.tests.java.nio.charset;
 
+import junit.framework.TestCase;
 import java.nio.ByteBuffer;
 import java.nio.CharBuffer;
 import java.nio.charset.CharacterCodingException;
@@ -26,13 +27,11 @@ import java.nio.charset.CodingErrorAction;
 import java.nio.charset.MalformedInputException;
 import java.nio.charset.UnmappableCharacterException;
 
-import junit.framework.TestCase;
-
 public class ASCIICharsetEncoderTest extends TestCase {
 
 	// charset for ascii
-	private static final Charset cs = Charset.forName("ascii");
-    private static final CharsetEncoder encoder = cs.newEncoder();
+	private final Charset cs = Charset.forName("ascii");
+    private final CharsetEncoder encoder = cs.newEncoder();
     private static final int MAXCODEPOINT = 0x7F;
 	/*
 	 * @see CharsetEncoderTest#setUp()
@@ -83,15 +82,21 @@ public class ASCIICharsetEncoderTest extends TestCase {
 		}
 		encoder.reset();
 		ByteBuffer out = ByteBuffer.allocate(10);
-		assertTrue(encoder.encode(CharBuffer.wrap("\ud800"), out, true)
-				.isMalformed());
-		encoder.flush(out);
+		assertEquals(CoderResult.UNDERFLOW,
+                encoder.encode(CharBuffer.wrap("\ud800"), out, true));
+        assertTrue(encoder.flush(out).isMalformed());
 		encoder.reset();
+
 		out = ByteBuffer.allocate(10);
-		assertSame(CoderResult.UNDERFLOW, encoder.encode(CharBuffer
-				.wrap("\ud800"), out, false));
-		assertTrue(encoder.encode(CharBuffer.wrap("\udc00"), out, true)
-				.isMalformed());
+        CharBuffer buffer1 = CharBuffer.wrap("\ud800");
+        CharBuffer buffer2 = CharBuffer.wrap("\udc00");
+		assertSame(CoderResult.UNDERFLOW, encoder.encode(buffer1, out, false));
+        // We consume the entire input buffer because we're in an underflow
+        // state. We can't make a decision on whether the char in this buffer
+        // is unmappable or malformed without looking at the next input buffer.
+        assertEquals(1, buffer1.position());
+		assertTrue(encoder.encode(buffer2, out, true).isUnmappable());
+        assertEquals(0, buffer2.position());
 	}
 
     public void testEncodeMapping() throws CharacterCodingException {
diff --git a/luni/src/main/java/java/nio/charset/CharsetEncoderICU.java b/luni/src/main/java/java/nio/charset/CharsetEncoderICU.java
index 5141d80..23aa093 100644
--- a/luni/src/main/java/java/nio/charset/CharsetEncoderICU.java
+++ b/luni/src/main/java/java/nio/charset/CharsetEncoderICU.java
@@ -240,7 +240,17 @@ final class CharsetEncoderICU extends CharsetEncoder {
     }
 
     private void setPosition(CharBuffer in) {
-        in.position(in.position() + data[INPUT_OFFSET] - data[INVALID_CHAR_COUNT]);
+        int position = in.position() + data[INPUT_OFFSET] - data[INVALID_CHAR_COUNT];
+        if (position < 0) {
+            // The calculated position might be negative if we encountered an
+            // invalid char that spanned input buffers. We adjust it to 0 in this case.
+            //
+            // NOTE: The API doesn't allow us to adjust the position of the previous
+            // input buffer. (Doing that wouldn't serve any useful purpose anyway.)
+            position = 0;
+        }
+
+        in.position(position);
         // release reference to input array, which may not be ours
         input = null;
     }