Fix integer overflow in HttpObjectEncoder when handling chunked encoding and FileRegion > Integer.MAX_VALUE

Motivation:

Due to integer overflow bug, writes of FileRegions to http server pipeline (eg like one from HttpStaticFileServer example) with length greater than Integer.MAX_VALUE are ignored in 1/2 of cases (ie no data gets sent to client)

Modification:

Correctly handle chunk sized > Integer.MAX_VALUE

Result:

Be able to use FileRegion > Integer.MAX_VALUE when using chunked encoding.
This commit is contained in:
Alexey Parfenov 2014-06-15 22:44:09 -07:00 committed by Norman Maurer
parent 4a13f66e13
commit 063ca10d87
2 changed files with 116 additions and 5 deletions

View File

@ -78,7 +78,7 @@ public abstract class HttpObjectEncoder<H extends HttpMessage> extends MessageTo
throw new IllegalStateException("unexpected message type: " + StringUtil.simpleClassName(msg)); throw new IllegalStateException("unexpected message type: " + StringUtil.simpleClassName(msg));
} }
int contentLength = contentLength(msg); final long contentLength = contentLength(msg);
if (state == ST_CONTENT_NON_CHUNK) { if (state == ST_CONTENT_NON_CHUNK) {
if (contentLength > 0) { if (contentLength > 0) {
if (buf != null && buf.writableBytes() >= contentLength && msg instanceof HttpContent) { if (buf != null && buf.writableBytes() >= contentLength && msg instanceof HttpContent) {
@ -119,9 +119,9 @@ public abstract class HttpObjectEncoder<H extends HttpMessage> extends MessageTo
} }
} }
private void encodeChunkedContent(ChannelHandlerContext ctx, Object msg, int contentLength, List<Object> out) { private void encodeChunkedContent(ChannelHandlerContext ctx, Object msg, long contentLength, List<Object> out) {
if (contentLength > 0) { if (contentLength > 0) {
byte[] length = Integer.toHexString(contentLength).getBytes(CharsetUtil.US_ASCII); byte[] length = Long.toHexString(contentLength).getBytes(CharsetUtil.US_ASCII);
ByteBuf buf = ctx.alloc().buffer(length.length + 2); ByteBuf buf = ctx.alloc().buffer(length.length + 2);
buf.writeBytes(length); buf.writeBytes(length);
buf.writeBytes(CRLF); buf.writeBytes(CRLF);
@ -170,7 +170,7 @@ public abstract class HttpObjectEncoder<H extends HttpMessage> extends MessageTo
throw new IllegalStateException("unexpected message type: " + StringUtil.simpleClassName(msg)); throw new IllegalStateException("unexpected message type: " + StringUtil.simpleClassName(msg));
} }
private static int contentLength(Object msg) { private static long contentLength(Object msg) {
if (msg instanceof HttpContent) { if (msg instanceof HttpContent) {
return ((HttpContent) msg).content().readableBytes(); return ((HttpContent) msg).content().readableBytes();
} }
@ -178,7 +178,7 @@ public abstract class HttpObjectEncoder<H extends HttpMessage> extends MessageTo
return ((ByteBuf) msg).readableBytes(); return ((ByteBuf) msg).readableBytes();
} }
if (msg instanceof FileRegion) { if (msg instanceof FileRegion) {
return (int) ((FileRegion) msg).count(); return ((FileRegion) msg).count();
} }
throw new IllegalStateException("unexpected message type: " + StringUtil.simpleClassName(msg)); throw new IllegalStateException("unexpected message type: " + StringUtil.simpleClassName(msg));
} }

View File

@ -0,0 +1,111 @@
/*
* Copyright 2014 The Netty Project
*
* The Netty Project licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
package io.netty.handler.codec.http;
import io.netty.buffer.ByteBuf;
import io.netty.channel.FileRegion;
import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.util.CharsetUtil;
import org.junit.Test;
import java.io.IOException;
import java.nio.channels.WritableByteChannel;
import static org.junit.Assert.*;
public class HttpResponseEncoderTest {
private static final long INTEGER_OVERLFLOW = (long) Integer.MAX_VALUE + 1;
private static final FileRegion FILE_REGION = new DummyLongFileRegion();
@Test
public void testLargeFileRegionChunked() throws Exception {
EmbeddedChannel channel = new EmbeddedChannel(new HttpResponseEncoder());
HttpResponse response = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK);
response.headers().set(HttpHeaders.Names.TRANSFER_ENCODING, HttpHeaders.Values.CHUNKED);
assertTrue(channel.writeOutbound(response));
ByteBuf buffer = (ByteBuf) channel.readOutbound();
assertEquals("HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n\r\n", buffer.toString(CharsetUtil.US_ASCII));
buffer.release();
assertTrue(channel.writeOutbound(FILE_REGION));
buffer = (ByteBuf) channel.readOutbound();
assertEquals("80000000\r\n", buffer.toString(CharsetUtil.US_ASCII));
buffer.release();
FileRegion region = (FileRegion) channel.readOutbound();
assertSame(region, FILE_REGION);
region.release();
buffer = (ByteBuf) channel.readOutbound();
assertEquals("\r\n", buffer.toString(CharsetUtil.US_ASCII));
buffer.release();
assertTrue(channel.writeOutbound(LastHttpContent.EMPTY_LAST_CONTENT));
buffer = (ByteBuf) channel.readOutbound();
assertEquals("0\r\n\r\n", buffer.toString(CharsetUtil.US_ASCII));
buffer.release();
assertFalse(channel.finish());
}
private static class DummyLongFileRegion implements FileRegion {
@Override
public long position() {
return 0;
}
@Override
public long transfered() {
return 0;
}
@Override
public long count() {
return INTEGER_OVERLFLOW;
}
@Override
public long transferTo(WritableByteChannel target, long position) throws IOException {
throw new UnsupportedOperationException();
}
@Override
public FileRegion retain() {
return this;
}
@Override
public FileRegion retain(int increment) {
return this;
}
@Override
public int refCnt() {
return 1;
}
@Override
public boolean release() {
return false;
}
@Override
public boolean release(int decrement) {
return false;
}
}
}