grpc
/
grpc-java
mirror of https://github.com/grpc/grpc-java.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Guard against NPE in Netty handlers 

We've seen an NPE as a side-effect of a failure in protocol negotiation:

Oct 27, 2015 9:27:09 PM io.grpc.transport.netty.ProtocolNegotiators$AbstractBufferingHandler fail
SEVERE: Transport failed during protocol negotiation
java.lang.NullPointerException
	at io.grpc.transport.netty.NettyClientHandler$2.visit(NettyClientHandler.java:218)
This commit is contained in:
nmittler 2015-10-28 07:50:49 -07:00
parent a2216f665a
commit 3efc61b237
2 changed files with 27 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
netty/src/main/java/io/grpc/netty/NettyClientHandler.java
View File

@ -202,9 +202,11 @@ class NettyClientHandler extends AbstractNettyHandler {
* Handler for an inbound HTTP/2 RST_STREAM frame, terminating a stream. * Handler for an inbound HTTP/2 RST_STREAM frame, terminating a stream.
*/ */
private void onRstStreamRead(int streamId, long errorCode) throws Http2Exception { private void onRstStreamRead(int streamId, long errorCode) throws Http2Exception {
NettyClientStream stream = clientStream(requireHttp2Stream(streamId)); NettyClientStream stream = clientStream(connection().stream(streamId));
Status status = GrpcUtil.Http2Error.statusForCode((int) errorCode); if (stream != null) {
stream.transportReportStatus(status, false /*stop delivery*/, new Metadata()); Status status = GrpcUtil.Http2Error.statusForCode((int) errorCode);
stream.transportReportStatus(status, false /*stop delivery*/, new Metadata());
}
} }
@Override @Override
@ -226,7 +228,10 @@ class NettyClientHandler extends AbstractNettyHandler {
connection().forEachActiveStream(new Http2StreamVisitor() { connection().forEachActiveStream(new Http2StreamVisitor() {
@Override @Override
public boolean visit(Http2Stream stream) throws Http2Exception { public boolean visit(Http2Stream stream) throws Http2Exception {
clientStream(stream).transportReportStatus(goAwayStatus, false, new Metadata()); NettyClientStream clientStream = clientStream(stream);
if (clientStream != null) {
clientStream.transportReportStatus(goAwayStatus, false, new Metadata());
}
return true; return true;
} }
}); });
@ -248,9 +253,9 @@ class NettyClientHandler extends AbstractNettyHandler {
protected void onStreamError(ChannelHandlerContext ctx, Throwable cause, protected void onStreamError(ChannelHandlerContext ctx, Throwable cause,
Http2Exception.StreamException http2Ex) { Http2Exception.StreamException http2Ex) {
// Close the stream with a status that contains the cause. // Close the stream with a status that contains the cause.
Http2Stream stream = connection().stream(http2Ex.streamId()); NettyClientStream stream = clientStream(connection().stream(http2Ex.streamId()));
if (stream != null) { if (stream != null) {
clientStream(stream).transportReportStatus(statusFromError(cause), false, new Metadata()); stream.transportReportStatus(statusFromError(cause), false, new Metadata());
} }
// Delegate to the base class to send a RST_STREAM. // Delegate to the base class to send a RST_STREAM.
@ -398,8 +403,10 @@ class NettyClientHandler extends AbstractNettyHandler {
@Override @Override
public boolean visit(Http2Stream stream) throws Http2Exception { public boolean visit(Http2Stream stream) throws Http2Exception {
if (stream.id() > lastKnownStream) { if (stream.id() > lastKnownStream) {
clientStream(stream) NettyClientStream clientStream = clientStream(stream);
.transportReportStatus(goAwayStatus, false, new Metadata()); if (clientStream != null) {
clientStream.transportReportStatus(goAwayStatus, false, new Metadata());
}
stream.close(); stream.close();
} }
return true; return true;
@ -445,7 +452,7 @@ class NettyClientHandler extends AbstractNettyHandler {
* Gets the client stream associated to the given HTTP/2 stream object. * Gets the client stream associated to the given HTTP/2 stream object.
*/ */
private NettyClientStream clientStream(Http2Stream stream) { private NettyClientStream clientStream(Http2Stream stream) {
return stream.getProperty(streamKey); return stream == null ? null : (NettyClientStream) stream.getProperty(streamKey);
} }
private int getAndIncrementNextStreamId() throws StatusException { private int getAndIncrementNextStreamId() throws StatusException {

17
netty/src/main/java/io/grpc/netty/NettyServerHandler.java
View File

@ -199,8 +199,10 @@ class NettyServerHandler extends AbstractNettyHandler {
private void onRstStreamRead(int streamId) throws Http2Exception { private void onRstStreamRead(int streamId) throws Http2Exception {
try { try {
NettyServerStream stream = serverStream(requireHttp2Stream(streamId)); NettyServerStream stream = serverStream(connection().stream(streamId));
stream.abortStream(Status.CANCELLED, false); if (stream != null) {
stream.abortStream(Status.CANCELLED, false);
}
} catch (Throwable e) { } catch (Throwable e) {
logger.log(Level.WARNING, "Exception in onRstStreamRead()", e); logger.log(Level.WARNING, "Exception in onRstStreamRead()", e);
// Throw an exception that will get handled by onStreamError. // Throw an exception that will get handled by onStreamError.
@ -220,8 +222,8 @@ class NettyServerHandler extends AbstractNettyHandler {
protected void onStreamError(ChannelHandlerContext ctx, Throwable cause, protected void onStreamError(ChannelHandlerContext ctx, Throwable cause,
StreamException http2Ex) { StreamException http2Ex) {
logger.log(Level.WARNING, "Stream Error", cause); logger.log(Level.WARNING, "Stream Error", cause);
Http2Stream stream = connection().stream(Http2Exception.streamId(http2Ex)); NettyServerStream serverStream = serverStream(
NettyServerStream serverStream = stream != null ? serverStream(stream) : null; connection().stream(Http2Exception.streamId(http2Ex)));
if (serverStream != null) { if (serverStream != null) {
// Abort the stream with a status to help the client with debugging. // Abort the stream with a status to help the client with debugging.
serverStream.abortStream(cause instanceof Http2Exception serverStream.abortStream(cause instanceof Http2Exception
@ -242,7 +244,10 @@ class NettyServerHandler extends AbstractNettyHandler {
connection().forEachActiveStream(new Http2StreamVisitor() { connection().forEachActiveStream(new Http2StreamVisitor() {
@Override @Override
public boolean visit(Http2Stream stream) throws Http2Exception { public boolean visit(Http2Stream stream) throws Http2Exception {
serverStream(stream).abortStream(GOAWAY_STATUS, false); NettyServerStream serverStream = serverStream(stream);
if (serverStream != null) {
serverStream.abortStream(GOAWAY_STATUS, false);
}
return true; return true;
} }
}); });
@ -376,7 +381,7 @@ class NettyServerHandler extends AbstractNettyHandler {
* Returns the server stream associated to the given HTTP/2 stream object. * Returns the server stream associated to the given HTTP/2 stream object.
*/ */
private NettyServerStream serverStream(Http2Stream stream) { private NettyServerStream serverStream(Http2Stream stream) {
return stream.getProperty(streamKey); return stream == null ? null : (NettyServerStream) stream.getProperty(streamKey);
} }
private Http2Exception newStreamException(int streamId, Throwable cause) { private Http2Exception newStreamException(int streamId, Throwable cause) {