From 3fe1f715117d03d1597fc066ec2f8bc7abb51473 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 4 Oct 2017 12:06:59 -0400 Subject: [PATCH] Do not treat errors as decoder exception (redux) Motivation: Today when Netty encounters a general error while decoding it treats this as a decoder exception. However, for fatal causes this should not be treated as such, instead the fatal error should be carried up the stack without the callee having to unwind causes. This was probably done for byte to byte message decoder but is now done for all decoders. Modifications: Instead of translating any error to a decoder exception, we let those unwind out the stack (note that finally blocks still execute) except in places where an event needs to fire where we fire with the error instead of wrapping in a decoder exception. Result: Fatal errors will not be treated as innocent decoder exceptions. --- .../handler/codec/haproxy/HAProxyMessageDecoder.java | 10 +++++----- .../handler/codec/socksx/v4/Socks4ClientDecoder.java | 2 +- .../handler/codec/socksx/v4/Socks4ServerDecoder.java | 2 +- .../codec/socksx/v5/Socks5CommandRequestDecoder.java | 2 +- .../codec/socksx/v5/Socks5CommandResponseDecoder.java | 2 +- .../codec/socksx/v5/Socks5InitialRequestDecoder.java | 2 +- .../codec/socksx/v5/Socks5InitialResponseDecoder.java | 2 +- .../socksx/v5/Socks5PasswordAuthRequestDecoder.java | 2 +- .../socksx/v5/Socks5PasswordAuthResponseDecoder.java | 2 +- .../java/io/netty/handler/codec/ReplayingDecoder.java | 2 +- .../java/io/netty/handler/ssl/AbstractSniHandler.java | 4 +++- .../src/main/java/io/netty/handler/ssl/SniHandler.java | 6 +++++- 12 files changed, 22 insertions(+), 16 deletions(-) diff --git a/codec-haproxy/src/main/java/io/netty/handler/codec/haproxy/HAProxyMessageDecoder.java b/codec-haproxy/src/main/java/io/netty/handler/codec/haproxy/HAProxyMessageDecoder.java index 04c744811f..df2a663e89 100644 --- a/codec-haproxy/src/main/java/io/netty/handler/codec/haproxy/HAProxyMessageDecoder.java +++ b/codec-haproxy/src/main/java/io/netty/handler/codec/haproxy/HAProxyMessageDecoder.java @@ -355,16 +355,16 @@ public class HAProxyMessageDecoder extends ByteToMessageDecoder { fail(ctx, "header length (" + length + ") exceeds the allowed maximum (" + maxLength + ')', null); } - private void fail(final ChannelHandlerContext ctx, String errMsg, Throwable t) { + private void fail(final ChannelHandlerContext ctx, String errMsg, Exception e) { finished = true; ctx.close(); // drop connection immediately per spec HAProxyProtocolException ppex; - if (errMsg != null && t != null) { - ppex = new HAProxyProtocolException(errMsg, t); + if (errMsg != null && e != null) { + ppex = new HAProxyProtocolException(errMsg, e); } else if (errMsg != null) { ppex = new HAProxyProtocolException(errMsg); - } else if (t != null) { - ppex = new HAProxyProtocolException(t); + } else if (e != null) { + ppex = new HAProxyProtocolException(e); } else { ppex = new HAProxyProtocolException(); } diff --git a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v4/Socks4ClientDecoder.java b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v4/Socks4ClientDecoder.java index f3c627d8b7..6f69939762 100755 --- a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v4/Socks4ClientDecoder.java +++ b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v4/Socks4ClientDecoder.java @@ -78,7 +78,7 @@ public class Socks4ClientDecoder extends ReplayingDecoder { } } - private void fail(List out, Throwable cause) { + private void fail(List out, Exception cause) { if (!(cause instanceof DecoderException)) { cause = new DecoderException(cause); } diff --git a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v4/Socks4ServerDecoder.java b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v4/Socks4ServerDecoder.java index f31181e934..bcfec54922 100755 --- a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v4/Socks4ServerDecoder.java +++ b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v4/Socks4ServerDecoder.java @@ -99,7 +99,7 @@ public class Socks4ServerDecoder extends ReplayingDecoder { } } - private void fail(List out, Throwable cause) { + private void fail(List out, Exception cause) { if (!(cause instanceof DecoderException)) { cause = new DecoderException(cause); } diff --git a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5CommandRequestDecoder.java b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5CommandRequestDecoder.java index 5ec0683c90..eab1b1579e 100644 --- a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5CommandRequestDecoder.java +++ b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5CommandRequestDecoder.java @@ -92,7 +92,7 @@ public class Socks5CommandRequestDecoder extends ReplayingDecoder { } } - private void fail(List out, Throwable cause) { + private void fail(List out, Exception cause) { if (!(cause instanceof DecoderException)) { cause = new DecoderException(cause); } diff --git a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5CommandResponseDecoder.java b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5CommandResponseDecoder.java index 82a16162d9..e3ac387623 100644 --- a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5CommandResponseDecoder.java +++ b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5CommandResponseDecoder.java @@ -91,7 +91,7 @@ public class Socks5CommandResponseDecoder extends ReplayingDecoder { } } - private void fail(List out, Throwable cause) { + private void fail(List out, Exception cause) { if (!(cause instanceof DecoderException)) { cause = new DecoderException(cause); } diff --git a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5InitialRequestDecoder.java b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5InitialRequestDecoder.java index 0ff9646a46..f9a663a9a0 100644 --- a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5InitialRequestDecoder.java +++ b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5InitialRequestDecoder.java @@ -85,7 +85,7 @@ public class Socks5InitialRequestDecoder extends ReplayingDecoder { } } - private void fail(List out, Throwable cause) { + private void fail(List out, Exception cause) { if (!(cause instanceof DecoderException)) { cause = new DecoderException(cause); } diff --git a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5InitialResponseDecoder.java b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5InitialResponseDecoder.java index 9e8d168131..8eb429c634 100644 --- a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5InitialResponseDecoder.java +++ b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5InitialResponseDecoder.java @@ -76,7 +76,7 @@ public class Socks5InitialResponseDecoder extends ReplayingDecoder { } } - private void fail(List out, Throwable cause) { + private void fail(List out, Exception cause) { if (!(cause instanceof DecoderException)) { cause = new DecoderException(cause); } diff --git a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5PasswordAuthRequestDecoder.java b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5PasswordAuthRequestDecoder.java index 576c23e078..896c5f5ea3 100644 --- a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5PasswordAuthRequestDecoder.java +++ b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5PasswordAuthRequestDecoder.java @@ -83,7 +83,7 @@ public class Socks5PasswordAuthRequestDecoder extends ReplayingDecoder { } } - private void fail(List out, Throwable cause) { + private void fail(List out, Exception cause) { if (!(cause instanceof DecoderException)) { cause = new DecoderException(cause); } diff --git a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5PasswordAuthResponseDecoder.java b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5PasswordAuthResponseDecoder.java index 9c09bf9743..d338bd7009 100644 --- a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5PasswordAuthResponseDecoder.java +++ b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/Socks5PasswordAuthResponseDecoder.java @@ -73,7 +73,7 @@ public class Socks5PasswordAuthResponseDecoder extends ReplayingDecoder { } } - private void fail(List out, Throwable cause) { + private void fail(List out, Exception cause) { if (!(cause instanceof DecoderException)) { cause = new DecoderException(cause); } diff --git a/codec/src/main/java/io/netty/handler/codec/ReplayingDecoder.java b/codec/src/main/java/io/netty/handler/codec/ReplayingDecoder.java index 2e10d1ecdf..3d649437fb 100644 --- a/codec/src/main/java/io/netty/handler/codec/ReplayingDecoder.java +++ b/codec/src/main/java/io/netty/handler/codec/ReplayingDecoder.java @@ -418,7 +418,7 @@ public abstract class ReplayingDecoder extends ByteToMessageDecoder { } } catch (DecoderException e) { throw e; - } catch (Throwable cause) { + } catch (Exception cause) { throw new DecoderException(cause); } } diff --git a/handler/src/main/java/io/netty/handler/ssl/AbstractSniHandler.java b/handler/src/main/java/io/netty/handler/ssl/AbstractSniHandler.java index 86e7eb25be..44fda4d16e 100644 --- a/handler/src/main/java/io/netty/handler/ssl/AbstractSniHandler.java +++ b/handler/src/main/java/io/netty/handler/ssl/AbstractSniHandler.java @@ -234,8 +234,10 @@ public abstract class AbstractSniHandler extends ByteToMessageDecoder impleme onLookupComplete(ctx, hostname, future); } catch (DecoderException err) { ctx.fireExceptionCaught(err); - } catch (Throwable cause) { + } catch (Exception cause) { ctx.fireExceptionCaught(new DecoderException(cause)); + } catch (Throwable cause) { + ctx.fireExceptionCaught(cause); } } finally { if (readPending) { diff --git a/handler/src/main/java/io/netty/handler/ssl/SniHandler.java b/handler/src/main/java/io/netty/handler/ssl/SniHandler.java index ada75ecd7b..cda2bbdce7 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SniHandler.java +++ b/handler/src/main/java/io/netty/handler/ssl/SniHandler.java @@ -100,7 +100,11 @@ public class SniHandler extends AbstractSniHandler { protected final void onLookupComplete(ChannelHandlerContext ctx, String hostname, Future future) throws Exception { if (!future.isSuccess()) { - throw new DecoderException("failed to get the SslContext for " + hostname, future.cause()); + final Throwable cause = future.cause(); + if (cause instanceof Error) { + throw (Error) cause; + } + throw new DecoderException("failed to get the SslContext for " + hostname, cause); } SslContext sslContext = future.getNow();