You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2014/03/04 19:10:56 UTC

svn commit: r1574167 - /tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java

Author: remm
Date: Tue Mar  4 18:10:56 2014
New Revision: 1574167

URL: http://svn.apache.org/r1574167
Log:
Avoid possible NPE.

Modified:
    tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java

Modified: tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1574167&r1=1574166&r2=1574167&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Tue Mar  4 18:10:56 2014
@@ -426,10 +426,12 @@ public class CoyoteAdapter implements Ad
             if (!request.isAsync() && !comet) {
                 request.finishRequest();
                 response.finishResponse();
-                request.getMappingData().context.logAccess(
-                        request, response,
-                        System.currentTimeMillis() - req.getStartTime(),
-                        false);
+                if (request.getMappingData().context != null) {
+                    request.getMappingData().context.logAccess(
+                            request, response,
+                            System.currentTimeMillis() - req.getStartTime(),
+                            false);
+                }
             }
 
         } catch (IOException e) {



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


Re: svn commit: r1574167 - /tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java

Posted by Rémy Maucherat <re...@apache.org>.
2014-03-05 11:07 GMT+01:00 Konstantin Kolinko <kn...@gmail.com>:

> In my opinion, this is wrong. One should not skip access logging.
> If context==null, it can be logged via CoyoteAdapter.log(...) that
> logs into (ROOT app or Host or Engine).
>

Ok, I'll try to improve it. I see the call at the end of event is unsafe as
well, since it would skip recycling if something bad happens.

>
> There are several other places in CoyoteAdapter where
> context.logAccess(..) is called. Some are with similar NPE checks,
> some are not.
>
> Rémy

Re: svn commit: r1574167 - /tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java

Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-03-04 22:10 GMT+04:00  <re...@apache.org>:
> Author: remm
> Date: Tue Mar  4 18:10:56 2014
> New Revision: 1574167
>
> URL: http://svn.apache.org/r1574167
> Log:
> Avoid possible NPE.
>
> Modified:
>     tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
>
> Modified: tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1574167&r1=1574166&r2=1574167&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Tue Mar  4 18:10:56 2014
> @@ -426,10 +426,12 @@ public class CoyoteAdapter implements Ad
>              if (!request.isAsync() && !comet) {
>                  request.finishRequest();
>                  response.finishResponse();
> -                request.getMappingData().context.logAccess(
> -                        request, response,
> -                        System.currentTimeMillis() - req.getStartTime(),
> -                        false);
> +                if (request.getMappingData().context != null) {
> +                    request.getMappingData().context.logAccess(
> +                            request, response,
> +                            System.currentTimeMillis() - req.getStartTime(),
> +                            false);
> +                }
>              }
>
>          } catch (IOException e) {
>


In my opinion, this is wrong. One should not skip access logging.
If context==null, it can be logged via CoyoteAdapter.log(...) that
logs into (ROOT app or Host or Engine).

There are several other places in CoyoteAdapter where
context.logAccess(..) is called. Some are with similar NPE checks,
some are not.

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