RFR(M): 8061228 Allow JDWP socket connector to accept connections from certain ip addresses only

[Robbin Ehn]

On 08/25/2017 10:29 AM, Langer, Christoph wrote:
> Hi Serguei,
> 
> the change looks good to me, too.

+1

Thanks for fixing Dmitry and Serguei!

/Robbin

> 
> Minor remark: The copyright year in transport.h was not updated.
> 
> I'm also wondering why the transport layer is only capable of IPv4. But that's probably something that would need to be addressed separately.
> 
> Best regards
> Christoph
> 
> 
>> -----Original Message-----
>> From: serviceability-dev [mailto:serviceability-dev-
>> bounces at openjdk.java.net] On Behalf Of serguei.spitsyn at oracle.com
>> Sent: Donnerstag, 24. August 2017 11:20
>> To: Dmitry Samersoff <dms at samersoff.net>; serviceability-
>> dev at openjdk.java.net; Robbin Ehn <robbin.ehn at oracle.com>; Daniel
>> Daugherty <daniel.daugherty at oracle.com>; Alan Bateman
>> <Alan.Bateman at oracle.com>
>> Subject: Re: RFR(M): 8061228 Allow JDWP socket connector to accept
>> connections from certain ip addresses only
>>
>> Thanks, Dmitry!
>> I'm not sure, who is a reviewer in our case? :)
>> You are the author, aren't you?
>>
>> Thanks,
>> Serguei
>>
>>
>> On 8/24/17 01:08, Dmitry Samersoff wrote:
>>> Serguei,
>>>
>>> The changes looks good to me.
>>>
>>> Thank you for doing it.
>>>
>>> -Dmitry
>>>
>>> On 23.08.2017 23:59, serguei.spitsyn at oracle.com wrote:
>>>> Added Dmitry's email address to the list as he is not subscribed on the
>>>> serviceability-dev.
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>> On 8/22/17 16:22, serguei.spitsyn at oracle.com wrote:
>>>>> Please, review another revision of the fix for the enhancement:
>>>>>     https://bugs.openjdk.java.net/browse/JDK-8061228
>>>>>
>>>>> CSR:
>>>>>     https://bugs.openjdk.java.net/browse/CCC-8061228
>>>>>
>>>>>     The SCR is in the DRAFT state.
>>>>>     Joe suggested to consider this CSR approved and gave a GO for
>>>>> integration.
>>>>>     It will be moved to the right state later when the CSR tools are ready.
>>>>>     I'm still asking at least one reviewer to look at this CSR and give
>>>>> a thumbs up.
>>>>>     It is to ensure everything is going in a right direction.
>>>>>     I'll finalize the CSR after that.
>>>>>
>>>>> Webrev:
>>>>>
>>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2017/hotspot/8061228-
>> jdi-transport.1/
>>>>>
>>>>> The lastest webrev from Dmitry:
>>>>>     http://cr.openjdk.java.net/~dsamersoff/JDK-8061228/webrev.18/
>>>>>
>>>>> Incremental webrev vs the latest webrev from Dmitry:
>>>>>
>>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2017/hotspot/8061228-
>> jdi-transport.1.inc/
>>>>>
>>>>>
>>>>> Summary:
>>>>>     This enhancement was developed by Dmitry who left the team.
>>>>>     I don't know what email address to use to CC him at this point.
>>>>>     I hope, Dmitry will find this discussion and reply accordingly.
>>>>>     The latest webrev revision from Dmitry was v18 (please, see above).
>>>>>
>>>>>     This revision covers the following:
>>>>>       - Cleanup for versioning negotiation protocol (back up to the
>>>>> original).
>>>>>         Now the transport library supports both versions 1_0 and 1_1
>>>>> (newly introduced).
>>>>>       - The transport native interface was changed.
>>>>>         The function SetTransportConfiguration() is introduced instead
>>>>> of the
>>>>>         StartListeningWithAllow(). It allows to the same transport
>>>>> library to support
>>>>>         both old and new version of the transport interface. At this
>>>>> point, the
>>>>>         new structure jdwpTransportConfiguration includes only one field:
>>>>>            const char* allowed_peers;
>>>>>         But it can be extended in the future if any other update in
>>>>> configuration
>>>>>         will be required.
>>>>>       - The unit test was updated to provide better coverage of the
>>>>> corner cases
>>>>>         for 'allow' option introduced by this enhancement.
>>>>>       - Fixes to improve diagnosability.
>>>>>       - A couple of bugs/regressions were fixed so that all the JDI
>>>>> tests are passed now.
>>>>>       - A cleanup that includes some renaming and reformatting.
>>>>>
>>>>>
>>>>> Testing:
>>>>>     Tested new agent flag (allow), with new test:
>>>>>        jdk/test/com/sun/jdi/BasicJDWPConnectionTest.java
>>>>>     Ran the nsk.jdi, nsk.jdwp and jtreg jdk_jdi for both release and
>>>>> fastdebug builds.
>>>>>     All tests are passed.
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>
>