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

[Roger Riggs]

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
>>>>>
>>>>
>>>
>>
>