Repository: amzn/ion-java
Pull request #349 (2021-03-24 19:04:29)

Catches exception of unkown sid bigger than INT_MAX.
### Description:

For a file with text: `$33333333333333`, ion-java raise NumberFormatException exception instead of IonException.

The issue happens when we call the [hasNext()](https://github.com/amzn/ion-java/blob/30536fbee0cf93d7b21c1702cf4ba6df6a58965e/src/com/amazon/ion/impl/IonReaderTextRawX.java#L486) method. When we decode a SID that is bigger than INT_MAX, it will raise NumberFormatException in [parseInt method](https://github.com/amzn/ion-java/blob/30536fbee0cf93d7b21c1702cf4ba6df6a58965e/src/com/amazon/ion/impl/IonTokenConstsX.java#L516). 

hasNext() method is called before other logic (e.g. SymbolToken validate method), so I think we can just add a try...catch here.

### Test:

**unit test:**
`Tests passed: 23159, ignored: 391 of 23550 tests - 30 s 635 ms`


**manually test:**
For the file with text: `$33333333333333`, it raise UnknownSymbolException now.

```.java
Exception in thread "main" com.amazon.ion.UnknownSymbolException: 33333333333333
```

For exception, should I add more unit test for UnknownSymbolException line 520 - 521)


codecov[bot]: # [Codecov](https://codecov.io/gh/amzn/ion-java/pull/349?src=pr&el=h1) Report
> Merging [#349](https://codecov.io/gh/amzn/ion-java/pull/349?src=pr&el=desc) (037f6a1) into [master](https://codecov.io/gh/amzn/ion-java/commit/30536fbee0cf93d7b21c1702cf4ba6df6a58965e?el=desc) (30536fb) will **increase** coverage by `0.00%`.
> The diff coverage is `33.33%`.

[![Impacted file tree graph](https://codecov.io/gh/amzn/ion-java/pull/349/graphs/tree.svg?width=650&height=150&src=pr&token=M93Y3e3kKn)](https://codecov.io/gh/amzn/ion-java/pull/349?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##             master     #349   +/-   ##
=========================================
  Coverage     63.89%   63.89%           
  Complexity     4811     4811           
=========================================
  Files           136      136           
  Lines         21058    21060    +2     
  Branches       3815     3815           
=========================================
+ Hits          13454    13456    +2     
- Misses         6267     6268    +1     
+ Partials       1337     1336    -1     
```


| [Impacted Files](https://codecov.io/gh/amzn/ion-java/pull/349?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [src/com/amazon/ion/impl/IonTokenConstsX.java](https://codecov.io/gh/amzn/ion-java/pull/349/diff?src=pr&el=tree#diff-c3JjL2NvbS9hbWF6b24vaW9uL2ltcGwvSW9uVG9rZW5Db25zdHNYLmphdmE=) | `54.23% <33.33%> (-0.24%)` | `142.00 <0.00> (ø)` | |
| [src/com/amazon/ion/impl/BlockedBuffer.java](https://codecov.io/gh/amzn/ion-java/pull/349/diff?src=pr&el=tree#diff-c3JjL2NvbS9hbWF6b24vaW9uL2ltcGwvQmxvY2tlZEJ1ZmZlci5qYXZh) | `50.97% <0.00%> (+0.24%)` | `77.00% <0.00%> (ø%)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/amzn/ion-java/pull/349?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/amzn/ion-java/pull/349?src=pr&el=footer). Last update [30536fb...037f6a1](https://codecov.io/gh/amzn/ion-java/pull/349?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

cheqianh: Sure. I didn't add test immediately is because in case we can find a better way to throw the exception here.
cheqianh: Added unit test and changed exception.

Patch:
diff --git a/src/com/amazon/ion/impl/IonTokenConstsX.java b/src/com/amazon/ion/impl/IonTokenConstsX.java
index c3616066..33a8e8e3 100644
--- a/src/com/amazon/ion/impl/IonTokenConstsX.java
+++ b/src/com/amazon/ion/impl/IonTokenConstsX.java
@@ -513,7 +513,12 @@ final class IonTokenConstsX
         assert length > 1;
 
         String digits = sidToken.subSequence(1, length).toString();
-        return Integer.parseInt(digits);
+
+        try {
+            return Integer.parseInt(digits);
+        } catch (Exception e) {
+            throw new IonException(String.format("Unable to parse SID %s", digits), e);
+        }
     }
 
     static public int keyword(CharSequence word, int start_word, int end_word)


Test patch:
diff --git a/test/com/amazon/ion/LoaderTest.java b/test/com/amazon/ion/LoaderTest.java
index fc020077..ff936a6a 100644
--- a/test/com/amazon/ion/LoaderTest.java
+++ b/test/com/amazon/ion/LoaderTest.java
@@ -160,6 +160,17 @@ public class LoaderTest
         checkInt(123, value);
     }
 
+    @Test
+    public void testLargeSidSymbol()
+    {
+        String text = "$11111111111111";
+
+        // it's supposed to throw an IonException since sid is greater than INT_MAX.
+        try {
+            IonSymbol value = (IonSymbol) loadOneValue(text);
+            Assert.fail("Expected IonException to be thrown.");
+        } catch (IonException ignore) { /* expected to reach here */ }
+    }
 
     private static class FailingInputStream extends InputStream
     {