Repository: amzn/ion-java
Pull request #357 (2021-04-30 19:10:06)

Fix unexpected assertion error.
**Description:**

Assertions error raised sometimes in IonUTF8.java when uses unexpected encoding, e.g. `iso-8859-1`.

**Summary of the code:**

Changed assert to IonException and added unit test.

&nbsp;

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.


codecov[bot]: # [Codecov](https://codecov.io/gh/amzn/ion-java/pull/357?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#357](https://codecov.io/gh/amzn/ion-java/pull/357?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (771dacb) into [master](https://codecov.io/gh/amzn/ion-java/commit/0eb62ba92633f03e9f3175bb77f1b3214fcc1cdd?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (0eb62ba) will **increase** coverage by `0.01%`.
> The diff coverage is `25.00%`.

[![Impacted file tree graph](https://codecov.io/gh/amzn/ion-java/pull/357/graphs/tree.svg?width=650&height=150&src=pr&token=M93Y3e3kKn&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)](https://codecov.io/gh/amzn/ion-java/pull/357?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

```diff
@@             Coverage Diff              @@
##             master     #357      +/-   ##
============================================
+ Coverage     64.00%   64.02%   +0.01%     
- Complexity     4828     4832       +4     
============================================
  Files           136      136              
  Lines         21090    21090              
  Branches       3814     3812       -2     
============================================
+ Hits          13498    13502       +4     
+ Misses         6256     6255       -1     
+ Partials       1336     1333       -3     
```


| [Impacted Files](https://codecov.io/gh/amzn/ion-java/pull/357?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [src/com/amazon/ion/impl/IonUTF8.java](https://codecov.io/gh/amzn/ion-java/pull/357/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbS9hbWF6b24vaW9uL2ltcGwvSW9uVVRGOC5qYXZh) | `55.93% <25.00%> (+0.42%)` | `76.00 <3.00> (+1.00)` | |
| [src/com/amazon/ion/impl/BlockedBuffer.java](https://codecov.io/gh/amzn/ion-java/pull/357/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbS9hbWF6b24vaW9uL2ltcGwvQmxvY2tlZEJ1ZmZlci5qYXZh) | `51.21% <0.00%> (+0.36%)` | `80.00% <0.00%> (+3.00%)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/amzn/ion-java/pull/357?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/amzn/ion-java/pull/357?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Last update [0eb62ba...771dacb](https://codecov.io/gh/amzn/ion-java/pull/357?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None).

cheqianh: Tried to avoid confusion, I created a new PR with only one commit.

Since we comment on this one I'll close that one and continue on this.

Patch:
diff --git a/src/com/amazon/ion/impl/IonUTF8.java b/src/com/amazon/ion/impl/IonUTF8.java
index 4b1c718b..f0980b78 100644
--- a/src/com/amazon/ion/impl/IonUTF8.java
+++ b/src/com/amazon/ion/impl/IonUTF8.java
@@ -378,6 +378,9 @@ class IonUTF8 {
     }
 
     public final static boolean needsSurrogateEncoding(int unicodeScalar) {
+        if (unicodeScalar > Character.MAX_CODE_POINT) {
+            throw new IonException("Invalid encoding: encountered non-Unicode character.");
+        }
         return (unicodeScalar > MAXIMUM_UTF16_1_CHAR_CODE_POINT);
     }
     public final static char highSurrogate(int unicodeScalar) {


Test patch:
diff --git a/test/com/amazon/ion/IonExceptionTest.java b/test/com/amazon/ion/IonExceptionTest.java
index dd7c18b0..52c40caa 100644
--- a/test/com/amazon/ion/IonExceptionTest.java
+++ b/test/com/amazon/ion/IonExceptionTest.java
@@ -15,11 +15,15 @@
 
 package com.amazon.ion;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.fail;
 
 import java.io.FileNotFoundException;
 import java.io.IOException;
+
+import com.amazon.ion.system.IonSystemBuilder;
 import org.junit.Test;
 
 
@@ -100,4 +104,20 @@ public class IonExceptionTest
         IonException ion = new IonException(ie1);
         assertNull(ion.causeOfType(IOException.class));
     }
+
+    @Test
+    public void testCauseOfWrongEncoding() {
+        try {
+            byte[] bytes_input = new byte[]{
+                    (byte) 0x27, (byte) 0x31, (byte) -0xB, (byte) 0x31, (byte) 0x31, (byte) 0x31, (byte) 0x27};
+
+            IonSystemBuilder.standard().build().newLoader().load(bytes_input);
+        } catch (IonException e) {
+            // The exception should be caught here
+            assertEquals(e.getMessage(), "Invalid encoding: encountered non-Unicode character.");
+            return;
+        } catch (Exception ignore) {}
+
+        fail();
+    }
 }