From 686df17b1a1bca9ca2d26fe53e5e4eb3404a6b3f Mon Sep 17 00:00:00 2001 From: Andrey Mizurov Date: Fri, 7 Aug 2020 14:53:16 +0300 Subject: [PATCH] Fix #10449, buffer.getBytes(...) not change a file channel position (#10453) Motivation: Regression appeared after making changes in fix #10360 . The main problem here that `buffer.getBytes(buffer.readerIndex(), fileChannel, fileChannel.position(), localsize)` doesn't change channel position after writes. Modification: Manually set position according to the written bytes. Result: Fixes #10449 . --- .../http/multipart/AbstractDiskHttpData.java | 17 ++++++++++++++--- .../http/multipart/DiskFileUploadTest.java | 19 +++++++++++-------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java index 3eac4ff99d..0844a613a4 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java @@ -156,7 +156,6 @@ public abstract class AbstractDiskHttpData extends AbstractHttpData { throw new IOException("Out of size: " + (size + localsize) + " > " + definedSize); } - int written = 0; if (file == null) { file = tempFile(); } @@ -164,9 +163,21 @@ public abstract class AbstractDiskHttpData extends AbstractHttpData { RandomAccessFile accessFile = new RandomAccessFile(file, "rw"); fileChannel = accessFile.getChannel(); } - buffer.getBytes(buffer.readerIndex(), fileChannel, fileChannel.position(), localsize); + int totalWritten = 0; + long position = fileChannel.position(); + int index = buffer.readerIndex(); + while (totalWritten < localsize) { + int written = buffer.getBytes(index, fileChannel, position, localsize - totalWritten); + if (written < 0) { + break; + } + totalWritten += written; + position += written; + index += written; + } + fileChannel.position(position); + buffer.readerIndex(index); size += localsize; - buffer.readerIndex(buffer.readerIndex() + written); } finally { // Release the buffer as it was retained before and we not need a reference to it at all // See https://github.com/netty/netty/issues/1516 diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java index 8fd665dbb4..49ad2d62f2 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java @@ -113,16 +113,19 @@ public class DiskFileUploadTest { public void testAddContents() throws Exception { DiskFileUpload f1 = new DiskFileUpload("file1", "file1", "application/json", null, null, 0); try { - String json = "{\"foo\":\"bar\"}"; - byte[] bytes = json.getBytes(CharsetUtil.UTF_8); - f1.addContent(Unpooled.wrappedBuffer(bytes), true); - assertEquals(json, f1.getString()); - assertArrayEquals(bytes, f1.get()); + byte[] jsonBytes = new byte[4096]; + PlatformDependent.threadLocalRandom().nextBytes(jsonBytes); + + f1.addContent(Unpooled.wrappedBuffer(jsonBytes, 0, 1024), false); + f1.addContent(Unpooled.wrappedBuffer(jsonBytes, 1024, jsonBytes.length - 1024), true); + assertArrayEquals(jsonBytes, f1.get()); + File file = f1.getFile(); - assertEquals((long) bytes.length, file.length()); + assertEquals(jsonBytes.length, file.length()); + FileInputStream fis = new FileInputStream(file); try { - byte[] buf = new byte[bytes.length]; + byte[] buf = new byte[jsonBytes.length]; int offset = 0; int read = 0; int len = buf.length; @@ -133,7 +136,7 @@ public class DiskFileUploadTest { break; } } - assertArrayEquals(bytes, buf); + assertArrayEquals(jsonBytes, buf); } finally { fis.close(); }