Handles the case where the reader unexpectedly encounters an oversized value, throwing IonException with a helpful message. #680
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #680 +/- ##
============================================
+ Coverage 67.06% 67.23% +0.17%
- Complexity 5469 5484 +15
============================================
Files 159 159
Lines 22970 23007 +37
Branches 4116 4122 +6
============================================
+ Hits 15404 15469 +65
+ Misses 6286 6260 -26
+ Partials 1280 1278 -2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Rather than handling nulls in the implementation here, why not just have a default buffer configuration (that throws IonException) that is used when no buffer configuration is set? Then all of this can be encapsulated in the builder, which I think is better for long term maintainability.
…d value, throwing IonException with a helpful message.
tgregg
force-pushed
the
throw-on-null-bufferconfiguration
branch
from
5ae7ba8 to
aa9a794
Compare
|
@popematt Yeah, I think making the throwing configuration the default makes sense. Updated in revision 2. |
| // Using null instead of a no-op handler enables a quick null check to skip calculating the amount of data | ||
| // processed, improving performance. | ||
| BufferConfiguration.DataHandler dataHandler = configuration.getDataHandler(); | ||
| return dataHandler == IonBufferConfiguration.DEFAULT.getDataHandler() ? null : dataHandler; |
There was a problem hiding this comment.
It would be nice if we could do something like this. It makes the intention a little more clear. However, if it would be one of those surprisingly invasive changes, then don't bother with it.
| return dataHandler == IonBufferConfiguration.DEFAULT.getDataHandler() ? null : dataHandler; | |
| return dataHandler == DataHandler.NO_OP ? null : dataHandler; |
Description of changes:
Before this change, if the binary reader encountered an oversized value (due to either corruption of the header or an excessively large scalar), and no IonBufferConfiguration had been set,
NullPointerExceptionwould be thrown. After this change,IonExceptionis thrown with a helpful message. I added some tests to verify in both of the cases listed above thatIonExceptions are thrown.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.