RFR(S): 8034087: XML parser may overwrite element content if that content falls onto the border of an entity scanner buffer

[huizhe wang]

Looks good to me.

Thanks,
Joe

On 2/18/2014 1:43 AM, Volker Simonis wrote:
> Hi,
>
> I've moved the test to test/javax/xml/jaxp/parsers/8027359 as requested:
>
> http://cr.openjdk.java.net/~simonis/webrevs/8034087_1/
>
> Can I please get a review for this change.
>
> Thank you and best regards,
> Volker
>
>
> On Wed, Feb 12, 2014 at 7:25 PM, huizhe wang <huizhe.wang at oracle.com> wrote:
>> Ok, understand.
>>
>> Thanks,
>> Joe
>>
>>
>> On 2/11/2014 11:40 PM, Schreiber, Steffen wrote:
>>> Hi Joe,
>>>
>>> Yes, that's certainly true, if the test is only to show the issue.
>>> We intended the test to work as a regression test as well and wanted to
>>> check the effects of the buffer boundaries on more possible places, i.e.
>>> before/at/after special characters like <, > or / as well as inside tags and
>>> content.
>>>
>>> We can easily adapt the loop, if runtime is a concern here.
>>>
>>> Regards,
>>> Steffen
>>>
>>> -----Original Message-----
>>> From: huizhe wang [mailto:huizhe.wang at oracle.com]
>>> Sent: Dienstag, 11. Februar 2014 22:57
>>> To: Volker Simonis
>>> Cc: Alan Bateman; Schreiber, Steffen; Java Core Libs
>>> Subject: Re: RFR(S): 8034087: XML parser may overwrite element content if
>>> that content falls onto the border of an entity scanner buffer
>>>
>>> Hi Volker,
>>>
>>> I agree with the approach below and jdk9/dev is the better forest.
>>>
>>> For the test itself, I would suggest reducing the following loop to 1 or
>>> 2 cases:
>>>
>>>            for (int i = 0; i < testString.length(); i++) {
>>>                test(createDocument(testString.toString(), i), ""+ i);
>>>            }
>>>
>>>
>>> when i=7, the problem starts to show. It's sufficient to demonstrate the
>>> issue then by just entering 7. It's unnecessary to run the test 43 times.
>>>
>>>
>>> Thanks,
>>> Joe
>>>
>>> On 2/11/2014 9:00 AM, Volker Simonis wrote:
>>>> Hi Alan,
>>>>
>>>> you're right. I initially didn't saw the test because I just looked at
>>>> the change in the jaxp repository.
>>>>
>>>> If it will be approved, I'll put the test in the same directory like
>>>> the other test (i.e. test/javax/xml/jaxp/parsers/8027359).
>>>>
>>>> And yes, my plan was to get approval for both, the tests and the fix,
>>>> when asking for the permission to downport to jdk8u-dev and jdk7u-dev.
>>>>
>>>> Thanks,
>>>> Volker
>>>>
>>>>
>>>> On Tue, Feb 11, 2014 at 5:44 PM, Alan Bateman <Alan.Bateman at oracle.com>
>>>> wrote:
>>>>> On 11/02/2014 14:57, Volker Simonis wrote:
>>>>>> Hi,
>>>>>>
>>>>>> after opening this bug yesterday for an issue found by my colleague
>>>>>> Steffen Schreiber we realized that this is actually a duplicate of
>>>>>> "8027359: XML parser returns incorrect parsing results"
>>>>>> (https://bugs.openjdk.java.net/browse/JDK-8027359).
>>>>>>
>>>>>> While there's no need now to submit a patch anymore,  we'd
>>>>>> nevertheless like to contribute at least our test case for this issue:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~simonis/webrevs/8034087/
>>>>>>
>>>>>> The webrev is against jdk9-client but we'd like to also downport this
>>>>>> test to jdk7 and jdk8 to track that the fix for 8027359 will be
>>>>>> correctly downported to these releases as well.
>>>>>>
>>>>>> I will sponsor this change if somebody would be so kind to review it.
>>>>>>
>>>>> I'll leave it to Joe Wang to comment on the test but just to mention
>>>>> that
>>>>> jdk9/dev is probably a better forest to aim for because that is where
>>>>> the
>>>>> XML (and its tests) usually go.  Also I wonder if it might be better to
>>>>> put
>>>>> it in the same directory as the test that Joe pushed with the change?
>>>>>
>>>>> If you are getting approval to push to jdk8u-dev and jdk7u-dev then it
>>>>> might
>>>>> be better to request a backport of Joe's change at the same time.
>>>>>
>>>>> -Alan.
>>