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

2015-07-14 Thread 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

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

2015-07-13 Thread Christopher Schultz 
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.)

-chris



signature.asc
Description: OpenPGP digital signature