From ca78919b42354bbdd553360c4485bcae8600f3f4 Mon Sep 17 00:00:00 2001 From: markt Date: Tue, 27 Sep 2011 20:15:41 +0000 Subject: [PATCH] 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. git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1176590 13f79535-47bb-0310-9956-ffa450edef68 --- java/org/apache/catalina/connector/CoyoteAdapter.java | 6 +----- java/org/apache/coyote/ajp/AbstractAjpProcessor.java | 7 +++---- java/org/apache/coyote/ajp/AjpAprProcessor.java | 2 +- java/org/apache/coyote/ajp/AjpNioProcessor.java | 2 +- java/org/apache/coyote/ajp/AjpProcessor.java | 2 +- java/org/apache/coyote/http11/AbstractHttp11Processor.java | 13 ++++++------- 6 files changed, 13 insertions(+), 19 deletions(-) diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java b/java/org/apache/catalina/connector/CoyoteAdapter.java index 428868164..2d4643f0b 100644 --- a/java/org/apache/catalina/connector/CoyoteAdapter.java +++ b/java/org/apache/catalina/connector/CoyoteAdapter.java @@ -466,10 +466,8 @@ public class CoyoteAdapter implements Adapter { 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); @@ -511,9 +509,7 @@ public class CoyoteAdapter implements Adapter { } catch (Throwable t) { ExceptionUtils.handleThrowable(t); log.warn(sm.getString("coyoteAdapter.accesslogFail"), t); - } - - if (create) { + } finally { request.recycle(); response.recycle(); } diff --git a/java/org/apache/coyote/ajp/AbstractAjpProcessor.java b/java/org/apache/coyote/ajp/AbstractAjpProcessor.java index fceefdddf..056c6caa5 100644 --- a/java/org/apache/coyote/ajp/AbstractAjpProcessor.java +++ b/java/org/apache/coyote/ajp/AbstractAjpProcessor.java @@ -759,7 +759,6 @@ public abstract class AbstractAjpProcessor extends AbstractProcessor { secret = true; if (!tmpMB.equals(requiredSecret)) { response.setStatus(403); - adapter.log(request, response, 0); error = true; } } @@ -776,7 +775,6 @@ public abstract class AbstractAjpProcessor extends AbstractProcessor { // Check if secret was submitted if required if ((requiredSecret != null) && !secret) { response.setStatus(403); - adapter.log(request, response, 0); error = true; } @@ -810,6 +808,9 @@ public abstract class AbstractAjpProcessor extends AbstractProcessor { MessageBytes valueMB = request.getMimeHeaders().getValue("host"); parseHost(valueMB); + if (error) { + adapter.log(request, response, 0); + } } @@ -825,7 +826,6 @@ public abstract class AbstractAjpProcessor extends AbstractProcessor { request.serverName().duplicate(request.localName()); } catch (IOException e) { response.setStatus(400); - adapter.log(request, response, 0); error = true; } return; @@ -877,7 +877,6 @@ public abstract class AbstractAjpProcessor extends AbstractProcessor { error = true; // 400 - Bad request response.setStatus(400); - adapter.log(request, response, 0); break; } port = port + (charValue * mult); diff --git a/java/org/apache/coyote/ajp/AjpAprProcessor.java b/java/org/apache/coyote/ajp/AjpAprProcessor.java index db4559047..eca67e10a 100644 --- a/java/org/apache/coyote/ajp/AjpAprProcessor.java +++ b/java/org/apache/coyote/ajp/AjpAprProcessor.java @@ -182,7 +182,7 @@ public class AjpAprProcessor extends AbstractAjpProcessor { } } - if (!cping && endpoint.isPaused()) { + if (!error && !cping && endpoint.isPaused()) { // 503 - Service unavailable response.setStatus(503); adapter.log(request, response, 0); diff --git a/java/org/apache/coyote/ajp/AjpNioProcessor.java b/java/org/apache/coyote/ajp/AjpNioProcessor.java index 4940a38fa..29f513513 100644 --- a/java/org/apache/coyote/ajp/AjpNioProcessor.java +++ b/java/org/apache/coyote/ajp/AjpNioProcessor.java @@ -169,7 +169,7 @@ public class AjpNioProcessor extends AbstractAjpProcessor { } } - if (!cping && endpoint.isPaused()) { + if (!error && !cping && endpoint.isPaused()) { // 503 - Service unavailable response.setStatus(503); adapter.log(request, response, 0); diff --git a/java/org/apache/coyote/ajp/AjpProcessor.java b/java/org/apache/coyote/ajp/AjpProcessor.java index 3941f18df..5998989d2 100644 --- a/java/org/apache/coyote/ajp/AjpProcessor.java +++ b/java/org/apache/coyote/ajp/AjpProcessor.java @@ -185,7 +185,7 @@ public class AjpProcessor extends AbstractAjpProcessor { } } - if (!cping && endpoint.isPaused()) { + if (!error && !cping && endpoint.isPaused()) { // 503 - Service unavailable response.setStatus(503); adapter.log(request, response, 0); diff --git a/java/org/apache/coyote/http11/AbstractHttp11Processor.java b/java/org/apache/coyote/http11/AbstractHttp11Processor.java index 1b8dd9652..cc7a8231e 100644 --- a/java/org/apache/coyote/http11/AbstractHttp11Processor.java +++ b/java/org/apache/coyote/http11/AbstractHttp11Processor.java @@ -893,7 +893,6 @@ public abstract class AbstractHttp11Processor extends AbstractProcessor { if (endpoint.isPaused()) { // 503 - Service unavailable response.setStatus(503); - adapter.log(request, response, 0); error = true; } else { request.setStartTime(System.currentTimeMillis()); @@ -1083,7 +1082,6 @@ public abstract class AbstractHttp11Processor extends AbstractProcessor { " Unsupported HTTP version \""+protocolMB+"\""); } response.setStatus(505); - adapter.log(request, response, 0); } MessageBytes methodMB = request.method(); @@ -1179,7 +1177,6 @@ public abstract class AbstractHttp11Processor extends AbstractProcessor { error = true; // 501 - Unimplemented response.setStatus(501); - adapter.log(request, response, 0); } startPos = commaPos + 1; commaPos = transferEncodingValue.indexOf(',', startPos); @@ -1195,7 +1192,6 @@ public abstract class AbstractHttp11Processor extends AbstractProcessor { " Unsupported transfer encoding \""+encodingName+"\""); } response.setStatus(501); - adapter.log(request, response, 0); } } @@ -1218,7 +1214,6 @@ public abstract class AbstractHttp11Processor extends AbstractProcessor { " host header missing"); } response.setStatus(400); - adapter.log(request, response, 0); } parseHost(valueMB); @@ -1248,6 +1243,10 @@ public abstract class AbstractHttp11Processor extends AbstractProcessor { request.setAttribute("org.apache.tomcat.comet.timeout.support", Boolean.TRUE); } + + if (error) { + adapter.log(request, response, 0); + } } @@ -1467,7 +1466,6 @@ public abstract class AbstractHttp11Processor extends AbstractProcessor { error = true; // 400 - Bad request response.setStatus(400); - adapter.log(request, response, 0); break; } port = port + (charValue * mult); @@ -1548,8 +1546,9 @@ public abstract class AbstractHttp11Processor extends AbstractProcessor { ExceptionUtils.handleThrowable(t); getLog().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 { -- 2.11.0