RFR: JDK-8215407: Javadoc throws StringIndexOutOfBoundsException: String index out of range: -2

[Jonathan Gibbons]

Much nicer; looks good to me.

-- Jon

On 3/20/19 12:22 AM, Jan Lahoda wrote:
> On 18.3.2019 23:17, Jonathan Gibbons wrote:
>> I think it would be better to put all the name checking together, in
>> simpleBinaryName.
>
> Updated webrev:
> http://cr.openjdk.java.net/~jlahoda/8215407/webrev.01/
>
> (The only difference compared to the .00 should be in ClassReader.java.)
>
> Does that look OK?
>
> Thanks,
>     Jan
>
>>
>> -- Jon
>>
>> On 3/18/19 9:20 AM, Jan Lahoda wrote:
>>> On 16.3.2019 01:10, Jonathan Gibbons wrote:
>>>> OK, I see the additional checks in simpleBinaryName, which is the sort
>>>> of check I was expecting.
>>>>
>>>> I'm a bit surprised you didn't put the check inside simpleBinaryName,
>>>> but that would trigger a subsequent question as to why we have
>>>
>>> I started with the check inside the simpleBinaryName method, but then
>>> moved it outside, as the check seemed like a "general" check, not
>>> something related to the simple name as such. I don't have a very
>>> strong opinion on this, I can move the check inside simpleBinaryName
>>> if preferred.
>>>
>>> Thanks,
>>>     Jan
>>>
>>>>
>>>> 1478 throw badEnclosingMethod(self);
>>>>
>>>> and ClassReader.simpleBinaryName:1507
>>>>
>>>>      throw badClassFile("bad.enclosing.method", self);
>>>>
>>>> I'm OK with either webrev as is or with the check moved into
>>>> simpleBinaryName,
>>>>
>>>> -- Jon
>>>>
>>>> On 03/06/2019 02:31 AM, Jan Lahoda wrote:
>>>>> Hi Jon,
>>>>>
>>>>> On 5.3.2019 21:20, Jonathan Gibbons wrote:
>>>>>> Jan,
>>>>>>
>>>>>> Is checking for a prefix good enough?   It seems like that would be
>>>>>> necessary but not sufficient.
>>>>>
>>>>> What specific additional checks do you foresee? The
>>>>> ClassReader.simpleBinaryName method does some additional checks, like
>>>>> that the class suffix is non-empty and starts with '$'.
>>>>>
>>>>> Thanks,
>>>>>     Jan
>>>>>
>>>>>>
>>>>>> -- Jon
>>>>>>
>>>>>> On 3/5/19 9:00 AM, Jan Lahoda wrote:
>>>>>>> On 5.3.2019 17:33, Joe Darcy wrote:
>>>>>>>> Hi Jan,
>>>>>>>>
>>>>>>>> Do you think this issue merits a CSR for the behavioral change?
>>>>>>>
>>>>>>> Good question! It is true that javac may now refuse to load
>>>>>>> classfiles
>>>>>>> that didn't lead to the exception before, so I guess a CSR may 
>>>>>>> be in
>>>>>>> order. I've started with it here:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8220167
>>>>>>>
>>>>>>> Feedback on the CSR is very welcome as well!
>>>>>>>
>>>>>>> Thanks,
>>>>>>>     Jan
>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> -Joe
>>>>>>>>
>>>>>>>> On 3/5/2019 7:55 AM, Jan Lahoda wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Seems that some classfiles have a weird EnclosingMethod 
>>>>>>>>> attribute -
>>>>>>>>> for (local/anonymous) class A, the EnclosingMethod attribute 
>>>>>>>>> states
>>>>>>>>> the enclosing class is C, but the name of C is not a prefix of 
>>>>>>>>> the
>>>>>>>>> name of A. This appears to be contradictory to JLS 13.1., so the
>>>>>>>>> proposal is to reject such classfiles.
>>>>>>>>>
>>>>>>>>> Webrev: http://cr.openjdk.java.net/~jlahoda/8215407/webrev.00/
>>>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8215407
>>>>>>>>>
>>>>>>>>> How does this look?
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>     Jan
>>>>