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

[Langer, Christoph]

Hi Serguei,

the change looks good to me, too.

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