👍

I ran into some trouble with the Netty HttpObjectEncoder class when I tried to get zero-copy with -1 contentLength to work. Here's what happens:

• In org/asynchttpclient/netty/request/body/NettyBodyBody.java#write(final Channel channel, NettyResponseFuture<?> future) we set msg = new BodyFileRegion((RandomAccessBody) body);
• In io/netty/netty-codec-http/4.1.30.Final/netty-codec-http-4.1.30.Final.jar!/io/netty/handler/codec/http/HttpObjectEncoder.class#encode(ChannelHandlerContext ctx, Object msg, List<Object> out) this causes us to hit the following block

 case 2:
 if (buf != null) {
 out.add(buf);
 }
 this.encodeChunkedContent(ctx, msg, contentLength(msg), out);
 break;

• Taking a look at the contentLength(msg) method, this returns -1 when msg matches FileRegion

private static long contentLength(Object msg) {
 if (msg instanceof HttpContent) {
 return (long)((HttpContent)msg).content().readableBytes();
 } else if (msg instanceof ByteBuf) {
 return (long)((ByteBuf)msg).readableBytes();
 } else if (msg instanceof FileRegion) {
 return ((FileRegion)msg).count();
 } else {
 throw new IllegalStateException("unexpected message type: " + StringUtil.simpleClassName(msg));
 }
 }

• In encodeChunkedContent(ChannelHandlerContext ctx, Object msg, long contentLength, List<Object> out):

private void encodeChunkedContent(ChannelHandlerContext ctx, Object msg, long contentLength, List<Object> out) {
 ByteBuf buf;
 if (contentLength > 0L) {
 String lengthHex = Long.toHexString(contentLength);
 buf = ctx.alloc().buffer(lengthHex.length() + 2);
 buf.writeCharSequence(lengthHex, CharsetUtil.US_ASCII);
 ByteBufUtil.writeShortBE(buf, 3338);
 out.add(buf);
 out.add(encodeAndRetain(msg));
 out.add(CRLF_BUF.duplicate());
 }
 if (msg instanceof LastHttpContent) {
 HttpHeaders headers = ((LastHttpContent)msg).trailingHeaders();
 if (headers.isEmpty()) {
 out.add(ZERO_CRLF_CRLF_BUF.duplicate());
 } else {
 buf = ctx.alloc().buffer((int)this.trailersEncodedSizeAccumulator);
 ByteBufUtil.writeMediumBE(buf, 3149066);
 this.encodeHeaders(headers, buf);
 ByteBufUtil.writeShortBE(buf, 3338);
 this.trailersEncodedSizeAccumulator = 0.2F * (float)padSizeForAccumulation(buf.readableBytes()) + 0.8F * this.trailersEncodedSizeAccumulator;
 out.add(buf);
 }
 } else if (contentLength == 0L) {
 out.add(encodeAndRetain(msg));
 }
 }

• We basically miss all the blocks since the encodeChunkedContent method doesn't handle contentLength < 0L.

@slandelle : Not sure if my solution is acceptable, I'd be open to better ideas.

samridh90 I don't think it's worth trying to contribute chunked transfert encoding support for Netty's FileRegion. We can just disable bypass zero-copy when content-length < 0.
InputStream isn't zero-copy anyway, such it would only be suboptimal when sending a multipart that mixes a local file and an InputStream.
Then, don't bother implementing zero-copy for InputStreamPart as it shouldn't be used.

WDYT?

@slandelle : Sounds good to me. I'll get rid of the zero-copy for InputStreamPart.

Edit: Done, latest update removes zero-copy for InputStreamPart and adds tests that assert that it should not be used.

One last nit and we'll be good :)

Thanks!

Merged. Great job, thanks!

@slandelle: Happy to contribute! I'm not familiar with AHC's release cadence, when do you expect to include this feature in a release?

2.6.0 should be available on maven central in 15 min.