Re: Applications setting connection specific HTTP headers

[Romain Manni-Bucau]

+1, out of curiosity, cant rewrite valve help somehow?

Le jeu. 4 févr. 2021 à 19:11, Mark Thomas  a écrit :

> On 04/02/2021 17:56, Romain Manni-Bucau wrote:
> > I always fixed it by a filter until now.
> > What about adding a filter in tomcat and if users complain move to
> another
> > solution? Can even be marked @Alpha for a few I guess.
>
> We have been talking about a ModHeadersFilter for quite some time.
>
> This might be the time to implement it.
>
> Mark
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>

Re: Applications setting connection specific HTTP headers

[Mark Thomas]

On 04/02/2021 17:56, Romain Manni-Bucau wrote:
> I always fixed it by a filter until now.
> What about adding a filter in tomcat and if users complain move to another
> solution? Can even be marked @Alpha for a few I guess.

We have been talking about a ModHeadersFilter for quite some time.

This might be the time to implement it.

Mark


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

Re: Applications setting connection specific HTTP headers

[Romain Manni-Bucau]

I always fixed it by a filter until now.
What about adding a filter in tomcat and if users complain move to another
solution? Can even be marked @Alpha for a few I guess.

Romain Manni-Bucau
@rmannibucau  |  Blog
 | Old Blog
 | Github  |
LinkedIn  | Book



Le jeu. 4 févr. 2021 à 17:55, Mark Thomas  a écrit :

> On 04/02/2021 16:37, Romain Manni-Bucau wrote:
> > (if it helps) From what I saw which was close to that it was due to the
> > current microservice erea where it is very common to have poor man
> proxies
> > coded in java/tomcat and forwarding all headers of a HTTP request, this
> is
> > not uncommon and will even work on some containers (and always in bad
> tests
> > ;)) but nothing a filter can't fix trivially.
>
> That is the scenario I am looking at. If your experience is that a
> filter is (nearly) always safe in those scenarios then that is the way
> to go.
>
> Thanks,
>
> Mark
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>

Re: Applications setting connection specific HTTP headers

[Mark Thomas]

On 04/02/2021 16:37, Romain Manni-Bucau wrote:
> (if it helps) From what I saw which was close to that it was due to the
> current microservice erea where it is very common to have poor man proxies
> coded in java/tomcat and forwarding all headers of a HTTP request, this is
> not uncommon and will even work on some containers (and always in bad tests
> ;)) but nothing a filter can't fix trivially.

That is the scenario I am looking at. If your experience is that a
filter is (nearly) always safe in those scenarios then that is the way
to go.

Thanks,

Mark

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

Re: Applications setting connection specific HTTP headers

[Michael Osipov]

Am 2021-02-04 um 17:36 schrieb Rémy Maucherat:

On Thu, Feb 4, 2021 at 5:31 PM Michael Osipov  wrote:


Am 2021-02-03 um 13:03 schrieb Mark Thomas:

Hi all,

We have an open PR related to this for HTTP/2 (#277) and I am seeing
related issues at $work with HTTP.

In short, applications are doing things like:

response.setHeader("Transfer-Encoding", "chunked");

which, as you'd expect is causing problems if:
- Tomcat doesn't chunk the response
- Tomcat does chunk the response, adds its own "chunked" value and the
user agent rightly objects to "chunked" appearing twice

And so on.

I'd like to put something into Tomcat to address this.

I think it should be disabled by default so correctly written
applications pay a very small penalty. Along the lines of

if (someSetting != null) {
  // Do header checks
}

In terms of options I think we need:
- something representing the current, allow anything, behaviour
- an option to log (with a stack trace so the offending code can be
identified) attempts to set such headers
- an option to ignore attempts to set such headers

Do we need an option that throws an exception if there is an attempt to
set such headers?

Do we need an option to control which headers and which values will
trigger this behaviour? This would make the configuration rather more
complex. You'd need to be able to set multiple combinations of header,
value and action.

Is adding debug (no stacktrace) and trace (with stacktrace) logging to
addHeader() sufficient? For identifying faulty code this helps but it
doesn't provide a way to work-around the problem. For that you need
something that blocks the adding of the header.

I'm still considering what might be the best way to fix this. Hence the
brain dump above. Thoughts?


Re-reading the PR and your description, I do not really understand why
the container should handle and automagically fix bad application code?
Doing non-sense in appication code shall lead to undefined behavior.
Autofixing means that devs will never fix the real cause and Tomcat will
handle the symptom.

Can you explain why the problems at work cannot be fixed in the webapp
itself?



Probably it's: Customer X using library Y, says there's no possible way he
could ever update library Y with a fixed version. Thankfully, getting
Tomcat devs to work around the library issue instead is easy to do.
Solution !


This doesn't count because with the same counter-argument we have 
rejected to reintroduce reason phrases in 9/10. Explanation: fix the 
client, not us.



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

Re: Applications setting connection specific HTTP headers

[Romain Manni-Bucau]

(if it helps) From what I saw which was close to that it was due to the
current microservice erea where it is very common to have poor man proxies
coded in java/tomcat and forwarding all headers of a HTTP request, this is
not uncommon and will even work on some containers (and always in bad tests
;)) but nothing a filter can't fix trivially.
So helping is clearly interesting, I'm not 100% sure it belongs to tomcat
but a configurable filter wouldnt hurt or impact other users while not
active by default I think and it will help for these cases.

Romain Manni-Bucau
@rmannibucau  |  Blog
 | Old Blog
 | Github  |
LinkedIn  | Book



Le jeu. 4 févr. 2021 à 17:31, Michael Osipov  a écrit :

> Am 2021-02-03 um 13:03 schrieb Mark Thomas:
> > Hi all,
> >
> > We have an open PR related to this for HTTP/2 (#277) and I am seeing
> > related issues at $work with HTTP.
> >
> > In short, applications are doing things like:
> >
> > response.setHeader("Transfer-Encoding", "chunked");
> >
> > which, as you'd expect is causing problems if:
> > - Tomcat doesn't chunk the response
> > - Tomcat does chunk the response, adds its own "chunked" value and the
> >user agent rightly objects to "chunked" appearing twice
> >
> > And so on.
> >
> > I'd like to put something into Tomcat to address this.
> >
> > I think it should be disabled by default so correctly written
> > applications pay a very small penalty. Along the lines of
> >
> > if (someSetting != null) {
> >  // Do header checks
> > }
> >
> > In terms of options I think we need:
> > - something representing the current, allow anything, behaviour
> > - an option to log (with a stack trace so the offending code can be
> >identified) attempts to set such headers
> > - an option to ignore attempts to set such headers
> >
> > Do we need an option that throws an exception if there is an attempt to
> > set such headers?
> >
> > Do we need an option to control which headers and which values will
> > trigger this behaviour? This would make the configuration rather more
> > complex. You'd need to be able to set multiple combinations of header,
> > value and action.
> >
> > Is adding debug (no stacktrace) and trace (with stacktrace) logging to
> > addHeader() sufficient? For identifying faulty code this helps but it
> > doesn't provide a way to work-around the problem. For that you need
> > something that blocks the adding of the header.
> >
> > I'm still considering what might be the best way to fix this. Hence the
> > brain dump above. Thoughts?
>
> Re-reading the PR and your description, I do not really understand why
> the container should handle and automagically fix bad application code?
> Doing non-sense in appication code shall lead to undefined behavior.
> Autofixing means that devs will never fix the real cause and Tomcat will
> handle the symptom.
>
> Can you explain why the problems at work cannot be fixed in the webapp
> itself?
>
> M
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>

Re: Applications setting connection specific HTTP headers

[Rémy Maucherat]

On Thu, Feb 4, 2021 at 5:31 PM Michael Osipov  wrote:

> Am 2021-02-03 um 13:03 schrieb Mark Thomas:
> > Hi all,
> >
> > We have an open PR related to this for HTTP/2 (#277) and I am seeing
> > related issues at $work with HTTP.
> >
> > In short, applications are doing things like:
> >
> > response.setHeader("Transfer-Encoding", "chunked");
> >
> > which, as you'd expect is causing problems if:
> > - Tomcat doesn't chunk the response
> > - Tomcat does chunk the response, adds its own "chunked" value and the
> >user agent rightly objects to "chunked" appearing twice
> >
> > And so on.
> >
> > I'd like to put something into Tomcat to address this.
> >
> > I think it should be disabled by default so correctly written
> > applications pay a very small penalty. Along the lines of
> >
> > if (someSetting != null) {
> >  // Do header checks
> > }
> >
> > In terms of options I think we need:
> > - something representing the current, allow anything, behaviour
> > - an option to log (with a stack trace so the offending code can be
> >identified) attempts to set such headers
> > - an option to ignore attempts to set such headers
> >
> > Do we need an option that throws an exception if there is an attempt to
> > set such headers?
> >
> > Do we need an option to control which headers and which values will
> > trigger this behaviour? This would make the configuration rather more
> > complex. You'd need to be able to set multiple combinations of header,
> > value and action.
> >
> > Is adding debug (no stacktrace) and trace (with stacktrace) logging to
> > addHeader() sufficient? For identifying faulty code this helps but it
> > doesn't provide a way to work-around the problem. For that you need
> > something that blocks the adding of the header.
> >
> > I'm still considering what might be the best way to fix this. Hence the
> > brain dump above. Thoughts?
>
> Re-reading the PR and your description, I do not really understand why
> the container should handle and automagically fix bad application code?
> Doing non-sense in appication code shall lead to undefined behavior.
> Autofixing means that devs will never fix the real cause and Tomcat will
> handle the symptom.
>
> Can you explain why the problems at work cannot be fixed in the webapp
> itself?
>

Probably it's: Customer X using library Y, says there's no possible way he
could ever update library Y with a fixed version. Thankfully, getting
Tomcat devs to work around the library issue instead is easy to do.
Solution !

Rémy


>
> M
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>

Re: Applications setting connection specific HTTP headers

[Michael Osipov]

Am 2021-02-03 um 13:03 schrieb Mark Thomas:

Hi all,

We have an open PR related to this for HTTP/2 (#277) and I am seeing
related issues at $work with HTTP.

In short, applications are doing things like:

response.setHeader("Transfer-Encoding", "chunked");

which, as you'd expect is causing problems if:
- Tomcat doesn't chunk the response
- Tomcat does chunk the response, adds its own "chunked" value and the
   user agent rightly objects to "chunked" appearing twice

And so on.

I'd like to put something into Tomcat to address this.

I think it should be disabled by default so correctly written
applications pay a very small penalty. Along the lines of

if (someSetting != null) {
 // Do header checks
}

In terms of options I think we need:
- something representing the current, allow anything, behaviour
- an option to log (with a stack trace so the offending code can be
   identified) attempts to set such headers
- an option to ignore attempts to set such headers

Do we need an option that throws an exception if there is an attempt to
set such headers?

Do we need an option to control which headers and which values will
trigger this behaviour? This would make the configuration rather more
complex. You'd need to be able to set multiple combinations of header,
value and action.

Is adding debug (no stacktrace) and trace (with stacktrace) logging to
addHeader() sufficient? For identifying faulty code this helps but it
doesn't provide a way to work-around the problem. For that you need
something that blocks the adding of the header.

I'm still considering what might be the best way to fix this. Hence the
brain dump above. Thoughts?


Re-reading the PR and your description, I do not really understand why 
the container should handle and automagically fix bad application code? 
Doing non-sense in appication code shall lead to undefined behavior. 
Autofixing means that devs will never fix the real cause and Tomcat will 
handle the symptom.


Can you explain why the problems at work cannot be fixed in the webapp 
itself?


M

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

Re: Applications setting connection specific HTTP headers

[Mark Thomas]

On 04/02/2021 15:33, Christopher Schultz wrote:
> Mark,
> 
> On 2/4/21 8:52 AM, Mark Thomas wrote:
>> On 04/02/2021 13:28, Christopher Schultz wrote:
>>> I think this should be done in a Valve that is enabled by default, but
>>> can be disabled, rendering zero penalty for "well-behaved" applications.
>>> The Valve can simply wrap the response with a wrapper that either prints
>>> an error to the log (maybe first migration path, enabled now) or throws
>>> an exception (second migration path, in the future).
>>
>> Valves can't wrap requests or responses. It would require some major
>> refactoring to make that possible.
> 
> Well, you *can* wrap them. Maybe it's not a good idea[1], but its
> certainly possible.

That is a lot simpler than I recall.

> [1] https://bz.apache.org/bugzilla/show_bug.cgi?id=45014
> 
> I still don't understand why wrapping the request is such a risky thing.
> Only in Tomcat code, suddenly OOP decoration pattern doesn't apply?

My concern is if an application is doing things like
addHeader("Transfer-Encoding","chunked") it is messing around at a
fairly low level and I would not be surprised to see the same
application trying to cast HttpServletResponse to ResponseFacade and
assuming it is not wrapped.

For a spec compliant app, I agree a Filter is the obvious choice.

Maybe I am over estimating the risk of apps making inappropriate casts
and/or assumptions of no wrapping. For the specific cases I have in mind
I don't have hard information so I am leaning towards assuming the worst.

Mark

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

Re: Applications setting connection specific HTTP headers

[Christopher Schultz]

Mark,

On 2/4/21 8:52 AM, Mark Thomas wrote:
> On 04/02/2021 13:28, Christopher Schultz wrote:
>> I think this should be done in a Valve that is enabled by default, but
>> can be disabled, rendering zero penalty for "well-behaved" applications.
>> The Valve can simply wrap the response with a wrapper that either prints
>> an error to the log (maybe first migration path, enabled now) or throws
>> an exception (second migration path, in the future).
> 
> Valves can't wrap requests or responses. It would require some major
> refactoring to make that possible.

Well, you *can* wrap them. Maybe it's not a good idea[1], but its
certainly possible.

-chris

[1] https://bz.apache.org/bugzilla/show_bug.cgi?id=45014

I still don't understand why wrapping the request is such a risky thing.
Only in Tomcat code, suddenly OOP decoration pattern doesn't apply?

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

Re: Applications setting connection specific HTTP headers

[Mark Thomas]

On 04/02/2021 13:28, Christopher Schultz wrote:
> I think this should be done in a Valve that is enabled by default, but
> can be disabled, rendering zero penalty for "well-behaved" applications.
> The Valve can simply wrap the response with a wrapper that either prints
> an error to the log (maybe first migration path, enabled now) or throws
> an exception (second migration path, in the future).

Valves can't wrap requests or responses. It would require some major
refactoring to make that possible.

Mark

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

Re: Applications setting connection specific HTTP headers

[Christopher Schultz]

Mark,

On 2/3/21 07:03, Mark Thomas wrote:

Hi all,

We have an open PR related to this for HTTP/2 (#277) and I am seeing
related issues at $work with HTTP.

In short, applications are doing things like:

response.setHeader("Transfer-Encoding", "chunked");

which, as you'd expect is causing problems if:
- Tomcat doesn't chunk the response
- Tomcat does chunk the response, adds its own "chunked" value and the
   user agent rightly objects to "chunked" appearing twice

And so on.

I'd like to put something into Tomcat to address this.

I think it should be disabled by default so correctly written
applications pay a very small penalty. Along the lines of

if (someSetting != null) {
 // Do header checks
}

In terms of options I think we need:
- something representing the current, allow anything, behaviour
- an option to log (with a stack trace so the offending code can be
   identified) attempts to set such headers
- an option to ignore attempts to set such headers

Do we need an option that throws an exception if there is an attempt to
set such headers?

Do we need an option to control which headers and which values will
trigger this behaviour? This would make the configuration rather more
complex. You'd need to be able to set multiple combinations of header,
value and action.

Is adding debug (no stacktrace) and trace (with stacktrace) logging to
addHeader() sufficient? For identifying faulty code this helps but it
doesn't provide a way to work-around the problem. For that you need
something that blocks the adding of the header.

I'm still considering what might be the best way to fix this. Hence the
brain dump above. Thoughts?


Hmm. How hard would it be to allow the application to '/instruct/ Tomcat 
to chunk the response by setting this header?


On second thought, there is a better way to do that: flush the output 
stream explicitly, right?


I think this should be done in a Valve that is enabled by default, but 
can be disabled, rendering zero penalty for "well-behaved" applications. 
The Valve can simply wrap the response with a wrapper that either prints 
an error to the log (maybe first migration path, enabled now) or throws 
an exception (second migration path, in the future).


Making it possible to check for various combinations of header names and 
values doesn't seem worth the effort at the moment, but perhaps naming 
the valve something generic so it could be expanded in the future would 
be a good idea.


-chris

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

Re: Applications setting connection specific HTTP headers

[Romain Manni-Bucau]

Le jeu. 4 févr. 2021 à 11:09, Mark Thomas  a écrit :

> Responses in line:
>
> On 03/02/2021 19:55, Raymond Auge wrote:
> > What about an integration point that acts as a passthrough to such
> changes
> > that let you "monitor" and/or "defend" against these operations (using
> > whatever policy you wish).
> > The default would be no-op.
>
> That certainly provides most flexibility and could easily be implemented
> in a way that avoids the issues with the Filter approach.
>
> I suspect a lot of users that want/need the sort of functionality we are
> discussing here won't want to / be able to deploy custom code and would
> prefer a configuration only solution. We could address that by providing
> some custom implementations.
>
> > On Wed, Feb 3, 2021 at 11:15 AM Romain Manni-Bucau <
> rmannibu...@gmail.com>
> > wrote:
>
> 
>
> >> So you mean the application uses tomcat internals (like casting the
> >> Response/Request) but does not work on Tomcat? :s
>
> Yes, casting is what concerns me. And making assumptions about whether
> the response is wrapped and with what concrete classes in what order.
> Adding another wrapper might break things.
>

Not if we enforce it to be first which is only configuration as of today
and what you are looking for, no?
If not I'm not sure how the hypothesis that the app is broken on tomcat can
be (if the app uses tomcat internals it works on tomcat or needs fixes
tomcat does not need to do for the app IMHO).
Concretely, I only see this feature as a portability helper (from another
container/env), in other cases (ie the app was developped for tomcat but
does not work on tomcat) I'm not sure tomcat should help.
When I do "1+2" and expect 2 the language/compiler/other never help for
good reasons.

What I like with the filter option is that the user will understand what
tomcat does even not aware of tomcat internals.
If we add a HttpOutputBuffer wrapper which handles header keys/values and
an error is logged/thrown from there I think it will be way less clear for
most people even if it is very close.
Overall a new Header dedicate spi seems overkill since we have already
multiple extension point to control headers, no?



>
> 
>
> I spent a little time this morning looking through the Tomcat code base
> for calls that manipulated the response headers to see what might be
> problematic.
>
> If we look at which headers and/or values are known to cause problems or
> might cause problems the list is a lot shorter than I expected.
>
> Tomcat already silently prevents an application from setting multiple
> content-length (and content-type) headers.
>
> As far as I can tell we'd need at least the following additional checks:
>
> - Any app set vary header needs to by syntactically correct (Tomcat
>   depends on this if Tomcat needs to manipulate the header).
>
> - The app must not set "Transfer-Encoding: chunked" as Tomcat always
>   adds this if required and multiple values are known to be rejected by
>   some reverse proxies.
>
> - The app must not set "Connection: keep-alive" if
>   useKeepAliveResponseHeader="true" is set on the Connector.
>
> - It is debatable whether or not we allow users to set
>   "Connection: close". Given there may still be valid use cases for this
>I think we should allow it.
>
> I don't think we need to restrict Connection any further. We need to
> allow "Connection: upgrade" so we can't block it completely.
>
> I am currently wondering whether a simple solution that blocks known bad
> header/value combinations and logs what has been blocked is sufficient
> or whether we need / should go down the integration point approach and
> introduce "HeaderValidator" or similar.
>
> Mark
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>

Re: Applications setting connection specific HTTP headers

[Mark Thomas]

Responses in line:

On 03/02/2021 19:55, Raymond Auge wrote:
> What about an integration point that acts as a passthrough to such changes
> that let you "monitor" and/or "defend" against these operations (using
> whatever policy you wish).
> The default would be no-op.

That certainly provides most flexibility and could easily be implemented
in a way that avoids the issues with the Filter approach.

I suspect a lot of users that want/need the sort of functionality we are
discussing here won't want to / be able to deploy custom code and would
prefer a configuration only solution. We could address that by providing
some custom implementations.

> On Wed, Feb 3, 2021 at 11:15 AM Romain Manni-Bucau 
> wrote:



>> So you mean the application uses tomcat internals (like casting the
>> Response/Request) but does not work on Tomcat? :s

Yes, casting is what concerns me. And making assumptions about whether
the response is wrapped and with what concrete classes in what order.
Adding another wrapper might break things.



I spent a little time this morning looking through the Tomcat code base
for calls that manipulated the response headers to see what might be
problematic.

If we look at which headers and/or values are known to cause problems or
might cause problems the list is a lot shorter than I expected.

Tomcat already silently prevents an application from setting multiple
content-length (and content-type) headers.

As far as I can tell we'd need at least the following additional checks:

- Any app set vary header needs to by syntactically correct (Tomcat
  depends on this if Tomcat needs to manipulate the header).

- The app must not set "Transfer-Encoding: chunked" as Tomcat always
  adds this if required and multiple values are known to be rejected by
  some reverse proxies.

- The app must not set "Connection: keep-alive" if
  useKeepAliveResponseHeader="true" is set on the Connector.

- It is debatable whether or not we allow users to set
  "Connection: close". Given there may still be valid use cases for this
   I think we should allow it.

I don't think we need to restrict Connection any further. We need to
allow "Connection: upgrade" so we can't block it completely.

I am currently wondering whether a simple solution that blocks known bad
header/value combinations and logs what has been blocked is sufficient
or whether we need / should go down the integration point approach and
introduce "HeaderValidator" or similar.

Mark

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

Re: Applications setting connection specific HTTP headers

[Raymond Auge]

What about an integration point that acts as a passthrough to such changes
that let you "monitor" and/or "defend" against these operations (using
whatever policy you wish).
The default would be no-op.

- Ray

On Wed, Feb 3, 2021 at 11:15 AM Romain Manni-Bucau 
wrote:

> Le mer. 3 févr. 2021 à 17:10, Mark Thomas  a écrit :
>
> > On 03/02/2021 16:06, Romain Manni-Bucau wrote:
> > > Hi,
> > >
> > > Why not just adding a tomcat-application-fixer module (a more ugly name
> > can
> > > be relevant ;)) application could add in their WEB-INF/lib through web
> > > resource definition (ie plain context.xml config or programmatic
> > > equivalent) which would have a @WebFilter(/*, asyncSupported=true)
> which
> > > would wrap the response to do the fixes you want for these apps but it
> > > wouldnt be seen in the most common case at all (or if there is only
> this
> > > small fix it can be a default filter of tomcat, depends the number of
> > fixes
> > > and related code IMHO).
> >
> > Primarily because of the risk that wrapping the request breaks the
> > application.
> >
> > While a well-behaved app should be unaffected by a Filter adding a
> > wrapped response, we already know that these applications are not
> > well-behaved - else they wouldn't be setting these headers in the first
> > place.
> >
> > Hence I am looking at solutions further down the stack in the Tomcat
> > internals.
> >
>
> So you mean the application uses tomcat internals (like casting the
> Response/Request) but does not work on Tomcat? :s
>
> Otherwise there is no real way a filter and wrapper breaks a "broken"
> application since the application will take the wrapped instance as a
> standard servlet instance - as tomcat already do with its facade layer. The
> only constraint is to ensure the filter is first which can require another
> solution than @WebFilter but web.xml solves it.
>
>
>
> >
> > Mark
> >
> >
> > >
> > > Romain Manni-Bucau
> > > @rmannibucau  |  Blog
> > >  | Old Blog
> > >  | Github <
> > https://github.com/rmannibucau> |
> > > LinkedIn  | Book
> > > <
> >
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> > >
> > >
> > >
> > > Le mer. 3 févr. 2021 à 16:50, Rémy Maucherat  a
> écrit :
> > >
> > >> On Wed, Feb 3, 2021 at 1:03 PM Mark Thomas  wrote:
> > >>
> > >>> Hi all,
> > >>>
> > >>> We have an open PR related to this for HTTP/2 (#277) and I am seeing
> > >>> related issues at $work with HTTP.
> > >>>
> > >>> In short, applications are doing things like:
> > >>>
> > >>> response.setHeader("Transfer-Encoding", "chunked");
> > >>>
> > >>> which, as you'd expect is causing problems if:
> > >>> - Tomcat doesn't chunk the response
> > >>> - Tomcat does chunk the response, adds its own "chunked" value and
> the
> > >>>   user agent rightly objects to "chunked" appearing twice
> > >>>
> > >>> And so on.
> > >>>
> > >>> I'd like to put something into Tomcat to address this.
> > >>>
> > >>> I think it should be disabled by default so correctly written
> > >>> applications pay a very small penalty. Along the lines of
> > >>>
> > >>> if (someSetting != null) {
> > >>> // Do header checks
> > >>> }
> > >>>
> > >>> In terms of options I think we need:
> > >>> - something representing the current, allow anything, behaviour
> > >>> - an option to log (with a stack trace so the offending code can be
> > >>>   identified) attempts to set such headers
> > >>> - an option to ignore attempts to set such headers
> > >>>
> > >>> Do we need an option that throws an exception if there is an attempt
> to
> > >>> set such headers?
> > >>>
> > >>> Do we need an option to control which headers and which values will
> > >>> trigger this behaviour? This would make the configuration rather more
> > >>> complex. You'd need to be able to set multiple combinations of
> header,
> > >>> value and action.
> > >>>
> > >>> Is adding debug (no stacktrace) and trace (with stacktrace) logging
> to
> > >>> addHeader() sufficient? For identifying faulty code this helps but it
> > >>> doesn't provide a way to work-around the problem. For that you need
> > >>> something that blocks the adding of the header.
> > >>>
> > >>> I'm still considering what might be the best way to fix this. Hence
> the
> > >>> brain dump above. Thoughts?
> > >>>
> > >>
> > >> There has been some debate about this before, and you did add quite a
> > bit
> > >> of code to catch things that would break the protocol. So it seems
> this
> > >> would go above and beyond, and attempt to catch *anything* that could
> > make
> > >> a response non compliant with the underlying protocol ?
> > >>
> > >> Rémy
> > >>
> > >>
> > >>>
> > >>> Mark
> > >>>
> > >>>
> > >>> -
> > >>> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > >>> For additional commands, 

Re: Applications setting connection specific HTTP headers

[Romain Manni-Bucau]

Le mer. 3 févr. 2021 à 17:10, Mark Thomas  a écrit :

> On 03/02/2021 16:06, Romain Manni-Bucau wrote:
> > Hi,
> >
> > Why not just adding a tomcat-application-fixer module (a more ugly name
> can
> > be relevant ;)) application could add in their WEB-INF/lib through web
> > resource definition (ie plain context.xml config or programmatic
> > equivalent) which would have a @WebFilter(/*, asyncSupported=true) which
> > would wrap the response to do the fixes you want for these apps but it
> > wouldnt be seen in the most common case at all (or if there is only this
> > small fix it can be a default filter of tomcat, depends the number of
> fixes
> > and related code IMHO).
>
> Primarily because of the risk that wrapping the request breaks the
> application.
>
> While a well-behaved app should be unaffected by a Filter adding a
> wrapped response, we already know that these applications are not
> well-behaved - else they wouldn't be setting these headers in the first
> place.
>
> Hence I am looking at solutions further down the stack in the Tomcat
> internals.
>

So you mean the application uses tomcat internals (like casting the
Response/Request) but does not work on Tomcat? :s

Otherwise there is no real way a filter and wrapper breaks a "broken"
application since the application will take the wrapped instance as a
standard servlet instance - as tomcat already do with its facade layer. The
only constraint is to ensure the filter is first which can require another
solution than @WebFilter but web.xml solves it.



>
> Mark
>
>
> >
> > Romain Manni-Bucau
> > @rmannibucau  |  Blog
> >  | Old Blog
> >  | Github <
> https://github.com/rmannibucau> |
> > LinkedIn  | Book
> > <
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >
> >
> >
> > Le mer. 3 févr. 2021 à 16:50, Rémy Maucherat  a écrit :
> >
> >> On Wed, Feb 3, 2021 at 1:03 PM Mark Thomas  wrote:
> >>
> >>> Hi all,
> >>>
> >>> We have an open PR related to this for HTTP/2 (#277) and I am seeing
> >>> related issues at $work with HTTP.
> >>>
> >>> In short, applications are doing things like:
> >>>
> >>> response.setHeader("Transfer-Encoding", "chunked");
> >>>
> >>> which, as you'd expect is causing problems if:
> >>> - Tomcat doesn't chunk the response
> >>> - Tomcat does chunk the response, adds its own "chunked" value and the
> >>>   user agent rightly objects to "chunked" appearing twice
> >>>
> >>> And so on.
> >>>
> >>> I'd like to put something into Tomcat to address this.
> >>>
> >>> I think it should be disabled by default so correctly written
> >>> applications pay a very small penalty. Along the lines of
> >>>
> >>> if (someSetting != null) {
> >>> // Do header checks
> >>> }
> >>>
> >>> In terms of options I think we need:
> >>> - something representing the current, allow anything, behaviour
> >>> - an option to log (with a stack trace so the offending code can be
> >>>   identified) attempts to set such headers
> >>> - an option to ignore attempts to set such headers
> >>>
> >>> Do we need an option that throws an exception if there is an attempt to
> >>> set such headers?
> >>>
> >>> Do we need an option to control which headers and which values will
> >>> trigger this behaviour? This would make the configuration rather more
> >>> complex. You'd need to be able to set multiple combinations of header,
> >>> value and action.
> >>>
> >>> Is adding debug (no stacktrace) and trace (with stacktrace) logging to
> >>> addHeader() sufficient? For identifying faulty code this helps but it
> >>> doesn't provide a way to work-around the problem. For that you need
> >>> something that blocks the adding of the header.
> >>>
> >>> I'm still considering what might be the best way to fix this. Hence the
> >>> brain dump above. Thoughts?
> >>>
> >>
> >> There has been some debate about this before, and you did add quite a
> bit
> >> of code to catch things that would break the protocol. So it seems this
> >> would go above and beyond, and attempt to catch *anything* that could
> make
> >> a response non compliant with the underlying protocol ?
> >>
> >> Rémy
> >>
> >>
> >>>
> >>> Mark
> >>>
> >>>
> >>> -
> >>> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> >>> For additional commands, e-mail: dev-h...@tomcat.apache.org
> >>>
> >>>
> >>
> >
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>

Re: Applications setting connection specific HTTP headers

[Mark Thomas]

On 03/02/2021 16:06, Romain Manni-Bucau wrote:
> Hi,
> 
> Why not just adding a tomcat-application-fixer module (a more ugly name can
> be relevant ;)) application could add in their WEB-INF/lib through web
> resource definition (ie plain context.xml config or programmatic
> equivalent) which would have a @WebFilter(/*, asyncSupported=true) which
> would wrap the response to do the fixes you want for these apps but it
> wouldnt be seen in the most common case at all (or if there is only this
> small fix it can be a default filter of tomcat, depends the number of fixes
> and related code IMHO).

Primarily because of the risk that wrapping the request breaks the
application.

While a well-behaved app should be unaffected by a Filter adding a
wrapped response, we already know that these applications are not
well-behaved - else they wouldn't be setting these headers in the first
place.

Hence I am looking at solutions further down the stack in the Tomcat
internals.

Mark


> 
> Romain Manni-Bucau
> @rmannibucau  |  Blog
>  | Old Blog
>  | Github  |
> LinkedIn  | Book
> 
> 
> 
> Le mer. 3 févr. 2021 à 16:50, Rémy Maucherat  a écrit :
> 
>> On Wed, Feb 3, 2021 at 1:03 PM Mark Thomas  wrote:
>>
>>> Hi all,
>>>
>>> We have an open PR related to this for HTTP/2 (#277) and I am seeing
>>> related issues at $work with HTTP.
>>>
>>> In short, applications are doing things like:
>>>
>>> response.setHeader("Transfer-Encoding", "chunked");
>>>
>>> which, as you'd expect is causing problems if:
>>> - Tomcat doesn't chunk the response
>>> - Tomcat does chunk the response, adds its own "chunked" value and the
>>>   user agent rightly objects to "chunked" appearing twice
>>>
>>> And so on.
>>>
>>> I'd like to put something into Tomcat to address this.
>>>
>>> I think it should be disabled by default so correctly written
>>> applications pay a very small penalty. Along the lines of
>>>
>>> if (someSetting != null) {
>>> // Do header checks
>>> }
>>>
>>> In terms of options I think we need:
>>> - something representing the current, allow anything, behaviour
>>> - an option to log (with a stack trace so the offending code can be
>>>   identified) attempts to set such headers
>>> - an option to ignore attempts to set such headers
>>>
>>> Do we need an option that throws an exception if there is an attempt to
>>> set such headers?
>>>
>>> Do we need an option to control which headers and which values will
>>> trigger this behaviour? This would make the configuration rather more
>>> complex. You'd need to be able to set multiple combinations of header,
>>> value and action.
>>>
>>> Is adding debug (no stacktrace) and trace (with stacktrace) logging to
>>> addHeader() sufficient? For identifying faulty code this helps but it
>>> doesn't provide a way to work-around the problem. For that you need
>>> something that blocks the adding of the header.
>>>
>>> I'm still considering what might be the best way to fix this. Hence the
>>> brain dump above. Thoughts?
>>>
>>
>> There has been some debate about this before, and you did add quite a bit
>> of code to catch things that would break the protocol. So it seems this
>> would go above and beyond, and attempt to catch *anything* that could make
>> a response non compliant with the underlying protocol ?
>>
>> Rémy
>>
>>
>>>
>>> Mark
>>>
>>>
>>> -
>>> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
>>> For additional commands, e-mail: dev-h...@tomcat.apache.org
>>>
>>>
>>
> 


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

Re: Applications setting connection specific HTTP headers

[Mark Thomas]

On 03/02/2021 15:50, Rémy Maucherat wrote:
> On Wed, Feb 3, 2021 at 1:03 PM Mark Thomas  wrote:



>> I'm still considering what might be the best way to fix this. Hence the
>> brain dump above. Thoughts?
> 
> There has been some debate about this before, and you did add quite a bit
> of code to catch things that would break the protocol. So it seems this
> would go above and beyond, and attempt to catch *anything* that could make
> a response non compliant with the underlying protocol ?

The stuff I added before was mostly on the input side to protect against
non-compliant user agents. The changes I am thinking about here are more
geared towards preventing apps from setting response headers they shouldn't.

How far to go is the question:
a) a few we know are definitely wrong (like TE: chunked) ?
b) any we think Tomcat should / needs to control ?
c) make it fully customisable ?
d) something else?

Maybe I need to make a list of the headers I think fall under a) and b)
and see if that helps clarify things.

Mark

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

Re: Applications setting connection specific HTTP headers

[Romain Manni-Bucau]

Hi,

Why not just adding a tomcat-application-fixer module (a more ugly name can
be relevant ;)) application could add in their WEB-INF/lib through web
resource definition (ie plain context.xml config or programmatic
equivalent) which would have a @WebFilter(/*, asyncSupported=true) which
would wrap the response to do the fixes you want for these apps but it
wouldnt be seen in the most common case at all (or if there is only this
small fix it can be a default filter of tomcat, depends the number of fixes
and related code IMHO).

Romain Manni-Bucau
@rmannibucau  |  Blog
 | Old Blog
 | Github  |
LinkedIn  | Book



Le mer. 3 févr. 2021 à 16:50, Rémy Maucherat  a écrit :

> On Wed, Feb 3, 2021 at 1:03 PM Mark Thomas  wrote:
>
> > Hi all,
> >
> > We have an open PR related to this for HTTP/2 (#277) and I am seeing
> > related issues at $work with HTTP.
> >
> > In short, applications are doing things like:
> >
> > response.setHeader("Transfer-Encoding", "chunked");
> >
> > which, as you'd expect is causing problems if:
> > - Tomcat doesn't chunk the response
> > - Tomcat does chunk the response, adds its own "chunked" value and the
> >   user agent rightly objects to "chunked" appearing twice
> >
> > And so on.
> >
> > I'd like to put something into Tomcat to address this.
> >
> > I think it should be disabled by default so correctly written
> > applications pay a very small penalty. Along the lines of
> >
> > if (someSetting != null) {
> > // Do header checks
> > }
> >
> > In terms of options I think we need:
> > - something representing the current, allow anything, behaviour
> > - an option to log (with a stack trace so the offending code can be
> >   identified) attempts to set such headers
> > - an option to ignore attempts to set such headers
> >
> > Do we need an option that throws an exception if there is an attempt to
> > set such headers?
> >
> > Do we need an option to control which headers and which values will
> > trigger this behaviour? This would make the configuration rather more
> > complex. You'd need to be able to set multiple combinations of header,
> > value and action.
> >
> > Is adding debug (no stacktrace) and trace (with stacktrace) logging to
> > addHeader() sufficient? For identifying faulty code this helps but it
> > doesn't provide a way to work-around the problem. For that you need
> > something that blocks the adding of the header.
> >
> > I'm still considering what might be the best way to fix this. Hence the
> > brain dump above. Thoughts?
> >
>
> There has been some debate about this before, and you did add quite a bit
> of code to catch things that would break the protocol. So it seems this
> would go above and beyond, and attempt to catch *anything* that could make
> a response non compliant with the underlying protocol ?
>
> Rémy
>
>
> >
> > Mark
> >
> >
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > For additional commands, e-mail: dev-h...@tomcat.apache.org
> >
> >
>

Re: Applications setting connection specific HTTP headers

[Rémy Maucherat]

On Wed, Feb 3, 2021 at 1:03 PM Mark Thomas  wrote:

> Hi all,
>
> We have an open PR related to this for HTTP/2 (#277) and I am seeing
> related issues at $work with HTTP.
>
> In short, applications are doing things like:
>
> response.setHeader("Transfer-Encoding", "chunked");
>
> which, as you'd expect is causing problems if:
> - Tomcat doesn't chunk the response
> - Tomcat does chunk the response, adds its own "chunked" value and the
>   user agent rightly objects to "chunked" appearing twice
>
> And so on.
>
> I'd like to put something into Tomcat to address this.
>
> I think it should be disabled by default so correctly written
> applications pay a very small penalty. Along the lines of
>
> if (someSetting != null) {
> // Do header checks
> }
>
> In terms of options I think we need:
> - something representing the current, allow anything, behaviour
> - an option to log (with a stack trace so the offending code can be
>   identified) attempts to set such headers
> - an option to ignore attempts to set such headers
>
> Do we need an option that throws an exception if there is an attempt to
> set such headers?
>
> Do we need an option to control which headers and which values will
> trigger this behaviour? This would make the configuration rather more
> complex. You'd need to be able to set multiple combinations of header,
> value and action.
>
> Is adding debug (no stacktrace) and trace (with stacktrace) logging to
> addHeader() sufficient? For identifying faulty code this helps but it
> doesn't provide a way to work-around the problem. For that you need
> something that blocks the adding of the header.
>
> I'm still considering what might be the best way to fix this. Hence the
> brain dump above. Thoughts?
>

There has been some debate about this before, and you did add quite a bit
of code to catch things that would break the protocol. So it seems this
would go above and beyond, and attempt to catch *anything* that could make
a response non compliant with the underlying protocol ?

Rémy


>
> Mark
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>

Applications setting connection specific HTTP headers

[Mark Thomas]

Hi all,

We have an open PR related to this for HTTP/2 (#277) and I am seeing
related issues at $work with HTTP.

In short, applications are doing things like:

response.setHeader("Transfer-Encoding", "chunked");

which, as you'd expect is causing problems if:
- Tomcat doesn't chunk the response
- Tomcat does chunk the response, adds its own "chunked" value and the
  user agent rightly objects to "chunked" appearing twice

And so on.

I'd like to put something into Tomcat to address this.

I think it should be disabled by default so correctly written
applications pay a very small penalty. Along the lines of

if (someSetting != null) {
// Do header checks
}

In terms of options I think we need:
- something representing the current, allow anything, behaviour
- an option to log (with a stack trace so the offending code can be
  identified) attempts to set such headers
- an option to ignore attempts to set such headers

Do we need an option that throws an exception if there is an attempt to
set such headers?

Do we need an option to control which headers and which values will
trigger this behaviour? This would make the configuration rather more
complex. You'd need to be able to set multiple combinations of header,
value and action.

Is adding debug (no stacktrace) and trace (with stacktrace) logging to
addHeader() sufficient? For identifying faulty code this helps but it
doesn't provide a way to work-around the problem. For that you need
something that blocks the adding of the header.

I'm still considering what might be the best way to fix this. Hence the
brain dump above. Thoughts?

Mark


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