You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2018/05/14 13:45:05 UTC
svn commit: r1831568 - in /tomcat/trunk:
java/org/apache/coyote/AbstractProcessor.java
java/org/apache/coyote/LocalStrings.properties
java/org/apache/coyote/http11/Http11Processor.java webapps/docs/changelog.xml
Author: markt
Date: Mon May 14 13:45:05 2018
New Revision: 1831568
URL: http://svn.apache.org/viewvc?rev=1831568&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62371
Improve logging of Host validation failures.
Modified:
tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java
tomcat/trunk/java/org/apache/coyote/LocalStrings.properties
tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java?rev=1831568&r1=1831567&r2=1831568&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java Mon May 14 13:45:05 2018
@@ -29,6 +29,7 @@ import org.apache.tomcat.util.ExceptionU
import org.apache.tomcat.util.buf.ByteChunk;
import org.apache.tomcat.util.buf.MessageBytes;
import org.apache.tomcat.util.http.parser.Host;
+import org.apache.tomcat.util.log.UserDataHelper;
import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
import org.apache.tomcat.util.net.DispatchType;
import org.apache.tomcat.util.net.SSLSupport;
@@ -61,6 +62,7 @@ public abstract class AbstractProcessor
*/
private ErrorState errorState = ErrorState.NONE;
+ protected final UserDataHelper userDataHelper;
public AbstractProcessor(Adapter adapter) {
this(adapter, new Request(), new Response());
@@ -75,6 +77,7 @@ public abstract class AbstractProcessor
response.setHook(this);
request.setResponse(response);
request.setHook(this);
+ userDataHelper = new UserDataHelper(getLog());
}
/**
@@ -294,8 +297,23 @@ public abstract class AbstractProcessor
} catch (IllegalArgumentException e) {
// IllegalArgumentException indicates that the host name is invalid
+ UserDataHelper.Mode logMode = userDataHelper.getNextMode();
+ if (logMode != null) {
+ String message = sm.getString("abstractProcessor.hostInvalid", valueMB.toString());
+ switch (logMode) {
+ case INFO_THEN_DEBUG:
+ message += sm.getString("abstractProcessor.fallToDebug");
+ //$FALL-THROUGH$
+ case INFO:
+ getLog().info(message, e);
+ break;
+ case DEBUG:
+ getLog().debug(message, e);
+ }
+ }
+
response.setStatus(400);
- setErrorState(ErrorState.CLOSE_CLEAN, null);
+ setErrorState(ErrorState.CLOSE_CLEAN, e);
}
}
Modified: tomcat/trunk/java/org/apache/coyote/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/LocalStrings.properties?rev=1831568&r1=1831567&r2=1831568&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/coyote/LocalStrings.properties Mon May 14 13:45:05 2018
@@ -23,7 +23,9 @@ abstractConnectionHandler.socketexceptio
abstractConnectionHandler.negotiatedProcessor.fail=Failed to create Processor for negotiated protocol [{0}]
abstractConnectionHandler.upgradeCreate=Created upgrade processor [{0}] for socket wrapper [{1}]
+abstractProcessor.fallToDebug=\n Note: further occurrences of request parsing errors will be logged at DEBUG level.
abstractProcessor.httpupgrade.notsupported=HTTP upgrade is not supported by this protocol
+abstractProcessor.hostInvalid=The host [{0}] is not valid
abstractProcessor.noExecute=Unable to transfer processing to a container thread because this Processor is not currently associated with a SocketWrapper
abstractProcessor.nonContainerThreadError=An error occurred in processing while on a non-container thread. The connection will be closed immediately
abstractProcessor.pushrequest.notsupported=Server push requests are not supported by this protocol
Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=1831568&r1=1831567&r2=1831568&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java Mon May 14 13:45:05 2018
@@ -73,8 +73,6 @@ public class Http11Processor extends Abs
private final AbstractHttp11Protocol<?> protocol;
- private final UserDataHelper userDataHelper;
-
/**
* Input.
*/
@@ -151,8 +149,6 @@ public class Http11Processor extends Abs
super(adapter);
this.protocol = protocol;
- userDataHelper = new UserDataHelper(log);
-
httpParser = new HttpParser(protocol.getRelaxedPathChars(),
protocol.getRelaxedQueryChars());
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1831568&r1=1831567&r2=1831568&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Mon May 14 13:45:05 2018
@@ -109,6 +109,9 @@
Relax Host validation by removing the requirement that the final
component of a FQDN must be alphabetic. (markt)
</fix>
+ <fix>
+ <bug>62371</bug>: Improve logging of Host validation failures. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1831568 - in /tomcat/trunk:
java/org/apache/coyote/AbstractProcessor.java
java/org/apache/coyote/LocalStrings.properties
java/org/apache/coyote/http11/Http11Processor.java webapps/docs/changelog.xml
Posted by Mark Thomas <ma...@apache.org>.
On 14/05/18 15:18, Konstantin Kolinko wrote:
> 2018-05-14 16:45 GMT+03:00 <ma...@apache.org>:
>> Author: markt
>> Date: Mon May 14 13:45:05 2018
>> New Revision: 1831568
>>
>> URL: http://svn.apache.org/viewvc?rev=1831568&view=rev
>> Log:
>> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62371
>> Improve logging of Host validation failures.
>>
>> Modified:
>> tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java
>> tomcat/trunk/java/org/apache/coyote/LocalStrings.properties
>> tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
>> tomcat/trunk/webapps/docs/changelog.xml
>>
>
> Mark,
>
> 1) Documentation.
>
> The cases when UserDataHelper.CONFIG is used are listed at "System
> properties" page,
> http://tomcat.apache.org/tomcat-9.0-doc/config/systemprops.html#Logging
Tx for the heads up. I'll get that updated.
> 2) I wonder: there are many instances of Processor? One for each
> request processing thread?
> I wonder whether this implemenation of silencing with UserDataHelper
> works, as there are many copies of it. Though their total count is
> limited, so the log message will be silenced eventually.
>
> Looking at the history, such handling of UserDataHelper is here since
> r1489437 (5 years ago), updated at r1655973.
I noticed that too. I opted for the status quo. Thinking about it, the
issue is that if you make it static there will be a single instance
shared by all sub-classes. That isn't ideal either. I guess it could be
moved to the protocol but I haven't looked at the code to see how messy
(or not) that gets.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1831568 - in /tomcat/trunk: java/org/apache/coyote/AbstractProcessor.java
java/org/apache/coyote/LocalStrings.properties java/org/apache/coyote/http11/Http11Processor.java
webapps/docs/changelog.xml
Posted by Konstantin Kolinko <kn...@gmail.com>.
2018-05-14 16:45 GMT+03:00 <ma...@apache.org>:
> Author: markt
> Date: Mon May 14 13:45:05 2018
> New Revision: 1831568
>
> URL: http://svn.apache.org/viewvc?rev=1831568&view=rev
> Log:
> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62371
> Improve logging of Host validation failures.
>
> Modified:
> tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java
> tomcat/trunk/java/org/apache/coyote/LocalStrings.properties
> tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
> tomcat/trunk/webapps/docs/changelog.xml
>
Mark,
1) Documentation.
The cases when UserDataHelper.CONFIG is used are listed at "System
properties" page,
http://tomcat.apache.org/tomcat-9.0-doc/config/systemprops.html#Logging
2) I wonder: there are many instances of Processor? One for each
request processing thread?
I wonder whether this implemenation of silencing with UserDataHelper
works, as there are many copies of it. Though their total count is
limited, so the log message will be silenced eventually.
Looking at the history, such handling of UserDataHelper is here since
r1489437 (5 years ago), updated at r1655973.
> Modified: tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java?rev=1831568&r1=1831567&r2=1831568&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java (original)
> +++ tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java Mon May 14 13:45:05 2018
> @@ -29,6 +29,7 @@ import org.apache.tomcat.util.ExceptionU
> import org.apache.tomcat.util.buf.ByteChunk;
> import org.apache.tomcat.util.buf.MessageBytes;
> import org.apache.tomcat.util.http.parser.Host;
> +import org.apache.tomcat.util.log.UserDataHelper;
> import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
> import org.apache.tomcat.util.net.DispatchType;
> import org.apache.tomcat.util.net.SSLSupport;
> @@ -61,6 +62,7 @@ public abstract class AbstractProcessor
> */
> private ErrorState errorState = ErrorState.NONE;
>
> + protected final UserDataHelper userDataHelper;
>
> public AbstractProcessor(Adapter adapter) {
> this(adapter, new Request(), new Response());
> @@ -75,6 +77,7 @@ public abstract class AbstractProcessor
> response.setHook(this);
> request.setResponse(response);
> request.setHook(this);
> + userDataHelper = new UserDataHelper(getLog());
> }
>
> /**
> @@ -294,8 +297,23 @@ public abstract class AbstractProcessor
>
> } catch (IllegalArgumentException e) {
> // IllegalArgumentException indicates that the host name is invalid
> + UserDataHelper.Mode logMode = userDataHelper.getNextMode();
> + if (logMode != null) {
> + String message = sm.getString("abstractProcessor.hostInvalid", valueMB.toString());
> + switch (logMode) {
> + case INFO_THEN_DEBUG:
> + message += sm.getString("abstractProcessor.fallToDebug");
> + //$FALL-THROUGH$
> + case INFO:
> + getLog().info(message, e);
> + break;
> + case DEBUG:
> + getLog().debug(message, e);
> + }
> + }
> +
> response.setStatus(400);
> - setErrorState(ErrorState.CLOSE_CLEAN, null);
> + setErrorState(ErrorState.CLOSE_CLEAN, e);
> }
> }
>
>
> Modified: tomcat/trunk/java/org/apache/coyote/LocalStrings.properties
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/LocalStrings.properties?rev=1831568&r1=1831567&r2=1831568&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/LocalStrings.properties (original)
> +++ tomcat/trunk/java/org/apache/coyote/LocalStrings.properties Mon May 14 13:45:05 2018
> @@ -23,7 +23,9 @@ abstractConnectionHandler.socketexceptio
> abstractConnectionHandler.negotiatedProcessor.fail=Failed to create Processor for negotiated protocol [{0}]
> abstractConnectionHandler.upgradeCreate=Created upgrade processor [{0}] for socket wrapper [{1}]
>
> +abstractProcessor.fallToDebug=\n Note: further occurrences of request parsing errors will be logged at DEBUG level.
> abstractProcessor.httpupgrade.notsupported=HTTP upgrade is not supported by this protocol
> +abstractProcessor.hostInvalid=The host [{0}] is not valid
> abstractProcessor.noExecute=Unable to transfer processing to a container thread because this Processor is not currently associated with a SocketWrapper
> abstractProcessor.nonContainerThreadError=An error occurred in processing while on a non-container thread. The connection will be closed immediately
> abstractProcessor.pushrequest.notsupported=Server push requests are not supported by this protocol
>
> Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=1831568&r1=1831567&r2=1831568&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java (original)
> +++ tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java Mon May 14 13:45:05 2018
> @@ -73,8 +73,6 @@ public class Http11Processor extends Abs
>
> private final AbstractHttp11Protocol<?> protocol;
>
> - private final UserDataHelper userDataHelper;
> -
> /**
> * Input.
> */
> @@ -151,8 +149,6 @@ public class Http11Processor extends Abs
> super(adapter);
> this.protocol = protocol;
>
> - userDataHelper = new UserDataHelper(log);
> -
> httpParser = new HttpParser(protocol.getRelaxedPathChars(),
> protocol.getRelaxedQueryChars());
>
>
> Modified: tomcat/trunk/webapps/docs/changelog.xml
> URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1831568&r1=1831567&r2=1831568&view=diff
> ==============================================================================
> --- tomcat/trunk/webapps/docs/changelog.xml (original)
> +++ tomcat/trunk/webapps/docs/changelog.xml Mon May 14 13:45:05 2018
> @@ -109,6 +109,9 @@
> Relax Host validation by removing the requirement that the final
> component of a FQDN must be alphabetic. (markt)
> </fix>
> + <fix>
> + <bug>62371</bug>: Improve logging of Host validation failures. (markt)
> + </fix>
> </changelog>
> </subsection>
> <subsection name="Jasper">
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org