RFR: 8264161: BigDecimal#stripTrailingZeros can throw undocumented ArithmeticException
Adds missing @throws declarations for ArithmeticException to the public function java.math.BigDecimal#stripTrailingZeros as well as the private helper functions java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long) java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long) stripTrailingZeros calls one of the two helper functions, both of which can repeatedly decrease the scale by 1 until it underflows. If it does, the call to checkScale will result in an ArithmeticException (overflow). ------------- Commit messages: - 8264161: ArithmeticException not declared for BigDecimal#stripTrailingZeros Changes: https://git.openjdk.java.net/jdk/pull/3189/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3189&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8264161 Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3189.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3189/head:pull/3189 PR: https://git.openjdk.java.net/jdk/pull/3189
On Thu, 25 Mar 2021 08:19:24 GMT, Fabian Meumertzheim <github.com+4312191+fmeum@openjdk.org> wrote:
Adds missing @throws declarations for ArithmeticException to the public function java.math.BigDecimal#stripTrailingZeros as well as the private helper functions java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long) java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long)
stripTrailingZeros calls one of the two helper functions, both of which can repeatedly decrease the scale by 1 until it underflows. If it does, the call to checkScale will result in an ArithmeticException (overflow).
Hi, please send me an e-mail at Dalibor.topic@oracle.com so that I can verify your account. ------------- PR: https://git.openjdk.java.net/jdk/pull/3189
On Thu, 25 Mar 2021 08:19:24 GMT, Fabian Meumertzheim <github.com+4312191+fmeum@openjdk.org> wrote:
Adds missing @throws declarations for ArithmeticException to the public function java.math.BigDecimal#stripTrailingZeros as well as the private helper functions java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long) java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long)
stripTrailingZeros calls one of the two helper functions, both of which can repeatedly decrease the scale by 1 until it underflows. If it does, the call to checkScale will result in an ArithmeticException (overflow).
Please note the issue [8264161](https://bugs.openjdk.java.net/browse/JDK-8264161) has already been addressed by: https://github.com/openjdk/jdk/pull/3204 ------------- PR: https://git.openjdk.java.net/jdk/pull/3189
On Thu, 25 Mar 2021 08:19:24 GMT, Fabian Meumertzheim <github.com+4312191+fmeum@openjdk.org> wrote:
Adds missing @throws declarations for ArithmeticException to the public function java.math.BigDecimal#stripTrailingZeros as well as the private helper functions java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long) java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long)
stripTrailingZeros calls one of the two helper functions, both of which can repeatedly decrease the scale by 1 until it underflows. If it does, the call to checkScale will result in an ArithmeticException (overflow).
Please note the issue [8264161](https://bugs.openjdk.java.net/browse/JDK-8264161) has already been addressed by: #3204
@RogerRiggs The fix in #3204 remains incomplete as it does not update the `@throws` declarations. I could open a new bug for that, but unfortunately the form at https://bugreport.java.com/bugreport/start_form.do appears to be down: There is no server response when clicking "Submit", the request to https://bugreport.java.com/bugreport/submit_start.do just hangs forever. ------------- PR: https://git.openjdk.java.net/jdk/pull/3189
On Apr 21, 2021, at 9:14 AM, Fabian Meumertzheim <github.com+4312191+fmeum@openjdk.java.net<mailto:github.com+4312191+fmeum@openjdk.java.net>> wrote: On Thu, 25 Mar 2021 08:19:24 GMT, Fabian Meumertzheim <github.com+4312191+fmeum@openjdk.org<mailto:github.com+4312191+fmeum@openjdk.org>> wrote: Adds missing @throws declarations for ArithmeticException to the public function java.math.BigDecimal#stripTrailingZeros as well as the private helper functions java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long) java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long) stripTrailingZeros calls one of the two helper functions, both of which can repeatedly decrease the scale by 1 until it underflows. If it does, the call to checkScale will result in an ArithmeticException (overflow). Please note the issue [8264161](https://bugs.openjdk.java.net/browse/JDK-8264161) has already been addressed by: #3204 @RogerRiggs The fix in #3204 remains incomplete as it does not update the `@throws` declarations. I could open a new bug for that, but unfortunately the form at https://bugreport.java.com/bugreport/start_form.do appears to be down: There is no server response when clicking "Submit", the request to https://bugreport.java.com/bugreport/submit_start.do just hangs forever. There is in fact some system maintenance which should be completed this morning, Pacific Time. After that a new issue for this should be filed as we cannot reuse issue IDs. Thanks.
On Apr 21, 2021, at 9:23 AM, Brian Burkhalter <brian.burkhalter@oracle.com<mailto:brian.burkhalter@oracle.com>> wrote: There is in fact some system maintenance which should be completed this morning, Pacific Time. After that a new issue for this should be filed as we cannot reuse issue IDs. Correction: not sure about system maintenance (dates confused). Sorry.
On 4/21/2021 9:14 AM, Fabian Meumertzheim wrote:
On Thu, 25 Mar 2021 08:19:24 GMT, Fabian Meumertzheim <github.com+4312191+fmeum@openjdk.org> wrote:
Adds missing @throws declarations for ArithmeticException to the public function java.math.BigDecimal#stripTrailingZeros as well as the private helper functions java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long) java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long)
stripTrailingZeros calls one of the two helper functions, both of which can repeatedly decrease the scale by 1 until it underflows. If it does, the call to checkScale will result in an ArithmeticException (overflow). Please note the issue [8264161](https://bugs.openjdk.java.net/browse/JDK-8264161) has already been addressed by: #3204 @RogerRiggs The fix in #3204 remains incomplete as it does not update the `@throws` declarations. I could open a new bug for that, but unfortunately the form at https://bugreport.java.com/bugreport/start_form.do appears to be down: There is no server response when clicking "Submit", the request to https://bugreport.java.com/bugreport/submit_start.do just hangs forever.
Just was we don't think it is helpful to put an explicit @throws NullPointerException if a argument is null on every method that throws a NPE for a null argument, I don't think it is helpful or necessary to explicitly note in every BigDecimal method with rounding that it could throw ArithmeticException. The general class-level statement * <p>As a 32-bit integer, the set of values for the scale is large, * but bounded. If the scale of a result would exceed the range of a * 32-bit integer, either by overflow or underflow, the operation may * throw an {@code ArithmeticException}. in meant to capture the needed information. -Joe
On Wed, 21 Apr 2021 16:27:14 GMT, Joe Darcy <joe.darcy@oracle.com> wrote:
Just was we don't think it is helpful to put an explicit
@throws NullPointerException if a argument is null
on every method that throws a NPE for a null argument, I don't think it is helpful or necessary to explicitly note in every BigDecimal method with rounding that it could throw ArithmeticException. The general class-level statement
?* <p>As a 32-bit integer, the set of values for the scale is large, ?* but bounded. If the scale of a result would exceed the range of a ?* 32-bit integer, either by overflow or underflow, the operation may ?* throw an {@code ArithmeticException}.
in meant to capture the needed information.
-Joe
I do agree with this in general, but I think that the situation at hand is a bit different from your example for two reasons: * The `BigDecimal` class already contains many explicit `@throws` annotations for `RuntimeException`s. The absence of such an annotation from a particular method would thus naturally be interpreted as saying that the method does not throw. * For someone not intimately familiar with the internal representation of `BigDecimal`s, it is probably quite unexpected that a function called `stripTrailingZeros` would perform rounding and thus be liable to scale overflows. (This is what happened to the maintainers of jackson-core, where this lead to an unexpected RuntimeException that was only discovered via fuzzing. They simply took this function to perform some kind of "harmless" canonicalization.) That is why I do see value in adding these annotations in this particular case. ------------- PR: https://git.openjdk.java.net/jdk/pull/3189
On Thu, 25 Mar 2021 08:19:24 GMT, Fabian Meumertzheim <github.com+4312191+fmeum@openjdk.org> wrote:
Adds missing @throws declarations for ArithmeticException to the public function java.math.BigDecimal#stripTrailingZeros as well as the private helper functions java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long) java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long)
stripTrailingZeros calls one of the two helper functions, both of which can repeatedly decrease the scale by 1 until it underflows. If it does, the call to checkScale will result in an ArithmeticException (overflow).
Just was we don't think it is helpful to put an explicit @throws NullPointerException if a argument is null on every method that throws a NPE for a null argument, I don't think it is helpful or necessary to explicitly note in every BigDecimal method with rounding that it could throw ArithmeticException. The general class-level statement ?* As a 32-bit integer, the set of values for the scale is large, ?* but bounded. If the scale of a result would exceed the range of a ?* 32-bit integer, either by overflow or underflow, the operation may ?* throw an {@code ArithmeticException}. in meant to capture the needed information. -Joe
I do agree with this in general, but I think that the situation at hand is a bit different from your example for two reasons:
* The `BigDecimal` class already contains many explicit `@throws` annotations for `RuntimeException`s. The absence of such an annotation from a particular method would thus naturally be interpreted as saying that the method does not throw.
* For someone not intimately familiar with the internal representation of `BigDecimal`s, it is probably quite unexpected that a function called `stripTrailingZeros` would perform rounding and thus be liable to scale overflows. (This is what happened to the maintainers of jackson-core, where this lead to an unexpected RuntimeException that was only discovered via fuzzing. They simply took this function to perform some kind of "harmless" canonicalization.)
That is why I do see value in adding these annotations in this particular case.
Thanks for the additional context. Fuzzers would be likely to run into an exception from stripTrailingZeros as they would be likely to start with extreme exponent values. Looking over BigDecimal's throws clauses more closely, I've filed JDK-8265700: Regularize throws clauses in BigDecimal and will send out a PR shortly. This change will add a throws clause for the unexpected exception from stripTrailingZeros and remove several "throws ArithmeticException when rounding occurs and the rounding mode is UNNECESSARY" clauses. ------------- PR: https://git.openjdk.java.net/jdk/pull/3189
On Thu, 25 Mar 2021 08:19:24 GMT, Fabian Meumertzheim <github.com+4312191+fmeum@openjdk.org> wrote:
Adds missing @throws declarations for ArithmeticException to the public function java.math.BigDecimal#stripTrailingZeros as well as the private helper functions java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long) java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long)
stripTrailingZeros calls one of the two helper functions, both of which can repeatedly decrease the scale by 1 until it underflows. If it does, the call to checkScale will result in an ArithmeticException (overflow).
Thanks! ------------- PR: https://git.openjdk.java.net/jdk/pull/3189
On Thu, 25 Mar 2021 08:19:24 GMT, Fabian Meumertzheim <github.com+4312191+fmeum@openjdk.org> wrote:
Adds missing @throws declarations for ArithmeticException to the public function java.math.BigDecimal#stripTrailingZeros as well as the private helper functions java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long) java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long)
stripTrailingZeros calls one of the two helper functions, both of which can repeatedly decrease the scale by 1 until it underflows. If it does, the call to checkScale will result in an ArithmeticException (overflow).
This pull request has been closed without being integrated. ------------- PR: https://git.openjdk.java.net/jdk/pull/3189
participants (6)
-
Brian Burkhalter
-
Dalibor Topic
-
Fabian Meumertzheim
-
Joe Darcy
-
Joe Darcy
-
Roger Riggs