Camel Netty Http component only shows io.netty.handler.codec.TooLongFrameException at debug log level

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Camel Netty Http component only shows io.netty.handler.codec.TooLongFrameException at debug log level

Bob Paulin-2
Hi,

When the Camel Netty Http component receives a header that exceeds it's
maxHeaderSize the
org.apache.camel.component.netty.http.handlers.HttpServerChannelHandler
simply logs that a message was received at DEBUG level:

2019-11-19T19:34:02,418 | DEBUG | Camel (camel-2) thread #189 -
NettyEventExecutorGroup | NettyHttpConsumer                | 95 -
org.apache.camel.camel-core - 2.22.3 | Message received:
HttpObjectAggregator$AggregatedFullHttpRequest(decodeResult:
failure(io.netty.handler.codec.TooLongFrameException: HTTP header is
larger than 8192 bytes.), version: HTTP/1.1, content: EmptyByteBufBE)

This the request is allowed to proceed to code upstream which
unfortunately leads the developer to believe that there is nothing wrong
with the request.  In my case the request failed when I went to
unmarshal the body which was unexpectedly empty.  I'd like to propose
one of the follow options:

1) Throw an exception

2) Log an Error

Both of the above would be possible if with some changes to the
HttpServerChannelHandler.channelRead0

Currently the code only performs the following.

LOG.debug("Message received: {}", request);

However there could be a check on the request.decoderResult().cause(); 
If this is not null we could just throw the exception or a minimum log
an error level.  My preference would be to throw an exception as this
will make unrecoverable failures more obvious to developers.  Let me
know if this direction works for the team.  Happy to help with a
patch/tests.

Sincerely,

Bob



signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Camel Netty Http component only shows io.netty.handler.codec.TooLongFrameException at debug log level

Claus Ibsen-2
Hi Bob

Yeah we can make better logging, not sure if throwing an exception is
the right course as then your server logs can get spammed if http
clients send data with long headers.
So maybe there should be an option to set the logging level etc.

You are welcome to create a JIRA and work on a PR
https://camel.apache.org/manual/latest/contributing.html

On Tue, Nov 19, 2019 at 9:39 PM Bob Paulin <[hidden email]> wrote:

>
> Hi,
>
> When the Camel Netty Http component receives a header that exceeds it's
> maxHeaderSize the
> org.apache.camel.component.netty.http.handlers.HttpServerChannelHandler
> simply logs that a message was received at DEBUG level:
>
> 2019-11-19T19:34:02,418 | DEBUG | Camel (camel-2) thread #189 -
> NettyEventExecutorGroup | NettyHttpConsumer                | 95 -
> org.apache.camel.camel-core - 2.22.3 | Message received:
> HttpObjectAggregator$AggregatedFullHttpRequest(decodeResult:
> failure(io.netty.handler.codec.TooLongFrameException: HTTP header is
> larger than 8192 bytes.), version: HTTP/1.1, content: EmptyByteBufBE)
>
> This the request is allowed to proceed to code upstream which
> unfortunately leads the developer to believe that there is nothing wrong
> with the request.  In my case the request failed when I went to
> unmarshal the body which was unexpectedly empty.  I'd like to propose
> one of the follow options:
>
> 1) Throw an exception
>
> 2) Log an Error
>
> Both of the above would be possible if with some changes to the
> HttpServerChannelHandler.channelRead0
>
> Currently the code only performs the following.
>
> LOG.debug("Message received: {}", request);
>
> However there could be a check on the request.decoderResult().cause();
> If this is not null we could just throw the exception or a minimum log
> an error level.  My preference would be to throw an exception as this
> will make unrecoverable failures more obvious to developers.  Let me
> know if this direction works for the team.  Happy to help with a
> patch/tests.
>
> Sincerely,
>
> Bob
>
>


--
Claus Ibsen
-----------------
http://davsclaus.com @davsclaus
Camel in Action 2: https://www.manning.com/ibsen2
Reply | Threaded
Open this post in threaded view
|

Re: Camel Netty Http component only shows io.netty.handler.codec.TooLongFrameException at debug log level

Bob Paulin-2
Hi Claus,

I understand what you mean that an exception could fill up the logs.  If
you look at apache tomcat they are taking the approach of halting
processing with an exception[1].  That is probably why I thought that
might be appropriate.  I was originally thinking for the exception
approach we could do this:

        DecoderResult decoderResult = request.decoderResult();

        if(decoderResult != null  && decoderResult.cause() != null) {
            LOG.error("Netty Request Decoder Failure: {}",
decoderResult.cause().getMessage());
            HttpResponse response = new DefaultHttpResponse(HTTP_1_1,
BAD_REQUEST);
            response.headers().set(Exchange.CONTENT_TYPE, "text/plain");
            response.headers().set(Exchange.CONTENT_LENGTH, 0);
            ctx.writeAndFlush(response);
            ctx.channel().close();
            throw new CamelException(decoderResult.cause());
        }


The logging approach could simply be paired back:

       DecoderResult decoderResult = request.decoderResult();

        if(decoderResult != null  && decoderResult.cause() != null) {
            LOG.error("Netty Request Decoder Failure: {}",
decoderResult.cause().getMessage());
        }

Will leave this open a few days to see if there are any other opinions. 
Either approach I think is an improvement.  I've created a JIRA
https://issues.apache.org/jira/browse/CAMEL-14195

Thanks!

- Bob

[1]
https://github.com/apache/tomcat/blob/f4e7cdcff5abc239866e1d69906086cf1bae78ad/java/org/apache/coyote/http11/Http11InputBuffer.java


On 2019/11/20 04:15:54, Claus Ibsen <[hidden email]> wrote:

> Hi Bob>
>
> Yeah we can make better logging, not sure if throwing an exception is>
> the right course as then your server logs can get spammed if http>
> clients send data with long headers.>
> So maybe there should be an option to set the logging level etc.>
>
> You are welcome to create a JIRA and work on a PR>
> https://camel.apache.org/manual/latest/contributing.html>
>
> On Tue, Nov 19, 2019 at 9:39 PM Bob Paulin <[hidden email]> wrote:>
> >>
> > Hi,>
> >>
> > When the Camel Netty Http component receives a header that exceeds
it's>
> > maxHeaderSize the>
> >
org.apache.camel.component.netty.http.handlers.HttpServerChannelHandler>

> > simply logs that a message was received at DEBUG level:>
> >>
> > 2019-11-19T19:34:02,418 | DEBUG | Camel (camel-2) thread #189 ->
> > NettyEventExecutorGroup | NettyHttpConsumer | 95 ->
> > org.apache.camel.camel-core - 2.22.3 | Message received:>
> > HttpObjectAggregator$AggregatedFullHttpRequest(decodeResult:>
> > failure(io.netty.handler.codec.TooLongFrameException: HTTP header is>
> > larger than 8192 bytes.), version: HTTP/1.1, content: EmptyByteBufBE)>
> >>
> > This the request is allowed to proceed to code upstream which>
> > unfortunately leads the developer to believe that there is nothing
wrong>

> > with the request. In my case the request failed when I went to>
> > unmarshal the body which was unexpectedly empty. I'd like to propose>
> > one of the follow options:>
> >>
> > 1) Throw an exception>
> >>
> > 2) Log an Error>
> >>
> > Both of the above would be possible if with some changes to the>
> > HttpServerChannelHandler.channelRead0>
> >>
> > Currently the code only performs the following.>
> >>
> > LOG.debug("Message received: {}", request);>
> >>
> > However there could be a check on the request.decoderResult().cause();>
> > If this is not null we could just throw the exception or a minimum log>
> > an error level. My preference would be to throw an exception as this>
> > will make unrecoverable failures more obvious to developers. Let me>
> > know if this direction works for the team. Happy to help with a>
> > patch/tests.>
> >>
> > Sincerely,>
> >>
> > Bob>
> >>
> >>
>
>
> -- >
> Claus Ibsen>
> ----------------->
> http://davsclaus.com @davsclaus>
> Camel in Action 2: https://www.manning.com/ibsen2>
>


signature.asc (836 bytes) Download Attachment