Adding SocketChannel toString to connection exception messages

[David Holmes]

On 22/12/2017 10:29 AM, Steven Schlansker wrote:
> 
>> On Dec 21, 2017, at 11:11 AM, Steven Schlansker <stevenschlansker at gmail.com> wrote:
>>
>> What if ConnectException included the attempted hostname / IP / port SocketAddress?
>> java.net.ConnectException: Connection to 'foo.mycorp.com[10.x.x.x]:12345' refused
>> Much more useful!  This could also be extended to various other socket exceptions.

I believe there are concerns with too much information that can be 
considered "sensitive" (like host names and IP addresses) appearing in 
error messages due to them ending up in log files and bug reports.

David
-----

> I started hacking around with this, first with PlainSocketImpl only.
> I apologize in advance for what is likely to be terrible JNI style, as I don't have much experience
> with it and I omitted any error handling for the POC - but I think it proves that this isn't too big of a project, as
> I would want to cover SocketChannelImpl and maybe UnixAsynchronousSocketChannelImpl and call it good enough for my uses.
> 
> Behold!
> 
> [me at myhost]% java pkg.SocketFail
> java.net.ConnectException: Connection refused (Connection refused)
> 
> [me at myhost]% ~/code/jdk10/build/macosx-x86_64-normal-server-release/jdk/bin/java pkg.SocketFail
> java.net.ConnectException: Connection refused (Connection refused: localhost/127.0.0.1:12345)
> 
> 
> Am I correct that the best path towards acceptance for this kind of change is to target jdk10 and then beg for a backport to jdk9u?
> Thanks again for considering this improvement! WIP patch below inline.
> 
> 
> diff --git a/src/java.base/unix/native/libnet/PlainSocketImpl.c b/src/java.base/unix/native/libnet/PlainSocketImpl.c
> --- a/src/java.base/unix/native/libnet/PlainSocketImpl.c
> +++ b/src/java.base/unix/native/libnet/PlainSocketImpl.c
> @@ -217,6 +217,9 @@
>       (*env)->SetIntField(env, fdObj, IO_fd_fdID, fd);
>   }
> 
> +// private helper for socketConnect to describe errors
> +static void sockErrDescribe(JNIEnv *env, char* errbuf, char* msg, jobject ia, jint port);
> +
>   /*
>    * inetAddress is the address object passed to the socket connect
>    * call.
> @@ -230,6 +233,7 @@
>                                               jobject iaObj, jint port,
>                                               jint timeout)
>   {
> +    char errmsg[256];
>       jint localport = (*env)->GetIntField(env, this, psi_localportID);
>       int len = 0;
>       /* fdObj is the FileDescriptor field on this */
> @@ -249,7 +253,8 @@
>       int connect_rv = -1;
> 
>       if (IS_NULL(fdObj)) {
> -        JNU_ThrowByName(env, JNU_JAVANETPKG "SocketException", "Socket closed");
> +        sockErrDescribe(env, errmsg, "Socket closed", iaObj, port);
> +        JNU_ThrowByName(env, JNU_JAVANETPKG "SocketException", errmsg);
>           return;
>       } else {
>           fd = (*env)->GetIntField(env, fdObj, IO_fd_fdID);
> @@ -329,8 +334,8 @@
>               jlong prevNanoTime = JVM_NanoTime(env, 0);
> 
>               if (errno != EINPROGRESS) {
> -                NET_ThrowByNameWithLastError(env, JNU_JAVANETPKG "ConnectException",
> -                             "connect failed");
> +                sockErrDescribe(env, errmsg, "connect failed", iaObj, port);
> +                NET_ThrowByNameWithLastError(env, JNU_JAVANETPKG "ConnectException", errmsg);
>                   SET_BLOCKING(fd);
>                   return;
>               }
> @@ -372,8 +377,8 @@
>               } /* while */
> 
>               if (connect_rv == 0) {
> -                JNU_ThrowByName(env, JNU_JAVANETPKG "SocketTimeoutException",
> -                            "connect timed out");
> +                sockErrDescribe(env, errmsg, "connect timed out", iaObj, port);
> +                JNU_ThrowByName(env, JNU_JAVANETPKG "SocketTimeoutException", errmsg);
> 
>                   /*
>                    * Timeout out but connection may still be established.
> @@ -419,36 +424,37 @@
>            * a more descriptive exception text is used.
>            */
>           if (connect_rv == -1 && errno == EINVAL) {
> -            JNU_ThrowByName(env, JNU_JAVANETPKG "SocketException",
> -                "Invalid argument or cannot assign requested address");
> +            sockErrDescribe(errmsg, "Invalid argument or cannot assign requested address", iaObj, port)
> +            JNU_ThrowByName(env, JNU_JAVANETPKG "SocketException", errmsg);
>               return;
>           }
>   #endif
>   #if defined(EPROTO)
>           if (errno == EPROTO) {
> -            NET_ThrowByNameWithLastError(env, JNU_JAVANETPKG "ProtocolException",
> -                           "Protocol error");
> +            sockErrDescribe(env, errmsg, "Protocol error", iaObj, port);
> +            NET_ThrowByNameWithLastError(env, JNU_JAVANETPKG "ProtocolException", errmsg);
>               return;
>           }
>   #endif
>           if (errno == ECONNREFUSED) {
> -            NET_ThrowByNameWithLastError(env, JNU_JAVANETPKG "ConnectException",
> -                           "Connection refused");
> +            sockErrDescribe(env, errmsg, "Connection refused", iaObj, port);
> +            NET_ThrowByNameWithLastError(env, JNU_JAVANETPKG "ConnectException", errmsg);
>           } else if (errno == ETIMEDOUT) {
> -            NET_ThrowByNameWithLastError(env, JNU_JAVANETPKG "ConnectException",
> -                           "Connection timed out");
> +            sockErrDescribe(env, errmsg, "Connection timed out", iaObj, port);
> +            NET_ThrowByNameWithLastError(env, JNU_JAVANETPKG "ConnectException", errmsg);
>           } else if (errno == EHOSTUNREACH) {
> -            NET_ThrowByNameWithLastError(env, JNU_JAVANETPKG "NoRouteToHostException",
> -                           "Host unreachable");
> +            sockErrDescribe(env, errmsg, "Host unreachable", iaObj, port);
> +            NET_ThrowByNameWithLastError(env, JNU_JAVANETPKG "NoRouteToHostException", errmsg);
>           } else if (errno == EADDRNOTAVAIL) {
> -            NET_ThrowByNameWithLastError(env, JNU_JAVANETPKG "NoRouteToHostException",
> -                             "Address not available");
> +            sockErrDescribe(env, errmsg, "Address not available", iaObj, port);
> +            NET_ThrowByNameWithLastError(env, JNU_JAVANETPKG "NoRouteToHostException", errmsg);
>           } else if ((errno == EISCONN) || (errno == EBADF)) {
> -            JNU_ThrowByName(env, JNU_JAVANETPKG "SocketException",
> -                            "Socket closed");
> +            sockErrDescribe(env, errmsg, "Socket closed", iaObj, port);
> +            JNU_ThrowByName(env, JNU_JAVANETPKG "SocketException", errmsg);
>           } else {
> +            sockErrDescribe(env, errmsg, "connect failed", iaObj, port);
>               JNU_ThrowByNameWithMessageAndLastError
> -                (env, JNU_JAVANETPKG "SocketException", "connect failed");
> +                (env, JNU_JAVANETPKG "SocketException", errmsg);
>           }
>           return;
>       }
> @@ -479,6 +485,15 @@
>       }
>   }
> 
> +static void sockErrDescribe(JNIEnv *env, char* errbuf, char* msg, jobject ia, jint port) {
> +    jclass obj = (*env)->FindClass(env, "java/lang/Object");
> +    jmethodID toStr = (*env)->GetMethodID(env, obj, "toString", "()Ljava/lang/String;");
> +    jstring iaJstr = (*env)->CallObjectMethod(env, ia, toStr);
> +    const char *iaStr = (*env)->GetStringUTFChars(env, iaJstr, 0);
> +    jio_snprintf(errbuf, 128, "%s: %s:%d", msg, iaStr, port);
> +    (*env)->ReleaseStringUTFChars(env, iaJstr, iaStr);
> +}
> +
>   /*
>    * Class:     java_net_PlainSocketImpl
>    * Method:    socketBind
> 
> 
>