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
parent e3b3cf27da
commit 5b00058fa7
5 changed files with 85 additions and 50 deletions

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

@ -15,7 +15,7 @@
package io.netty.handler.codec.http2;
import io.netty.util.internal.PlatformDependent;
import io.netty.util.internal.ThrowableUtil;
import io.netty.util.internal.UnstableApi;
import java.util.ArrayList;
@ -63,11 +63,10 @@ public class Http2Exception extends Exception {
this.shutdownHint = requireNonNull(shutdownHint, "shutdownHint");
}
static Http2Exception newStatic(Http2Error error, String message, ShutdownHint shutdownHint) {
if (PlatformDependent.javaVersion() >= 7) {
return new Http2Exception(error, message, shutdownHint, true);
}
return new Http2Exception(error, message, shutdownHint);
static Http2Exception newStatic(Http2Error error, String message, ShutdownHint shutdownHint,
Class<?> clazz, String method) {
return ThrowableUtil.unknownStackTrace(
new StacklessHttp2Exception(error, message, shutdownHint), clazz, method);
}
private Http2Exception(Http2Error error, String message, ShutdownHint shutdownHint, boolean shared) {
@ -302,4 +301,20 @@ 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, true);
}
// 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

@ -39,8 +39,8 @@ public class DefaultPromise<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;
@ -754,4 +754,22 @@ public class DefaultPromise<V> implements Promise<V> {
}
return stageAdapter;
}
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

@ -61,26 +61,21 @@ import static java.util.Objects.requireNonNull;
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;
@ -117,12 +112,21 @@ abstract class DnsResolveContext<T> {
static final class DnsResolveContextException extends RuntimeException {
private static final long serialVersionUID = 1209303419266433003L;
private DnsResolveContextException(String message) {
super(message, null, false, true);
}
static DnsResolveContextException newStatic(String message) {
return new DnsResolveContextException(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) {
return ThrowableUtil.unknownStackTrace(new DnsResolveContextException(message), clazz, method);
}
}