tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <knst.koli...@gmail.com>
Subject 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
Date Mon, 14 May 2018 14:18:13 GMT
2018-05-14 16:45 GMT+03:00  <markt@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


Mime
View raw message