On Fri, 6 Mar 2026 14:18:20 GMT, Sean Mullan <[email protected]> wrote:
>> I also recommend updating the `@throws InvalidKeyException` to state that it >> is also thrown if the offset is negative. For example: >> >> "if offset is negative, or the given key material is <code>null</code>, or >> starting at <code>offset</code> inclusive, is shorter than 8 bytes." >> >> Arguably, throwing `IllegalArgumentException` is more appropriate, but I >> think that would have a higher compatibility risk since it would be a new >> exception thrown (even though DES/DESede really shouldn't be used much >> anymore). > > Actually, on second thought, these methods already throw > `ArrayIndexOutOfBoundsException` when the offset is negative, right? > > Therefore I think it is better to throw that instead of `InvalidKeyException` > and add that to the javadoc. This would also align it with `SecretKeySpec` > which throws AIOBE for negative offsets. This is the existing behavior of > this class so the compatibility risk should be low. @seanjmullan thank you for your review. I've updated the PR to switch to `AIOBE` instead of `IKE` for both DES and 3DES in all relevant methods. In order to make the interfaces consistent I did have to explicitly check for null `key` arguments given the ordering with implicit throws. As a result, I needed to update the null `key` checks in DES and 3DES, along with the unit test for null `key` checks (where I test for both ciphers now). Couple of followup questions based on these changes: i) Is a CSR no longer required given that this is what is thrown implicitly given negative `offset` parameters? ii) Should I change the bug ID synopsis to something like: "DES/DES-EDE should consistently check 'key' and 'offset' arguments"? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30069#discussion_r2901229582
