Re: (tomcat) branch main updated: Fix matching of the pattern documented in the javadoc

On Fri, Jul 31, 2026 at 2:49=E2=80=AFPM Konstantin Kolinko
<[email protected]> wrote:
>
> =D0=BF=D1=82, 31 =D0=B8=D1=8E=D0=BB. 2026=E2=80=AF=D0=B3. =D0=B2 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 javad=
oc
> > 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 "mat=
ching",
> 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.j=
ava

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=C3=A9my

> Best regards,
> Konstantin Kolinko
>
> >     Also add a test case for the noCompressionUserAgents feature, coaut=
hored
> >     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/a=
pache/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 =3D request.getMimeHeade=
rs().getValue("user-agent");
> >                  if (userAgentValueMB !=3D null) {
> >                      String userAgentValue =3D userAgentValueMB.toStrin=
g();
> > -                    if (noCompressionUserAgents.matcher(userAgentValue=
).matches()) {
> > +                    if (noCompressionUserAgents.matcher(userAgentValue=
).find()) {
> >                          return false;
> >                      }
> >                  }
> > diff --git a/test/org/apache/coyote/TestCompressionConfigUserAgents.jav=
a 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 w=
ith
> > + *  this work for additional information regarding copyright ownership=
.
> > + *  The ASF licenses this file to You under the Apache License, Versio=
n 2.0
> > + *  (the "License"); you may not use this file except in compliance wi=
th
> > + *  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, softwar=
e
> > + *  distributed under the License is distributed on an "AS IS" BASIS,
> > + *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or im=
plied.
> > + *  See the License for the specific language governing permissions an=
d
> > + *  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 =3D new CompressionConfig();
> > +        config.setNoCompressionUserAgents("gorilla|MSIE|tigrus");
> > +
> > +        Request request =3D new Request();
> > +        request.getMimeHeaders().addValue("accept-encoding").setString=
("gzip");
> > +        Response response;
> > +
> > +        // Force mode (compressionLevel =3D=3D 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 =3D createResponse();
> > +        request.getMimeHeaders().addValue("user-agent").setString("Moz=
illa/4.0 (compatible; MSIE 6.0; Windows NT 5.1)");
> > +        Assert.assertFalse(config.useCompression(request, response));
> > +
> > +        // No user-agent header should be compressed
> > +        response =3D createResponse();
> > +        request.getMimeHeaders().removeHeader("user-agent");
> > +        Assert.assertTrue(config.useCompression(request, response));
> > +
> > +        // User-agent not matching the pattern should be compressed
> > +        response =3D createResponse();
> > +        request.getMimeHeaders().removeHeader("user-agent");
> > +        request.getMimeHeaders().addValue("user-agent").setString("Moz=
illa/5.0 (X11; Linux x86_64)");
> > +        Assert.assertTrue(config.useCompression(request, response));
> > +
> > +        // Force mode skips the user-agent check
> > +        response =3D createResponse();
> > +        config.setCompression("force");
> > +        request.getMimeHeaders().removeHeader("user-agent");
> > +        request.getMimeHeaders().addValue("user-agent").setString("Moz=
illa/4.0 (compatible; MSIE 6.0; Windows NT 5.1)");
> > +        Assert.assertTrue(config.useCompression(request, response));
> > +    }
> > +
> > +    private Response createResponse() {
> > +        Response response =3D 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 detec=
ted.
> >          (markt)
> >        </fix>
> > +      <fix>
> > +        Fix matching the compression config
> > +        <code>noCompressionUserAgents</code> with patterns of the styl=
e
> > +        of the example <code>gorilla|desesplorer|tigrus</code> pattern
> > +        documented in the javadoc. (remm)
> > +      </fix>
> >      </changelog>
> >    </subsection>
> >    <subsection name=3D"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]
>