Author: kkolinko
Date: Wed Oct 19 07:15:30 2011
New Revision: 1185998
URL: http://svn.apache.org/viewvc?rev=1185998&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51872
Ensure access log always logs the correct remote IP.
Ensure requests with multiple errors do not result in multiple access log
entries.
Modified:
tomcat/tc6.0.x/trunk/STATUS.txt
tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java
tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java
tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11Processor.java
tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1185998&r1=1185997&r2=1185998&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Wed Oct 19 07:15:30 2011
@@ -64,14 +64,6 @@ PATCHES PROPOSED TO BACKPORT:
- getStuckThreadIds() returns a list of ids. It might be useful to
have a similar method that returns Thread.getName() names.
-* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51872
- Ensure access log always logs the correct remote IP.
- Ensure requests with multiple errors do not result in multiple access log
- entries.
- http://people.apache.org/~markt/patches/2011-09-27-bug51872-tc6.patch
- +1: markt, kkolinko, rjung
- -1:
-
* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51905
Fix infinite loop in AprEndpoint shutdown if acceptor unlock
fails. Reduce timeout before forcefully closing the socket from 30s to 10s.
Modified:
tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1185998&r1=1185997&r2=1185998&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
Wed Oct 19 07:15:30 2011
@@ -24,6 +24,7 @@ import java.io.UnsupportedEncodingExcept
import org.apache.catalina.CometEvent;
import org.apache.catalina.Context;
import org.apache.catalina.Globals;
+import org.apache.catalina.Host;
import org.apache.catalina.Wrapper;
import org.apache.catalina.util.StringManager;
import org.apache.catalina.util.ServerInfo;
@@ -32,6 +33,7 @@ import org.apache.coyote.ActionCode;
import org.apache.coyote.Adapter;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.buf.B2CConverter;
import org.apache.tomcat.util.buf.ByteChunk;
import org.apache.tomcat.util.buf.CharChunk;
@@ -340,10 +342,8 @@ public class CoyoteAdapter implements Ad
Request request = (Request) req.getNote(ADAPTER_NOTES);
Response response = (Response) res.getNote(ADAPTER_NOTES);
- boolean create = false;
if (request == null) {
- create = true;
// Create objects
request = connector.createRequest();
request.setCoyoteRequest(req);
@@ -363,10 +363,29 @@ public class CoyoteAdapter implements Ad
(connector.getURIEncoding());
}
- connector.getService().getContainer().logAccess(
- request, response, time, true);
-
- if (create) {
+ try {
+ // Log at the lowest level available. logAccess() will be
+ // automatically called on parent containers.
+ boolean logged = false;
+ if (request.mappingData != null) {
+ if (request.mappingData.context != null) {
+ logged = true;
+ ((Context) request.mappingData.context).logAccess(
+ request, response, time, true);
+ } else if (request.mappingData.host != null) {
+ logged = true;
+ ((Host) request.mappingData.host).logAccess(
+ request, response, time, true);
+ }
+ }
+ if (!logged) {
+ connector.getService().getContainer().logAccess(
+ request, response, time, true);
+ }
+ } catch (Throwable t) {
+ ExceptionUtils.handleThrowable(t);
+ log.warn(sm.getString("coyoteAdapter.accesslogFail"), t);
+ } finally {
request.recycle();
response.recycle();
}
Modified: tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java?rev=1185998&r1=1185997&r2=1185998&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java Wed
Oct 19 07:15:30 2011
@@ -428,15 +428,17 @@ public class AjpAprProcessor implements
}
// Setting up filters, and parse some request headers
- rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE);
- try {
- prepareRequest();
- } catch (Throwable t) {
- log.debug(sm.getString("ajpprocessor.request.prepare"), t);
- // 400 - Internal Server Error
- response.setStatus(400);
- adapter.log(request, response, 0);
- error = true;
+ if (!error) {
+ rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE);
+ try {
+ prepareRequest();
+ } catch (Throwable t) {
+ log.debug(sm.getString("ajpprocessor.request.prepare"), t);
+ // 400 - Internal Server Error
+ response.setStatus(400);
+ adapter.log(request, response, 0);
+ error = true;
+ }
}
// Process the request in the adapter
@@ -839,7 +841,6 @@ public class AjpAprProcessor implements
secret = true;
if (!tmpMB.equals(requiredSecret)) {
response.setStatus(403);
- adapter.log(request, response, 0);
error = true;
}
}
@@ -856,7 +857,6 @@ public class AjpAprProcessor implements
// Check if secret was submitted if required
if ((requiredSecret != null) && !secret) {
response.setStatus(403);
- adapter.log(request, response, 0);
error = true;
}
@@ -890,6 +890,9 @@ public class AjpAprProcessor implements
MessageBytes valueMB = request.getMimeHeaders().getValue("host");
parseHost(valueMB);
+ if (error) {
+ adapter.log(request, response, 0);
+ }
}
@@ -905,7 +908,6 @@ public class AjpAprProcessor implements
request.serverName().duplicate(request.localName());
} catch (IOException e) {
response.setStatus(400);
- adapter.log(request, response, 0);
error = true;
}
return;
@@ -957,7 +959,6 @@ public class AjpAprProcessor implements
error = true;
// 400 - Bad request
response.setStatus(400);
- adapter.log(request, response, 0);
break;
}
port = port + (charValue * mult);
Modified: tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java?rev=1185998&r1=1185997&r2=1185998&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java Wed Oct
19 07:15:30 2011
@@ -445,15 +445,17 @@ public class AjpProcessor implements Act
}
// Setting up filters, and parse some request headers
- rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE);
- try {
- prepareRequest();
- } catch (Throwable t) {
- log.debug(sm.getString("ajpprocessor.request.prepare"), t);
- // 400 - Internal Server Error
- response.setStatus(400);
- adapter.log(request, response, 0);
- error = true;
+ if (!error) {
+ rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE);
+ try {
+ prepareRequest();
+ } catch (Throwable t) {
+ log.debug(sm.getString("ajpprocessor.request.prepare"), t);
+ // 400 - Internal Server Error
+ response.setStatus(400);
+ adapter.log(request, response, 0);
+ error = true;
+ }
}
// Process the request in the adapter
@@ -844,7 +846,6 @@ public class AjpProcessor implements Act
secret = true;
if (!tmpMB.equals(requiredSecret)) {
response.setStatus(403);
- adapter.log(request, response, 0);
error = true;
}
}
@@ -861,7 +862,6 @@ public class AjpProcessor implements Act
// Check if secret was submitted if required
if ((requiredSecret != null) && !secret) {
response.setStatus(403);
- adapter.log(request, response, 0);
error = true;
}
@@ -895,6 +895,9 @@ public class AjpProcessor implements Act
MessageBytes valueMB = request.getMimeHeaders().getValue("host");
parseHost(valueMB);
+ if (error) {
+ adapter.log(request, response, 0);
+ }
}
@@ -910,7 +913,6 @@ public class AjpProcessor implements Act
request.serverName().duplicate(request.localName());
} catch (IOException e) {
response.setStatus(400);
- adapter.log(request, response, 0);
error = true;
}
return;
@@ -962,7 +964,6 @@ public class AjpProcessor implements Act
error = true;
// 400 - Bad request
response.setStatus(400);
- adapter.log(request, response, 0);
break;
}
port = port + (charValue * mult);
Modified:
tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java?rev=1185998&r1=1185997&r2=1185998&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
Wed Oct 19 07:15:30 2011
@@ -972,8 +972,9 @@ public class Http11AprProcessor implemen
} catch (Throwable t) {
log.error(sm.getString("http11processor.request.finish"), t);
// 500 - Internal Server Error
+ // Can't add a 500 to the access log since that has already been
+ // written in the Adapter.service method.
response.setStatus(500);
- adapter.log(request, response, 0);
error = true;
}
try {
@@ -1327,7 +1328,6 @@ public class Http11AprProcessor implemen
error = true;
// Send 505; Unsupported HTTP version
response.setStatus(505);
- adapter.log(request, response, 0);
}
MessageBytes methodMB = request.method();
@@ -1425,7 +1425,6 @@ public class Http11AprProcessor implemen
error = true;
// 501 - Unimplemented
response.setStatus(501);
- adapter.log(request, response, 0);
}
startPos = commaPos + 1;
commaPos = transferEncodingValue.indexOf(',', startPos);
@@ -1437,7 +1436,6 @@ public class Http11AprProcessor implemen
error = true;
// 501 - Unimplemented
response.setStatus(501);
- adapter.log(request, response, 0);
}
}
@@ -1456,7 +1454,6 @@ public class Http11AprProcessor implemen
error = true;
// 400 - Bad request
response.setStatus(400);
- adapter.log(request, response, 0);
}
parseHost(valueMB);
@@ -1476,7 +1473,10 @@ public class Http11AprProcessor implemen
}
// Advertise comet support through a request attribute
request.setAttribute("org.apache.tomcat.comet.support", Boolean.TRUE);
-
+
+ if (error) {
+ adapter.log(request, response, 0);
+ }
}
@@ -1539,7 +1539,6 @@ public class Http11AprProcessor implemen
error = true;
// 400 - Bad request
response.setStatus(400);
- adapter.log(request, response, 0);
break;
}
port = port + (charValue * mult);
Modified:
tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java?rev=1185998&r1=1185997&r2=1185998&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
Wed Oct 19 07:15:30 2011
@@ -986,8 +986,9 @@ public class Http11NioProcessor implemen
} catch (Throwable t) {
log.error(sm.getString("http11processor.request.finish"), t);
// 500 - Internal Server Error
+ // Can't add a 500 to the access log since that has already been
+ // written in the Adapter.service method.
response.setStatus(500);
- adapter.log(request, response, 0);
error = true;
}
try {
@@ -1322,7 +1323,6 @@ public class Http11NioProcessor implemen
error = true;
// Send 505; Unsupported HTTP version
response.setStatus(505);
- adapter.log(request, response, 0);
}
MessageBytes methodMB = request.method();
@@ -1420,7 +1420,6 @@ public class Http11NioProcessor implemen
error = true;
// 501 - Unimplemented
response.setStatus(501);
- adapter.log(request, response, 0);
}
startPos = commaPos + 1;
commaPos = transferEncodingValue.indexOf(',', startPos);
@@ -1432,7 +1431,6 @@ public class Http11NioProcessor implemen
error = true;
// 501 - Unimplemented
response.setStatus(501);
- adapter.log(request, response, 0);
}
}
@@ -1451,7 +1449,6 @@ public class Http11NioProcessor implemen
error = true;
// 400 - Bad request
response.setStatus(400);
- adapter.log(request, response, 0);
}
parseHost(valueMB);
@@ -1473,6 +1470,9 @@ public class Http11NioProcessor implemen
// Advertise comet timeout support
request.setAttribute("org.apache.tomcat.comet.timeout.support",
Boolean.TRUE);
+ if (error) {
+ adapter.log(request, response, 0);
+ }
}
@@ -1535,7 +1535,6 @@ public class Http11NioProcessor implemen
error = true;
// 400 - Bad request
response.setStatus(400);
- adapter.log(request, response, 0);
break;
}
port = port + (charValue * mult);
Modified:
tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11Processor.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=1185998&r1=1185997&r2=1185998&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11Processor.java
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11Processor.java Wed
Oct 19 07:15:30 2011
@@ -893,7 +893,7 @@ public class Http11Processor implements
log.error(sm.getString("http11processor.request.finish"), t);
// 500 - Internal Server Error
response.setStatus(500);
- adapter.log(request, response, 0);
+ // No access logging since after service method
error = true;
}
try {
@@ -1201,7 +1201,6 @@ public class Http11Processor implements
" Unsupported HTTP version \""+protocolMB+"\"");
}
response.setStatus(505);
- adapter.log(request, response, 0);
}
MessageBytes methodMB = request.method();
@@ -1299,7 +1298,6 @@ public class Http11Processor implements
error = true;
// 501 - Unimplemented
response.setStatus(501);
- adapter.log(request, response, 0);
}
startPos = commaPos + 1;
commaPos = transferEncodingValue.indexOf(',', startPos);
@@ -1315,7 +1313,6 @@ public class Http11Processor implements
" Unsupported transfer encoding
\""+encodingName+"\"");
}
response.setStatus(501);
- adapter.log(request, response, 0);
}
}
@@ -1338,7 +1335,6 @@ public class Http11Processor implements
" host header missing");
}
response.setStatus(400);
- adapter.log(request, response, 0);
}
parseHost(valueMB);
@@ -1352,6 +1348,9 @@ public class Http11Processor implements
contentDelimitation = true;
}
+ if (error) {
+ adapter.log(request, response, 0);
+ }
}
@@ -1418,7 +1417,6 @@ public class Http11Processor implements
error = true;
// 400 - Bad request
response.setStatus(400);
- adapter.log(request, response, 0);
break;
}
port = port + (charValue * mult);
Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1185998&r1=1185997&r2=1185998&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Wed Oct 19 07:15:30 2011
@@ -66,6 +66,12 @@
<code>JreMemoryLeakPreventionListener</code> to allow pre-loading of
configurable
classes to avoid some classloader leaks. (slaurent)
</add>
+ <fix>
+ <bug>51872</bug>: Ensure that the access log always uses the correct
+ value for the remote IP address associated with the request and that
+ requests with multiple errors do not result in multiple entries in
+ the access log. (markt)
+ </fix>
<add>
Allow to overwrite the check for distributability
of session attributes by session implementations. (rjung)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
