Serguei, > I still see the C .vs. C++ related change in the jdwpTransport.h.
done. http://cr.openjdk.java.net/~dsamersoff/JDK-8061228/webrev.12/ see also inline. On 2017-03-15 00:40, serguei.spit...@oracle.com wrote: > Hi Dmitry, > > We already had a big review thread back in 2014 on this. > I think, it is again going in the wrong order. > > First, I think, it is better to start from a CCC (or its equivalent, CSR). > This will allow to focus on and sort out the changes in interface/behavior > first before going into the implementation details. CCC was filed and approved. The only reason I withdraw it - the fix didn't go to jdk9 but CCC tool doesn't have jdk10. CSR process is also not yet implemented. > Second, I'd suggest to separate a couple of other things. > I still see the C .vs. C++ related change in the jdwpTransport.h. > It is better to leave it along for now as it was suggested in early > review rounds. > If you still want to fix it then it is better to file a separate bug that > should include the JNI as well (as it was discussed with Alan before). Do you mean? 39 #ifdef __cplusplus 40 extern "C" { 41 #endif I'll add it back to avoid any confusion. > Also, I'm thinking if it is a good idea to separate the transport > versioning to an RFE. > It would allow to focus on this aspect as necessary. > In this case, the 8061228 will depend on the versioning. > However, it is much more simple and can be resolved faster. It's hard to test versioning code without implementation of new, VERSION_1_1 transport. Network part of 8061228 is simple and clear separated from versioning, so I would prefer to keep it together in one CR/one push. Restriction turned off by default (I'll file separate CR to enable it later), so we have enough time to fix any issues that might come. -Dmitry > > > Thanks, > Serguei > > > On 2/28/17 01:41, Dmitry Samersoff wrote: >> Everybody, >> >> Please review: >> >> http://cr.openjdk.java.net/~dsamersoff/JDK-8061228/webrev.10/ >> >> These changes introduce new parameter[1] of the socket transport - >> allow. Users can explicitly specify a list of hosts that allowed to >> connect to jdwp server and it's the second part of JDWP hardening[2]. >> >> No restrictions are applied by default now but I'll file a separate CR >> to restrict list of allowed peers to localhost by default. >> >> Also these changes implement versioning for jdwp transport and therefor >> simplify feature development of jdwp. >> >> >> 1. Example command line: >> >> -agentlib:jdwp=transport=dt_socket,server=y,suspend=n, >> address=*,allow="127.0.0.0/8;192.168.0.0/24" >> >> Possible values for allow parameter: >> * - accept connections from everywhere. >> N.N.N.N - accept connections from this IP address only >> N.N.N.N/nn - accept connections from particular ip subnet >> >> >> >> 2. JDK-8052136 JDWP hardening >> >> -Dmitry >> > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
