tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <ma...@apache.org>
Subject Re: [tomcat] branch 8.5.x updated: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829
Date Wed, 23 Oct 2019 15:59:56 GMT
On 23/10/2019 17:56, Christopher Schultz wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
> 
> Mark,
> 
> On 10/23/19 07:38, Mark Thomas wrote:
>> On 23/10/2019 13:28, Mark Thomas wrote:
>>> On 23/10/2019 11:09, markt@apache.org wrote:
>>>> This is an automated email from the ASF dual-hosted git
>>>> repository.
>>>>
>>>> markt pushed a commit to branch 8.5.x in repository
>>>> https://gitbox.apache.org/repos/asf/tomcat.git
>>>>
>>>>
>>>> The following commit(s) were added to refs/heads/8.5.x by this
>>>> push: new 9054e10  Fix
>>>> https://bz.apache.org/bugzilla/show_bug.cgi?id=63829 9054e10 is
>>>> described below
>>>>
>>>> commit 9054e10d53170afcd7dd85bd22335238625958dc Author: Mark
>>>> Thomas <markt@apache.org> AuthorDate: Wed Oct 23 08:50:11 2019
>>>> +0200
>>>>
>>>> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63829
>>>>
>>>> Improve the check of the Content-Encoding header when looking
>>>> to see if Tomcat is serving pre-compressed content. Ensure that
>>>> only a full token is matched and that the match is case
>>>> insensitive.
>>>
>>> This isn't complete for 8.5.x because only HTTP/2 uses
>>> CompressionConfig to make the compress / don't compress
>>> decision.
>>>
>>> I could apply a similar change to the relevant parts of
>>> Http11Processor but I was wondering about the possibility of a
>>> more extensive back-port that aligned the 8.5.x implementation
>>> with 9.0.x. This would involve API changes including: - retain a
>>> reference to the Protocol - change to constructor signature -
>>> remove unnecessary getters and setters
>>>
>>> While this is tempting from both a simplification PoV and from
>>> an aligning 8.5.x and 9.0.x PoV I do wonder what the risk of
>>> breakage is if users are extending Http11Processor. It is an
>>> internal API but I suspect it is still used by some.
>>>
>>> I think I am going to look at see if there is some sort of middle
>>> ground to be found. Meanwhile, what do people think about API
>>> changes along the lines of the above.
>>
>> Looking at the history, we have changed the API for the constructor
>> in the past so I think it would be safe to do that again. My plan
>> is to replace most of the parameters with the Protocol. I think the
>> getters and setters will need to stay. They can/should probably be
>> deprecated as they are removed in 9.0.x.
> 
> Maybe just add another constructor instead of replacing? I guess that
> would make the code uglier in other places where you'd prefer to use
> Protocol instead of Parts/Of/Protocol and have to check to see what
> you've got.

Exactly. On the grounds we have changed this constructor a few times
during the life of 8.5.x, I think this change is safe. It should also
reduce the need for future changes.

The next step (that wires in the CompressionConfig) might be a little
more controversial. I need to finish that work and see what it looks like.

Mark



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message