Ensure we don't leak the ClassLoader in the backtrace (#10691)

Motivation:

We have a few classes in which we store and reuse static instances of various exceptions. When doing so it is important to also override fillInStacktrace() and so prevent the leak of the ClassLoader in the internal backtrace field.

Modifications:

- Add overrides of fillInStracktrace when needed
- Move ThrowableUtil usage in the static methods

Result:

Fixes https://github.com/netty/netty/pull/10686
This commit is contained in:
Norman Maurer 2020-10-15 20:41:29 +02:00 committed by GitHub
parent a3c154f267
commit 6b613682ba
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 155 additions and 62 deletions

View File

@ -17,6 +17,7 @@ package io.netty.handler.codec.spdy;
import io.netty.util.internal.PlatformDependent;
import io.netty.util.internal.SuppressJava6Requirement;
import io.netty.util.internal.ThrowableUtil;
public class SpdyProtocolException extends Exception {
@ -48,11 +49,14 @@ public class SpdyProtocolException extends Exception {
super(cause);
}
static SpdyProtocolException newStatic(String message) {
static SpdyProtocolException newStatic(String message, Class<?> clazz, String method) {
final SpdyProtocolException exception;
if (PlatformDependent.javaVersion() >= 7) {
return new SpdyProtocolException(message, true);
exception = new StacklessSpdyProtocolException(message, true);
} else {
exception = new StacklessSpdyProtocolException(message);
}
return new SpdyProtocolException(message);
return ThrowableUtil.unknownStackTrace(exception, clazz, method);
}
@SuppressJava6Requirement(reason = "uses Java 7+ Exception.<init>(String, Throwable, boolean, boolean)" +
@ -61,4 +65,23 @@ public class SpdyProtocolException extends Exception {
super(message, null, false, true);
assert shared;
}
private static final class StacklessSpdyProtocolException extends SpdyProtocolException {
private static final long serialVersionUID = -6302754207557485099L;
StacklessSpdyProtocolException(String message) {
super(message);
}
StacklessSpdyProtocolException(String message, boolean shared) {
super(message, shared);
}
// Override fillInStackTrace() so we not populate the backtrace via a native call and so leak the
// Classloader.
@Override
public Throwable fillInStackTrace() {
return this;
}
}
}

View File

@ -21,7 +21,6 @@ import io.netty.channel.ChannelFutureListener;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelPromise;
import io.netty.util.internal.ObjectUtil;
import io.netty.util.internal.ThrowableUtil;
import java.util.concurrent.atomic.AtomicInteger;
@ -34,10 +33,10 @@ import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero;
*/
public class SpdySessionHandler extends ChannelDuplexHandler {
private static final SpdyProtocolException PROTOCOL_EXCEPTION = ThrowableUtil.unknownStackTrace(
SpdyProtocolException.newStatic(null), SpdySessionHandler.class, "handleOutboundMessage(...)");
private static final SpdyProtocolException STREAM_CLOSED = ThrowableUtil.unknownStackTrace(
SpdyProtocolException.newStatic("Stream closed"), SpdySessionHandler.class, "removeStream(...)");
private static final SpdyProtocolException PROTOCOL_EXCEPTION =
SpdyProtocolException.newStatic(null, SpdySessionHandler.class, "handleOutboundMessage(...)");
private static final SpdyProtocolException STREAM_CLOSED =
SpdyProtocolException.newStatic("Stream closed", SpdySessionHandler.class, "removeStream(...)");
private static final int DEFAULT_WINDOW_SIZE = 64 * 1024; // 64 KB default initial window size
private int initialSendWindowSize = DEFAULT_WINDOW_SIZE;

View File

@ -49,35 +49,34 @@ import static io.netty.handler.codec.http2.Http2Headers.PseudoHeaderName.getPseu
import static io.netty.handler.codec.http2.Http2Headers.PseudoHeaderName.hasPseudoHeaderFormat;
import static io.netty.util.AsciiString.EMPTY_STRING;
import static io.netty.util.internal.ObjectUtil.checkPositive;
import static io.netty.util.internal.ThrowableUtil.unknownStackTrace;
final class HpackDecoder {
private static final Http2Exception DECODE_ULE_128_DECOMPRESSION_EXCEPTION = unknownStackTrace(
private static final Http2Exception DECODE_ULE_128_DECOMPRESSION_EXCEPTION =
Http2Exception.newStatic(COMPRESSION_ERROR, "HPACK - decompression failure",
Http2Exception.ShutdownHint.HARD_SHUTDOWN), HpackDecoder.class,
Http2Exception.ShutdownHint.HARD_SHUTDOWN, HpackDecoder.class,
"decodeULE128(..)");
private static final Http2Exception DECODE_ULE_128_TO_LONG_DECOMPRESSION_EXCEPTION = unknownStackTrace(
private static final Http2Exception DECODE_ULE_128_TO_LONG_DECOMPRESSION_EXCEPTION =
Http2Exception.newStatic(COMPRESSION_ERROR, "HPACK - long overflow",
Http2Exception.ShutdownHint.HARD_SHUTDOWN), HpackDecoder.class, "decodeULE128(..)");
private static final Http2Exception DECODE_ULE_128_TO_INT_DECOMPRESSION_EXCEPTION = unknownStackTrace(
Http2Exception.ShutdownHint.HARD_SHUTDOWN, HpackDecoder.class, "decodeULE128(..)");
private static final Http2Exception DECODE_ULE_128_TO_INT_DECOMPRESSION_EXCEPTION =
Http2Exception.newStatic(COMPRESSION_ERROR, "HPACK - int overflow",
Http2Exception.ShutdownHint.HARD_SHUTDOWN), HpackDecoder.class, "decodeULE128ToInt(..)");
private static final Http2Exception DECODE_ILLEGAL_INDEX_VALUE = unknownStackTrace(
Http2Exception.ShutdownHint.HARD_SHUTDOWN, HpackDecoder.class, "decodeULE128ToInt(..)");
private static final Http2Exception DECODE_ILLEGAL_INDEX_VALUE =
Http2Exception.newStatic(COMPRESSION_ERROR, "HPACK - illegal index value",
Http2Exception.ShutdownHint.HARD_SHUTDOWN), HpackDecoder.class, "decode(..)");
private static final Http2Exception INDEX_HEADER_ILLEGAL_INDEX_VALUE = unknownStackTrace(
Http2Exception.ShutdownHint.HARD_SHUTDOWN, HpackDecoder.class, "decode(..)");
private static final Http2Exception INDEX_HEADER_ILLEGAL_INDEX_VALUE =
Http2Exception.newStatic(COMPRESSION_ERROR, "HPACK - illegal index value",
Http2Exception.ShutdownHint.HARD_SHUTDOWN), HpackDecoder.class, "indexHeader(..)");
private static final Http2Exception READ_NAME_ILLEGAL_INDEX_VALUE = unknownStackTrace(
Http2Exception.ShutdownHint.HARD_SHUTDOWN, HpackDecoder.class, "indexHeader(..)");
private static final Http2Exception READ_NAME_ILLEGAL_INDEX_VALUE =
Http2Exception.newStatic(COMPRESSION_ERROR, "HPACK - illegal index value",
Http2Exception.ShutdownHint.HARD_SHUTDOWN), HpackDecoder.class, "readName(..)");
private static final Http2Exception INVALID_MAX_DYNAMIC_TABLE_SIZE = unknownStackTrace(
Http2Exception.ShutdownHint.HARD_SHUTDOWN, HpackDecoder.class, "readName(..)");
private static final Http2Exception INVALID_MAX_DYNAMIC_TABLE_SIZE =
Http2Exception.newStatic(COMPRESSION_ERROR, "HPACK - invalid max dynamic table size",
Http2Exception.ShutdownHint.HARD_SHUTDOWN), HpackDecoder.class,
Http2Exception.ShutdownHint.HARD_SHUTDOWN, HpackDecoder.class,
"setDynamicTableSize(..)");
private static final Http2Exception MAX_DYNAMIC_TABLE_SIZE_CHANGE_REQUIRED = unknownStackTrace(
private static final Http2Exception MAX_DYNAMIC_TABLE_SIZE_CHANGE_REQUIRED =
Http2Exception.newStatic(COMPRESSION_ERROR, "HPACK - max dynamic table size change required",
Http2Exception.ShutdownHint.HARD_SHUTDOWN), HpackDecoder.class, "decode(..)");
Http2Exception.ShutdownHint.HARD_SHUTDOWN, HpackDecoder.class, "decode(..)");
private static final byte READ_HEADER_REPRESENTATION = 0;
private static final byte READ_MAX_DYNAMIC_TABLE_SIZE = 1;
private static final byte READ_INDEXED_HEADER = 2;

View File

@ -34,7 +34,6 @@ package io.netty.handler.codec.http2;
import io.netty.buffer.ByteBuf;
import io.netty.util.AsciiString;
import io.netty.util.ByteProcessor;
import io.netty.util.internal.ThrowableUtil;
import static io.netty.handler.codec.http2.Http2Error.COMPRESSION_ERROR;
@ -4665,9 +4664,9 @@ final class HpackHuffmanDecoder implements ByteProcessor {
HUFFMAN_FAIL << 8,
};
private static final Http2Exception BAD_ENCODING = ThrowableUtil.unknownStackTrace(
private static final Http2Exception BAD_ENCODING =
Http2Exception.newStatic(COMPRESSION_ERROR, "HPACK - Bad Encoding",
Http2Exception.ShutdownHint.HARD_SHUTDOWN), HpackHuffmanDecoder.class, "decode(..)");
Http2Exception.ShutdownHint.HARD_SHUTDOWN, HpackHuffmanDecoder.class, "decode(..)");
private byte[] dest;
private int k;

View File

@ -17,6 +17,7 @@ package io.netty.handler.codec.http2;
import io.netty.util.internal.PlatformDependent;
import io.netty.util.internal.SuppressJava6Requirement;
import io.netty.util.internal.ThrowableUtil;
import io.netty.util.internal.UnstableApi;
import java.util.ArrayList;
@ -64,11 +65,15 @@ public class Http2Exception extends Exception {
this.shutdownHint = checkNotNull(shutdownHint, "shutdownHint");
}
static Http2Exception newStatic(Http2Error error, String message, ShutdownHint shutdownHint) {
static Http2Exception newStatic(Http2Error error, String message, ShutdownHint shutdownHint,
Class<?> clazz, String method) {
final Http2Exception exception;
if (PlatformDependent.javaVersion() >= 7) {
return new Http2Exception(error, message, shutdownHint, true);
exception = new StacklessHttp2Exception(error, message, shutdownHint, true);
} else {
exception = new StacklessHttp2Exception(error, message, shutdownHint);
}
return new Http2Exception(error, message, shutdownHint);
return ThrowableUtil.unknownStackTrace(exception, clazz, method);
}
@SuppressJava6Requirement(reason = "uses Java 7+ Exception.<init>(String, Throwable, boolean, boolean)" +
@ -305,4 +310,24 @@ public class Http2Exception extends Exception {
return exceptions.iterator();
}
}
private static final class StacklessHttp2Exception extends Http2Exception {
private static final long serialVersionUID = 1077888485687219443L;
StacklessHttp2Exception(Http2Error error, String message, ShutdownHint shutdownHint) {
super(error, message, shutdownHint);
}
StacklessHttp2Exception(Http2Error error, String message, ShutdownHint shutdownHint, boolean shared) {
super(error, message, shutdownHint, shared);
}
// Override fillInStackTrace() so we not populate the backtrace via a native call and so leak the
// Classloader.
@Override
public Throwable fillInStackTrace() {
return this;
}
}
}

View File

@ -43,8 +43,8 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
AtomicReferenceFieldUpdater.newUpdater(DefaultPromise.class, Object.class, "result");
private static final Object SUCCESS = new Object();
private static final Object UNCANCELLABLE = new Object();
private static final CauseHolder CANCELLATION_CAUSE_HOLDER = new CauseHolder(ThrowableUtil.unknownStackTrace(
new CancellationException(), DefaultPromise.class, "cancel(...)"));
private static final CauseHolder CANCELLATION_CAUSE_HOLDER = new CauseHolder(
StacklessCancellationException.newInstance(DefaultPromise.class, "cancel(...)"));
private static final StackTraceElement[] CANCELLATION_STACK = CANCELLATION_CAUSE_HOLDER.cause.getStackTrace();
private volatile Object result;
@ -843,4 +843,22 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
rejectedExecutionLogger.error("Failed to submit a listener notification task. Event loop shut down?", t);
}
}
private static final class StacklessCancellationException extends CancellationException {
private static final long serialVersionUID = -2974906711413716191L;
private StacklessCancellationException() { }
// Override fillInStackTrace() so we not populate the backtrace via a native call and so leak the
// Classloader.
@Override
public Throwable fillInStackTrace() {
return this;
}
static StacklessCancellationException newInstance(Class<?> clazz, String method) {
return ThrowableUtil.unknownStackTrace(new StacklessCancellationException(), clazz, method);
}
}
}

View File

@ -62,26 +62,21 @@ import static java.lang.Math.min;
abstract class DnsResolveContext<T> {
private static final RuntimeException NXDOMAIN_QUERY_FAILED_EXCEPTION = ThrowableUtil.unknownStackTrace(
DnsResolveContextException.newStatic("No answer found and NXDOMAIN response code returned"),
DnsResolveContext.class,
"onResponse(..)");
private static final RuntimeException CNAME_NOT_FOUND_QUERY_FAILED_EXCEPTION = ThrowableUtil.unknownStackTrace(
DnsResolveContextException.newStatic("No matching CNAME record found"),
DnsResolveContext.class,
"onResponseCNAME(..)");
private static final RuntimeException NO_MATCHING_RECORD_QUERY_FAILED_EXCEPTION = ThrowableUtil.unknownStackTrace(
DnsResolveContextException.newStatic("No matching record type found"),
DnsResolveContext.class,
"onResponseAorAAAA(..)");
private static final RuntimeException UNRECOGNIZED_TYPE_QUERY_FAILED_EXCEPTION = ThrowableUtil.unknownStackTrace(
new RuntimeException("Response type was unrecognized"),
DnsResolveContext.class,
"onResponse(..)");
private static final RuntimeException NAME_SERVERS_EXHAUSTED_EXCEPTION = ThrowableUtil.unknownStackTrace(
DnsResolveContextException.newStatic("No name servers returned an answer"),
DnsResolveContext.class,
"tryToFinishResolve(..)");
private static final RuntimeException NXDOMAIN_QUERY_FAILED_EXCEPTION =
DnsResolveContextException.newStatic("No answer found and NXDOMAIN response code returned",
DnsResolveContext.class, "onResponse(..)");
private static final RuntimeException CNAME_NOT_FOUND_QUERY_FAILED_EXCEPTION =
DnsResolveContextException.newStatic("No matching CNAME record found",
DnsResolveContext.class, "onResponseCNAME(..)");
private static final RuntimeException NO_MATCHING_RECORD_QUERY_FAILED_EXCEPTION =
DnsResolveContextException.newStatic("No matching record type found",
DnsResolveContext.class, "onResponseAorAAAA(..)");
private static final RuntimeException UNRECOGNIZED_TYPE_QUERY_FAILED_EXCEPTION =
DnsResolveContextException.newStatic("Response type was unrecognized",
DnsResolveContext.class, "onResponse(..)");
private static final RuntimeException NAME_SERVERS_EXHAUSTED_EXCEPTION =
DnsResolveContextException.newStatic("No name servers returned an answer",
DnsResolveContext.class, "tryToFinishResolve(..)");
final DnsNameResolver parent;
private final Promise<?> originalPromise;
@ -118,6 +113,8 @@ abstract class DnsResolveContext<T> {
static final class DnsResolveContextException extends RuntimeException {
private static final long serialVersionUID = 1209303419266433003L;
private DnsResolveContextException(String message) {
super(message);
}
@ -129,11 +126,21 @@ abstract class DnsResolveContext<T> {
assert shared;
}
static DnsResolveContextException newStatic(String message) {
// Override fillInStackTrace() so we not populate the backtrace via a native call and so leak the
// Classloader.
@Override
public Throwable fillInStackTrace() {
return this;
}
static DnsResolveContextException newStatic(String message, Class<?> clazz, String method) {
final DnsResolveContextException exception;
if (PlatformDependent.javaVersion() >= 7) {
return new DnsResolveContextException(message, true);
exception = new DnsResolveContextException(message, true);
} else {
exception = new DnsResolveContextException(message);
}
return new DnsResolveContextException(message);
return ThrowableUtil.unknownStackTrace(exception, clazz, method);
}
}

View File

@ -17,6 +17,7 @@ package io.netty.channel;
import io.netty.util.internal.PlatformDependent;
import io.netty.util.internal.SuppressJava6Requirement;
import io.netty.util.internal.ThrowableUtil;
import io.netty.util.internal.UnstableApi;
/**
@ -61,10 +62,33 @@ public class ChannelException extends RuntimeException {
assert shared;
}
static ChannelException newStatic(String message, Throwable cause) {
static ChannelException newStatic(String message, Class<?> clazz, String method) {
ChannelException exception;
if (PlatformDependent.javaVersion() >= 7) {
return new ChannelException(message, cause, true);
exception = new StacklessChannelException(message, null, true);
} else {
exception = new StacklessChannelException(message, null);
}
return ThrowableUtil.unknownStackTrace(exception, clazz, method);
}
private static final class StacklessChannelException extends ChannelException {
private static final long serialVersionUID = -6384642137753538579L;
StacklessChannelException(String message, Throwable cause) {
super(message, cause);
}
StacklessChannelException(String message, Throwable cause, boolean shared) {
super(message, cause, shared);
}
// Override fillInStackTrace() so we not populate the backtrace via a native call and so leak the
// Classloader.
@Override
public Throwable fillInStackTrace() {
return this;
}
return new ChannelException(message, cause);
}
}

View File

@ -29,7 +29,6 @@ import io.netty.util.internal.EmptyArrays;
import io.netty.util.internal.ObjectUtil;
import io.netty.util.internal.PlatformDependent;
import io.netty.util.internal.ReadOnlyIterator;
import io.netty.util.internal.ThrowableUtil;
import java.util.Collections;
import java.util.Iterator;
@ -132,8 +131,8 @@ public class ThreadPerChannelEventLoopGroup extends AbstractEventExecutorGroup i
this.maxChannels = maxChannels;
this.executor = executor;
tooManyChannels = ThrowableUtil.unknownStackTrace(
ChannelException.newStatic("too many channels (max: " + maxChannels + ')', null),
tooManyChannels =
ChannelException.newStatic("too many channels (max: " + maxChannels + ')',
ThreadPerChannelEventLoopGroup.class, "nextChild()");
}