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 <c...@gmail.com> 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 <bo...@bobpaulin.com> 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
Description: OpenPGP digital signature
