Re: svn commit: r1690279 - /tomcat/native/trunk/native/src/network.c

[jean-frederic clere]

On 07/13/2015 07:35 PM, Christopher Schultz wrote:
Jean-Frederic,
On 7/10/15 10:48 AM, jfcl...@apache.org wrote:
Author: jfclere
Date: Fri Jul 10 15:48:01 2015
New Revision: 1690279

URL: http://svn.apache.org/r1690279
Log:
Make sure we don't core the JVM if called wrongly from a destroyed or closed 
socket.

Modified:
     tomcat/native/trunk/native/src/network.c

Modified: tomcat/native/trunk/native/src/network.c
URL: 
http://svn.apache.org/viewvc/tomcat/native/trunk/native/src/network.c?rev=1690279&r1=1690278&r2=1690279&view=diff
==============================================================================
--- tomcat/native/trunk/native/src/network.c (original)
+++ tomcat/native/trunk/native/src/network.c Fri Jul 10 15:48:01 2015
@@ -1152,8 +1152,10 @@ TCN_IMPLEMENT_CALL(jint, Socket, optSet)
      if (!s->sock) {
          return APR_ENOTSOCK;
      }
-    else
-        return (jint)(*s->net->opt_set)(s->opaque, (apr_int32_t)opt, 
(apr_int32_t)on);
+    if(!s->net) {
+        return -(jint)APR_EINVALSOCK;
+    }
+    return (jint)(*s->net->opt_set)(s->opaque, (apr_int32_t)opt, 
(apr_int32_t)on);
  }

  TCN_IMPLEMENT_CALL(jint, Socket, optGet)(TCN_STDARGS, jlong sock,
@@ -1163,12 +1165,16 @@ TCN_IMPLEMENT_CALL(jint, Socket, optGet)
      apr_int32_t on = 0;

      UNREFERENCED(o);
-    if (!s->sock)
+    if (!s->sock) {
          tcn_ThrowAPRException(e, APR_ENOTSOCK);
-    else {
-        TCN_THROW_IF_ERR((*s->net->opt_get)(s->opaque, (apr_int32_t)opt,
-                                            &on), on);
+        return APR_ENOTSOCK;
+    }
+    if(!s->net) {
+        tcn_ThrowAPRException(e, APR_EINVALSOCK);
+        return -(jint)APR_EINVALSOCK;
      }
+    TCN_THROW_IF_ERR((*s->net->opt_get)(s->opaque, (apr_int32_t)opt,
+                                        &on), on);
  cleanup:
      return (jint)on;
  }
@@ -1181,9 +1187,11 @@ TCN_IMPLEMENT_CALL(jint, Socket, timeout
      UNREFERENCED(o);
      TCN_ASSERT(s->opaque != NULL);
      if (!sock) {
-        tcn_ThrowAPRException(e, APR_ENOTSOCK);
          return APR_ENOTSOCK;
      }
+    if(!s->net) {
+        return -(jint)APR_EINVALSOCK;
+    }
      return (jint)(*s->net->timeout_set)(s->opaque, J2T(timeout));
  }

@@ -1197,6 +1205,10 @@ TCN_IMPLEMENT_CALL(jlong, Socket, timeou
          tcn_ThrowAPRException(e, APR_ENOTSOCK);
          return 0;
      }
+    if(!s->net) {
+        tcn_ThrowAPRException(e, APR_EINVALSOCK);
+        return -(jint)APR_EINVALSOCK;
+    }
      TCN_ASSERT(s->opaque != NULL);

      TCN_THROW_IF_ERR((*s->net->timeout_get)(s->opaque, &timeout), timeout);

It looks like we inconsistently throw an exception and/or return
APR_EINVALSOCK. Should we always do one or the other? (Unfortunately,
tcn_ThrowAPRException always throws a java.lang.Error and not something
more specific. It might be nice to change that to something like
APRException.)

Actually the getters are throwning but the setters aren't that isn't 
very clean :-(

The return after the throw makes the compiler happy that is dirty.

Cheers

Jean-Frederic


-chris



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org