RFR: 8150530:Improve javax.crypto.BadPaddingException messages

[Xuelei Fan]

On 8/24/2016 7:02 PM, Seán Coffey wrote:
>
> On 22/08/16 11:06, Xuelei Fan wrote:
>> Minor comments:
>>
>> CipherCore.java
>> ---------------
>> "... could arise if a bad key or password is used during decryption."
>> "password" may be confusing for some user cases.  This could also
>> happen if bad key used for encryption.  I may just say "could arise if
>> a bad key used."
> The exception message modified only gets thrown in a decryption call.
> Isn't it true that a password fed to an application may be used to
> generate a key ? For that reason, I was trying to suggest that other
> variables (from user) could be at fault for the BadPaddingException.
> Would it be ok to keep the password reference (as a hint?)
>
Password is just one way of many to generate a key.  There is no 
password in the context of this class.  Password based implementation 
should take care of to show the right message.  Adding password word to 
this message may be confusing to both users and developers, I think.

For example, I want to login with password but the key used to encrypt 
and decrypt the communication is not actually generated by the password 
(it's a very common case for HTTPS based login in practice).  This 
exception message may be very confusing.

Not a big issue, I'm OK if you want to keep the password reference.

> I've implemented the rest of your suggested edits. Makes sense - thanks.
> new webrev :
> http://cr.openjdk.java.net/~coffeys/webrev.8150530.v2/webrev/index.html
>
Looks fine to me.

Thanks,
Xuelei

> regards,
> Sean.
>
>>
>> RSAPadding.java
>> ---------------
>> I may prefer to use a sentence for the exception message.  For example:
>>    "Data must be shorter than ... bytes, but received ... bytes"
>>    "The pad array length (padded.length) is not the specified pad size
>> (paddedSize)  "
>>
>> CipherBox.java
>> --------------
>> I may not use the internal variable name in the exception message.  It
>> might be easier to read:
>>
>> 496/580: "The padding removed text (newLen bytes) should be bigger
>> than <blockSize> as explicit IV used."
>>
>> 763/810: "The padding length (padLen) of SSLv3 message should not
>> bigger than the block size (blockSize)."
>>
>> 934: "Insufficient buffer for AEAD cipher fragment, needs more than
>> (recordIvSize + tagSize) bytes, but only (bb.remaining()) remains in
>> the buffer"
>>
>> P11RSACipher.java
>> -----------------
>> 360: "The output buffer (outLen bytes) is too small to hold the
>> produced data (tmpBuffer.length bytes)"
>>
>> Thanks,
>> Xuelei
>>
>> On 8/22/2016 3:56 PM, Seán Coffey wrote:
>>> Looking to improve some of the messages used in generation of
>>> BadPaddingException messages. The 'Given final block not properly
>>> padded' one in particular has caused confusion for some users in the
>>> past.
>>>
>>> JBS report : https://bugs.openjdk.java.net/browse/JDK-8150530
>>> webrev : http://cr.openjdk.java.net/~coffeys/webrev.8150530/webrev/
>>>
>