RFR 8218228 : The constructor StringBuffer(CharSequence) violates spec for negatively sized argument

Tue Feb 12 16:51:54 UTC 2019

Hi Ivan,

Sorry, one more.  The compatibility risk description should be specific 
about
the behavior that is changing.  Something like:

Since JDK5, the exception thrown for negative lengths have been unspecified
and inconsistent; with this change, NegativeArraySizeException will be 
thrown.

Thanks, Roger

p.s.  A release note capturing the essence of the change is probably 
needed also.
Add label release-note=yes to the issue and create a subtask.

On 02/12/2019 11:39 AM, Ivan Gerasimov wrote:
>
>
> On 2/12/19 7:33 AM, Roger Riggs wrote:
>> Hi Ivan,
>>
>> The change to consistently throw NegativeSizeArrayException applies 
>> to StringBuilder also.
>> Please mention that with the solution.
>>
> Right, thanks.  I updated CSR accordingly.
>
> With kind regards,
> Ivan
>
>> Thanks, Roger
>>
>>
>> On 02/11/2019 06:12 PM, Ivan Gerasimov wrote:
>>> Hi Roger!
>>>
>>>
>>> On 2/11/19 7:30 AM, Roger Riggs wrote:
>>>> Hi Ivan,
>>>>
>>>> I called it out because the CSR does not mention that the behavior
>>>> of some of the cases (-1..-16) is changing and some of the emails 
>>>> asserted
>>>> there was no change in behavior.
>>>>
>>>> I'm fine with one changeset as long as both changes are explicit.
>>>> The bug and the CSR should both be updated.
>>>>
>>> Yes, this makes sense.
>>> I've updated CSR accordingly, and added a comment to the bug to make 
>>> the intention of the fix clear.
>>>
>>> Could you please review the updated CSR?
>>>
>>> With kind regards,
>>> Ivan
>>>
>>>
>>>
>>>> Thanks, Roger
>>>>
>>>>
>>>> On 02/08/2019 05:12 PM, Ivan Gerasimov wrote:
>>>>> Hi Roger!
>>>>>
>>>>> That's because two tiny changes are combined in the patch:
>>>>>
>>>>> 1) remove a problematic statement from the javadoc, as it doesn't 
>>>>> hold anyway.  This part does not change the behavior.
>>>>>
>>>>> 2) unify type of the exception thrown for all negative values of 
>>>>> CharSequence.length().  The regression test is to verify this, 
>>>>> second change, so it fails prior the fix for length in [-16, -1].
>>>>>
>>>>> I can separate them into different bugs, if you think it will make 
>>>>> it clearer.  Though I thought they can be fixed together.
>>>>>
>>>>> With kind regards,
>>>>>
>>>>> Ivan
>>>>>
>>>>>
>>>>> On 2/8/19 1:49 PM, Roger Riggs wrote:
>>>>>> Hi Ivan,
>>>>>>
>>>>>> In the direction of not changing the behavior; the webrev does 
>>>>>> change the behavior.
>>>>>>
>>>>>> In the case of CharSeqence with length -1..-16, the new behavior 
>>>>>> throws NegativeArrayIndexException
>>>>>> instead of java.lang.IndexOutOfBoundsException.
>>>>>>
>>>>>>
>>>>>> AbstractStringBuilder:101-102 throw an exception for length < 0.
>>>>>> However, the current behavior is to create a StringBuffer with 
>>>>>> length + 16 and
>>>>>> then append the contents.  For -1..-16, it will succeed in 
>>>>>> creating a StringBuffer
>>>>>> but fail with IndexOutOfBoundsException during the append of the 
>>>>>> contents.
>>>>>>
>>>>>> The new Test should pass both before and after the change to the 
>>>>>> code.
>>>>>>
>>>>>> Roger
>>>>>>
>>>>>>
>>>>>> On 02/07/2019 10:19 PM, Ivan Gerasimov wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 2/7/19 6:33 PM, David Holmes wrote:
>>>>>>>> On 8/02/2019 11:59 am, Ivan Gerasimov wrote:
>>>>>>>>> Hi David!
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2/7/19 5:16 PM, David Holmes wrote:
>>>>>>>>>> Hi Ivan,
>>>>>>>>>>
>>>>>>>>>> On 8/02/2019 11:02 am, Ivan Gerasimov wrote:
>>>>>>>>>>> Hello!
>>>>>>>>>>>
>>>>>>>>>>> The specification states:
>>>>>>>>>>> """
>>>>>>>>>>> If the length of the specified CharSequence is less than or 
>>>>>>>>>>> equal to zero, then an empty buffer of capacity 16 is returned.
>>>>>>>>>>> """
>>>>>>>>>>>
>>>>>>>>>>> However, the current implementation throws either 
>>>>>>>>>>> NegativeArraySizeException or IndexOutOfBounds instead (the 
>>>>>>>>>>> actual exception type depends on the value of reported 
>>>>>>>>>>> negative length).
>>>>>>>>>>
>>>>>>>>>> How can you have a negative length CharSequence ??
>>>>>>>>>>
>>>>>>>>> A custom CharSequence returning negative length() can be created.
>>>>>>>>> Not sure how useful/popular this may be, though.
>>>>>>>>
>>>>>>>> Seems pretty meaningless so just treating that as an error 
>>>>>>>> seems fine. Though somewhat debatable whether you need to add 
>>>>>>>> an appropriate @throws.
>>>>>>>>
>>>>>>> Right.  There were two reasons not to add @throws here:
>>>>>>> - by removing the problematic paragraph we just make the javadoc 
>>>>>>> almost the same as for StringBuilder(CharSequence),
>>>>>>> - we don't seem to have any other places (at least I couldn't 
>>>>>>> find any) specifying exceptions due to negatively sized 
>>>>>>> CharSequence.
>>>>>>>
>>>>>>> (To be precise, there is one candidate, but I'll file a separate 
>>>>>>> bug to fix it.)
>>>>>>>
>>>>>>>>> That's why I propose just removing the mentioning negative 
>>>>>>>>> length, and not changing the behavior.
>>>>>>>>>
>>>>>>>>> The proposed code change is to only unify the behavior for any 
>>>>>>>>> negative value of length.
>>>>>>>>
>>>>>>>> Ok.
>>>>>>>>
>>>>>>>>>> If its an empty CharSequence then it should return the empty 
>>>>>>>>>> buffer as indicated.
>>>>>>>>>>
>>>>>>>>> Empty CharSequence is processed correctly already.
>>>>>>>>
>>>>>>>> Okay so by removing this part:
>>>>>>>>
>>>>>>>> -      * <p>
>>>>>>>> -      * If the length of the specified {@code CharSequence} is
>>>>>>>> -      * less than or equal to zero, then an empty buffer of 
>>>>>>>> capacity
>>>>>>>> -      * {@code 16} is returned.
>>>>>>>>
>>>>>>>> you're relying on the main specification to implicitly handle 
>>>>>>>> the empty case.
>>>>>>>>
>>>>>>> Yes.
>>>>>>>
>>>>>>> With kind regards,
>>>>>>> Ivan
>>>>>>>
>>>>>>>
>>>>>>>> David
>>>>>>>> -----
>>>>>>>>
>>>>>>>>> With kind regards,
>>>>>>>>> Ivan
>>>>>>>>>
>>>>>>>>>> Cheers,
>>>>>>>>>> David
>>>>>>>>>>
>>>>>>>>>>> It is proposed to do two things:
>>>>>>>>>>> 1) remove the problematic sentence from the javadoc (CSR is 
>>>>>>>>>>> filed).
>>>>>>>>>>> 2) unify the exception type thrown, if the argument reports 
>>>>>>>>>>> negative length.
>>>>>>>>>>> NegativeArraySizeException will be consistent with, for 
>>>>>>>>>>> example, StringBuffer(negativeCapacity).
>>>>>>>>>>>
>>>>>>>>>>> Would you please help review the fix?
>>>>>>>>>>>
>>>>>>>>>>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8218228
>>>>>>>>>>> WEBREV: http://cr.openjdk.java.net/~igerasim/8218228/00/webrev/
>>>>>>>>>>> CRS: https://bugs.openjdk.java.net/browse/JDK-8218649
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>