Add Close method for closing OutputStream and PcapWriteHandler (#10638)

Motivation:
We should implement the Closeable method to properly close `OutputStream` and `PcapWriteHandler`. So whenever `handlerRemoved(ChannelHandlerContext)` is called or the user wants to stop the Pcap writes into `OutputStream`, we have a proper method to close it otherwise writing data to close `OutputStream` will result in `IOException`.

Modification:
Implemented `Closeable` in `PcapWriteHandler` which calls `PcapWriter#close` and closes `OutputStream` and stops Pcap writes.

Result:
Better handling of Pcap writes.
This commit is contained in:
Aayush Atharva 2020-10-08 15:22:17 +05:30 committed by GitHub
parent 3298f5efee
commit 461b6e0f7b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 77 additions and 25 deletions

View File

@ -17,8 +17,6 @@ package io.netty.handler.pcap;
import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBuf;
import java.util.concurrent.TimeUnit;
final class PcapHeaders { final class PcapHeaders {
/** /**

View File

@ -31,6 +31,7 @@ import io.netty.util.internal.ObjectUtil;
import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLogger;
import io.netty.util.internal.logging.InternalLoggerFactory; import io.netty.util.internal.logging.InternalLoggerFactory;
import java.io.Closeable;
import java.io.IOException; import java.io.IOException;
import java.io.OutputStream; import java.io.OutputStream;
import java.net.Inet4Address; import java.net.Inet4Address;
@ -61,8 +62,11 @@ import java.net.InetSocketAddress;
* </ul> * </ul>
* </p> * </p>
*/ */
public final class PcapWriteHandler extends ChannelDuplexHandler { public final class PcapWriteHandler extends ChannelDuplexHandler implements Closeable {
/**
* Logger for logging events
*/
private final InternalLogger logger = InternalLoggerFactory.getInstance(PcapWriteHandler.class); private final InternalLogger logger = InternalLoggerFactory.getInstance(PcapWriteHandler.class);
/** /**
@ -108,12 +112,18 @@ public final class PcapWriteHandler extends ChannelDuplexHandler {
*/ */
private InetSocketAddress dstAddr; private InetSocketAddress dstAddr;
/**
* Set to {@code true} if {@link #close()} is called and we should stop writing Pcap.
*/
private boolean isClosed;
/** /**
* Create new {@link PcapWriteHandler} Instance. * Create new {@link PcapWriteHandler} Instance.
* {@code captureZeroByte} is set to {@code false} and * {@code captureZeroByte} is set to {@code false} and
* {@code writePcapGlobalHeader} is set to {@code true}. * {@code writePcapGlobalHeader} is set to {@code true}.
* *
* @param outputStream OutputStream where Pcap data will be written * @param outputStream OutputStream where Pcap data will be written. Call {@link #close()} to close this
* OutputStream.
* @throws NullPointerException If {@link OutputStream} is {@code null} then we'll throw an * @throws NullPointerException If {@link OutputStream} is {@code null} then we'll throw an
* {@link NullPointerException} * {@link NullPointerException}
*/ */
@ -124,7 +134,8 @@ public final class PcapWriteHandler extends ChannelDuplexHandler {
/** /**
* Create new {@link PcapWriteHandler} Instance * Create new {@link PcapWriteHandler} Instance
* *
* @param outputStream OutputStream where Pcap data will be written * @param outputStream OutputStream where Pcap data will be written. Call {@link #close()} to close this
* OutputStream.
* @param captureZeroByte Set to {@code true} to enable capturing packets with empty (0 bytes) payload. * @param captureZeroByte Set to {@code true} to enable capturing packets with empty (0 bytes) payload.
* Otherwise, if set to {@code false}, empty packets will be filtered out. * Otherwise, if set to {@code false}, empty packets will be filtered out.
* @param writePcapGlobalHeader Set to {@code true} to write Pcap Global Header on initialization. * @param writePcapGlobalHeader Set to {@code true} to write Pcap Global Header on initialization.
@ -214,24 +225,28 @@ public final class PcapWriteHandler extends ChannelDuplexHandler {
@Override @Override
public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception { public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
if (ctx.channel() instanceof SocketChannel) { if (!isClosed) {
handleTCP(ctx, msg, false); if (ctx.channel() instanceof SocketChannel) {
} else if (ctx.channel() instanceof DatagramChannel) { handleTCP(ctx, msg, false);
handleUDP(ctx, msg); } else if (ctx.channel() instanceof DatagramChannel) {
} else { handleUDP(ctx, msg);
logger.debug("Discarding Pcap Write for Unknown Channel Type: {}", ctx.channel()); } else {
logger.debug("Discarding Pcap Write for Unknown Channel Type: {}", ctx.channel());
}
} }
super.channelRead(ctx, msg); super.channelRead(ctx, msg);
} }
@Override @Override
public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise) throws Exception { public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise) throws Exception {
if (ctx.channel() instanceof SocketChannel) { if (!isClosed) {
handleTCP(ctx, msg, true); if (ctx.channel() instanceof SocketChannel) {
} else if (ctx.channel() instanceof DatagramChannel) { handleTCP(ctx, msg, true);
handleUDP(ctx, msg); } else if (ctx.channel() instanceof DatagramChannel) {
} else { handleUDP(ctx, msg);
logger.debug("Discarding Pcap Write for Unknown Channel Type: {}", ctx.channel()); } else {
logger.debug("Discarding Pcap Write for Unknown Channel Type: {}", ctx.channel());
}
} }
super.write(ctx, msg, promise); super.write(ctx, msg, promise);
} }
@ -495,7 +510,7 @@ public final class PcapWriteHandler extends ChannelDuplexHandler {
logger.debug("Finished Fake TCP FIN+ACK Flow to close connection"); logger.debug("Finished Fake TCP FIN+ACK Flow to close connection");
} }
this.pCapWriter.close(); close();
super.handlerRemoved(ctx); super.handlerRemoved(ctx);
} }
@ -517,7 +532,25 @@ public final class PcapWriteHandler extends ChannelDuplexHandler {
logger.debug("Sent Fake TCP RST to close connection"); logger.debug("Sent Fake TCP RST to close connection");
} }
this.pCapWriter.close(); close();
ctx.fireExceptionCaught(cause); ctx.fireExceptionCaught(cause);
} }
/**
* <p> Close {@code PcapWriter} and {@link OutputStream}. </p>
* <p> Note: Calling this method does not close {@link PcapWriteHandler}.
* Only Pcap Writes are closed. </p>
*
* @throws IOException If {@link OutputStream#close()} throws an exception
*/
@Override
public void close() throws IOException {
if (isClosed) {
logger.debug("PcapWriterHandler is already closed");
} else {
isClosed = true;
pCapWriter.close();
logger.debug("PcapWriterHandler is now closed");
}
}
} }

View File

@ -16,19 +16,30 @@
package io.netty.handler.pcap; package io.netty.handler.pcap;
import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBuf;
import io.netty.util.internal.logging.InternalLogger;
import io.netty.util.internal.logging.InternalLoggerFactory;
import java.io.Closeable; import java.io.Closeable;
import java.io.IOException; import java.io.IOException;
import java.io.OutputStream; import java.io.OutputStream;
import java.util.concurrent.TimeUnit;
final class PcapWriter implements Closeable { final class PcapWriter implements Closeable {
/**
* Logger
*/
private static final InternalLogger logger = InternalLoggerFactory.getInstance(PcapWriter.class);
/** /**
* {@link OutputStream} where we'll write Pcap data. * {@link OutputStream} where we'll write Pcap data.
*/ */
private final OutputStream outputStream; private final OutputStream outputStream;
/**
* Set to {@code true} if {@link #outputStream} is closed.
*/
private boolean isClosed;
/** /**
* This uses {@link OutputStream} for writing Pcap. * This uses {@link OutputStream} for writing Pcap.
* Pcap Global Header is not written on construction. * Pcap Global Header is not written on construction.
@ -58,12 +69,16 @@ final class PcapWriter implements Closeable {
* @throws IOException If {@link OutputStream#write(byte[])} throws an exception * @throws IOException If {@link OutputStream#write(byte[])} throws an exception
*/ */
void writePacket(ByteBuf packetHeaderBuf, ByteBuf packet) throws IOException { void writePacket(ByteBuf packetHeaderBuf, ByteBuf packet) throws IOException {
long currentTime = System.currentTimeMillis(); if (isClosed) {
logger.debug("Pcap Write attempted on closed PcapWriter");
}
long timestamp = System.currentTimeMillis();
PcapHeaders.writePacketHeader( PcapHeaders.writePacketHeader(
packetHeaderBuf, packetHeaderBuf,
(int) TimeUnit.MILLISECONDS.toSeconds(currentTime), (int) (timestamp / 1000L),
(int) TimeUnit.MILLISECONDS.toMicros(currentTime) % 1000000, (int) (timestamp % 1000L * 1000L),
packet.readableBytes(), packet.readableBytes(),
packet.readableBytes() packet.readableBytes()
); );
@ -74,7 +89,13 @@ final class PcapWriter implements Closeable {
@Override @Override
public void close() throws IOException { public void close() throws IOException {
outputStream.flush(); if (isClosed) {
outputStream.close(); logger.debug("PcapWriter is already closed");
} else {
isClosed = true;
outputStream.flush();
outputStream.close();
logger.debug("PcapWriter is now closed");
}
} }
} }