On Fri, Jul 31, 2026 at 2:49 PM Konstantin Kolinko
<[email protected]> wrote:
>
> пт, 31 июл. 2026 г. в 12:36, <[email protected]>:
> >
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > rmaucher pushed a commit to branch main
> > in repository https://gitbox.apache.org/repos/asf/tomcat.git
> >
> >
> > The following commit(s) were added to refs/heads/main by this push:
> >      new 1f29baf341 Fix matching of the pattern documented in the javadoc
> > 1f29baf341 is described below
> >
> > commit 1f29baf341220048c66e30aedf43c5b7d0266060
> > Author: remm <[email protected]>
> > AuthorDate: Fri Jul 31 11:35:54 2026 +0200
> >
> >     Fix matching of the pattern documented in the javadoc
>
> 1. The online documentation was correct (before your change), saying 
> "matching",
> and it was not updated by this commit.
>
> https://tomcat.apache.org/tomcat-11.0-doc/config/http.html
> see "noCompressionUserAgents"

I don't consider matching meant using matches().
The javadoc has always been using the "gorilla|desesplorer|tigrus"
example pattern, which IMO makes sense.

> 2. The code using "matches()" is there at least from the first
> revision of CompressionConfig.java (year 2017)
> when it was moved there out of org/apache/coyote/http11/Http11Processor.java

I know. There was no test case for the feature either. There are two
other occurrences of this usage, all documented with the same kind of
user-agent pattern matching which won't work. I'd rather fix it.
In CrawlerSessionManagerValve the default value is that kind of
pattern. I did not bother adding a test for this one.

>
> I know that HTTPD uses find rather than matching the whole string, but
> our code is 9+ old,
> so I think that it would be better to just align the documentation
> with the behaviour.
>
> https://httpd.apache.org/docs/current/mod/mod_setenvif.html
> see BrowserMatch, BrowserMatchNoCase

This is a bit different since in that case you can add more
directives. We don't have a valve equivalent of this one.

Rémy

> Best regards,
> Konstantin Kolinko
>
> >     Also add a test case for the noCompressionUserAgents feature, coauthored
> >     with OpenCode.
> > ---
> >  java/org/apache/coyote/CompressionConfig.java      |  2 +-
> >  .../coyote/TestCompressionConfigUserAgents.java    | 67 
> > ++++++++++++++++++++++
> >  webapps/docs/changelog.xml                         |  6 ++
> >  3 files changed, 74 insertions(+), 1 deletion(-)
> >
> > diff --git a/java/org/apache/coyote/CompressionConfig.java 
> > b/java/org/apache/coyote/CompressionConfig.java
> > index 7c5efcfec9..b07b8c10b5 100644
> > --- a/java/org/apache/coyote/CompressionConfig.java
> > +++ b/java/org/apache/coyote/CompressionConfig.java
> > @@ -375,7 +375,7 @@ public class CompressionConfig {
> >                  MessageBytes userAgentValueMB = 
> > request.getMimeHeaders().getValue("user-agent");
> >                  if (userAgentValueMB != null) {
> >                      String userAgentValue = userAgentValueMB.toString();
> > -                    if 
> > (noCompressionUserAgents.matcher(userAgentValue).matches()) {
> > +                    if 
> > (noCompressionUserAgents.matcher(userAgentValue).find()) {
> >                          return false;
> >                      }
> >                  }
> > diff --git a/test/org/apache/coyote/TestCompressionConfigUserAgents.java 
> > b/test/org/apache/coyote/TestCompressionConfigUserAgents.java
> > new file mode 100644
> > index 0000000000..43f988533f
> > --- /dev/null
> > +++ b/test/org/apache/coyote/TestCompressionConfigUserAgents.java
> > @@ -0,0 +1,67 @@
> > +/*
> > + *  Licensed to the Apache Software Foundation (ASF) under one or more
> > + *  contributor license agreements.  See the NOTICE file distributed with
> > + *  this work for additional information regarding copyright ownership.
> > + *  The ASF licenses this file to You under the Apache License, Version 2.0
> > + *  (the "License"); you may not use this file except in compliance with
> > + *  the License.  You may obtain a copy of the License at
> > + *
> > + *      http://www.apache.org/licenses/LICENSE-2.0
> > + *
> > + *  Unless required by applicable law or agreed to in writing, software
> > + *  distributed under the License is distributed on an "AS IS" BASIS,
> > + *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
> > implied.
> > + *  See the License for the specific language governing permissions and
> > + *  limitations under the License.
> > + */
> > +package org.apache.coyote;
> > +
> > +import org.junit.Assert;
> > +import org.junit.Test;
> > +
> > +public class TestCompressionConfigUserAgents {
> > +
> > +    @Test
> > +    public void testNoCompressionUserAgents() {
> > +        CompressionConfig config = new CompressionConfig();
> > +        config.setNoCompressionUserAgents("gorilla|MSIE|tigrus");
> > +
> > +        Request request = new Request();
> > +        
> > request.getMimeHeaders().addValue("accept-encoding").setString("gzip");
> > +        Response response;
> > +
> > +        // Force mode (compressionLevel == 2) skips the user-agent check,
> > +        // so use "on" mode where the check applies
> > +        config.setCompression("on");
> > +
> > +        // User-agent matching the pattern should not be compressed
> > +        response = createResponse();
> > +        
> > request.getMimeHeaders().addValue("user-agent").setString("Mozilla/4.0 
> > (compatible; MSIE 6.0; Windows NT 5.1)");
> > +        Assert.assertFalse(config.useCompression(request, response));
> > +
> > +        // No user-agent header should be compressed
> > +        response = createResponse();
> > +        request.getMimeHeaders().removeHeader("user-agent");
> > +        Assert.assertTrue(config.useCompression(request, response));
> > +
> > +        // User-agent not matching the pattern should be compressed
> > +        response = createResponse();
> > +        request.getMimeHeaders().removeHeader("user-agent");
> > +        
> > request.getMimeHeaders().addValue("user-agent").setString("Mozilla/5.0 
> > (X11; Linux x86_64)");
> > +        Assert.assertTrue(config.useCompression(request, response));
> > +
> > +        // Force mode skips the user-agent check
> > +        response = createResponse();
> > +        config.setCompression("force");
> > +        request.getMimeHeaders().removeHeader("user-agent");
> > +        
> > request.getMimeHeaders().addValue("user-agent").setString("Mozilla/4.0 
> > (compatible; MSIE 6.0; Windows NT 5.1)");
> > +        Assert.assertTrue(config.useCompression(request, response));
> > +    }
> > +
> > +    private Response createResponse() {
> > +        Response response = new Response();
> > +        response.setContentLength(4096);
> > +        response.setContentType("text/html");
> > +        return response;
> > +    }
> > +}
> > diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
> > index 173f38f99e..f550280459 100644
> > --- a/webapps/docs/changelog.xml
> > +++ b/webapps/docs/changelog.xml
> > @@ -339,6 +339,12 @@
> >          of how early in the HEADERS frame processing an error is detected.
> >          (markt)
> >        </fix>
> > +      <fix>
> > +        Fix matching the compression config
> > +        <code>noCompressionUserAgents</code> with patterns of the style
> > +        of the example <code>gorilla|desesplorer|tigrus</code> pattern
> > +        documented in the javadoc. (remm)
> > +      </fix>
> >      </changelog>
> >    </subsection>
> >    <subsection name="Jasper">
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [email protected]
> > For additional commands, e-mail: [email protected]
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to