RFR(S): JDK-8041435 Make JDWP socket connector accept only local connections by default

[Dmitry Samersoff]

Dan,

Thank you for the review. Next version is:

http://cr.openjdk.java.net/~dsamersoff/JDK-8041435/webrev.02/

please, see below.

On 2014-05-15 19:58, Daniel D. Daugherty wrote:
> On 5/15/14 5:50 AM, Dmitry Samersoff wrote:
>> Hi Everyone,
>>
>> Please review the fix:
>>
>> http://cr.openjdk.java.net/~dsamersoff/JDK-8041435/webrev.01/
> 
> src/share/transport/socket/socketTransport.c
>     line 200:   // it lookups "localhost" and return 127.0.0.1 if lookup
>         Typo: "it lookups" -> "it looks up"
>         Typo: "and return 127.0.0.1" -> "and returns 127.0.0.1"

fixed. .

> 
>     line 210:     return dbgsysHostToNetworkLong(INADDR_LOOPBACK);
>         nit: indent should be 4-spaces (for this file)

fixed.

> 
>     line 235:         u_short port = (u_short)atoi(address+10);
>         Will this crash on "localhost:"? (colon, but no port number)
> 
>     line 236:         sa->sin_port = dbgsysHostToNetworkShort(port);
>         If atoi() on line 235 is passed a non-decimal, what will the
>         value of 'port' be and what will happen here?
> 
>     line 241:         u_short port = (u_short)atoi(address+2);
>     line 242:         sa->sin_port = dbgsysHostToNetworkShort(port);
>         Same questions about these two lines...

Original code doesn't have any guards for invalid user input and I plan
to add it later as a part of upcoming cleanup.

But it's worth to do it now. I also add a test for it.


> 
>     old line 440:     err = parseAddress(addressString, &sa, 0x7f000001);
>         Is it bad that my brain automatically translated that HEX
>         string into 127.0.0.1? I guess I still remember networking
>         code...

Yes, you are correct. It's a bad way to specify loopback address. I
replaced it with INADDR_LOOPBACK.


> I checked out this doc:
> 
> http://docs.oracle.com/javase/7/docs/technotes/guides/jpda/conninv.html

Thanks!

-Dmitry

> 
> and the help message:
> 
> $ java -agentlib:jdwp=help
>                Java Debugger JDWP Agent Library
>                --------------------------------
> 
>   (see http://java.sun.com/products/jpda for more information)
> 
> jdwp usage: java -agentlib:jdwp=[help]|[<option>=<value>, ...]
> 
> Option Name and Value            Description Default
> ---------------------            ----------- -------
> suspend=y|n                      wait on startup? y
> transport=<name>                 transport spec                    none
> address=<listen/attach address>  transport spec                    ""
> server=y|n                       listen for debugger? n
> launch=<command line>            run debugger on event             none
> onthrow=<exception name>         debug on throw                    none
> onuncaught=y|n                   debug on any uncaught? n
> timeout=<timeout value>          for listen/attach in milliseconds n
> mutf8=y|n                        output modified utf-8 n
> quiet=y|n                        control over terminal messages n
> 
> Obsolete Options
> ----------------
> strict=y|n
> stdalloc=y|n
> 
> Examples
> --------
>   - Using sockets connect to a debugger at a specific address:
>     java -agentlib:jdwp=transport=dt_socket,address=localhost:8000 ...
>   - Using sockets listen for a debugger to attach:
>     java -agentlib:jdwp=transport=dt_socket,server=y,suspend=y ...
> 
> Notes
> -----
>   - A timeout value of 0 (the default) is no timeout.
> 
> Warnings
> --------
>   - The older -Xrunjdwp interface can still be used, but will be removed in
>     a future release, for example:
>         java -Xdebug -Xrunjdwp:[help]|[<option>=<value>, ...]
> 
> 
> I don't think your change requires any doc changes...
> 
> Dan
> 
> 
>>
>> After the fix, JDWP server attempts to guess localhost address and bind
>> to it only if no address is specified in command line but user can
>> explicitly bind server to all of available addresses by using * as an
>> address.
>>
> 


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.