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

On Mon, Aug 3, 2026 at 3:23=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 17:18, R=
=C3=A9my Maucherat <[email protected]>:
> >
> > 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:3=
6, <[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 j=
avadoc
> > > > 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.
>
> Yes,
>  the sample pattern makes sense, but it is not a real-world example.
>
> (Real names in a User-Agent header start with an Uppercase character.)
>
> Personally, I do not consider JavaDoc to be proper documentation on
> how to configure Tomcat.

Ok, so I've reverted the change then.

R=C3=A9my

> It is not there by default (only included in the fulldocs bundle), and
> we have proper documentation (User Guide and Configuration Reference)
> nowadays.
>
> > > 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/Http11Process=
or.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.
> >
>
> Patterns used for mathes() vs used for find() can be converted from
> one to another by adding either "^","$"  (anchors) or ".*" (any
> character pattern) at their ends.
>
> Case-insensitive matching can be turned on with "(?i)".
>
> So once you decide on what API to use, it is easy to tailor your
> pattern to that API.
>
> Changing between find() and matches() is not "fixing what does not
> work" - either API can be used to get the intended result if you
> adjust your pattern.
>
> > >
> > > I know that HTTPD uses find rather than matching the whole string, bu=
t
> > > 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.
>
> That was not the point. The point was the regexp patterns used by
> those directives.
>
> Those patterns are for matching using "find()", not "matches()"
> Thus they use "^Mozilla" with a "^" anchor character in the example.
>
> (As I have some background with administering Apache HTTPD, I thought
> that was your inspiration as well.)
>
> So using a pattern tailored for "find()" makes sense.
>
> Documentation and examples of real-world values:
> https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/User-=
Agent
>
>
> My problem is that you are changing behaviour in a point release,
> and this change is not properly documented.
>
> I ask to either document the change
> - be clear that it is a behaviour change in the changelog,
> - be ready to mention this in the Migration Guides,
> - update the Configuration Reference
> or to revert it.
>
> Best regards,
> Konstantin Kolinko
>
> > > >     Also add a test case for the noCompressionUserAgents feature, c=
oauthored
> > > >     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/o=
rg/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 =3D request.getMimeH=
eaders().getValue("user-agent");
> > > >                  if (userAgentValueMB !=3D null) {
> > > >                      String userAgentValue =3D userAgentValueMB.toS=
tring();
> > > > -                    if (noCompressionUserAgents.matcher(userAgentV=
alue).matches()) {
> > > > +                    if (noCompressionUserAgents.matcher(userAgentV=
alue).find()) {
> > > >                          return false;
> > > >                      }
> > > >                  }
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>