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

[Ivan Gerasimov]

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

-- 
With kind regards,
Ivan Gerasimov