{@link ByteBuf#release()} ownership of {@code buffer} is transferred to this {@link HttpData}. * * @param buffer Must be not null. * @throws IOException If an IO error occurs when setting the content of this HttpData. @@ -57,6 +58,7 @@ public interface HttpData extends InterfaceHttpData, ByteBufHolder { /** * Add the content from the ChannelBuffer + *
{@link ByteBuf#release()} ownership of {@code buffer} is transferred to this {@link HttpData}. * * @param buffer * must be not null except if last is set to False diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MemoryAttribute.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MemoryAttribute.java index ba2696313a..150cc9b031 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MemoryAttribute.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MemoryAttribute.java @@ -80,7 +80,12 @@ public class MemoryAttribute extends AbstractMemoryHttpData implements Attribute @Override public void addContent(ByteBuf buffer, boolean last) throws IOException { int localsize = buffer.readableBytes(); - checkSize(size + localsize); + try { + checkSize(size + localsize); + } catch (IOException e) { + buffer.release(); + throw e; + } if (definedSize > 0 && definedSize < size + localsize) { definedSize = size + localsize; } diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedAttribute.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedAttribute.java index fbd23fd2fb..f148e4e256 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedAttribute.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedAttribute.java @@ -122,16 +122,21 @@ public class MixedAttribute implements Attribute { @Override public void addContent(ByteBuf buffer, boolean last) throws IOException { if (attribute instanceof MemoryAttribute) { - checkSize(attribute.length() + buffer.readableBytes()); - if (attribute.length() + buffer.readableBytes() > limitSize) { - DiskAttribute diskAttribute = new DiskAttribute(attribute - .getName(), attribute.definedLength(), baseDir, deleteOnExit); - diskAttribute.setMaxSize(maxSize); - if (((MemoryAttribute) attribute).getByteBuf() != null) { - diskAttribute.addContent(((MemoryAttribute) attribute) - .getByteBuf(), false); + try { + checkSize(attribute.length() + buffer.readableBytes()); + if (attribute.length() + buffer.readableBytes() > limitSize) { + DiskAttribute diskAttribute = new DiskAttribute(attribute + .getName(), attribute.definedLength(), baseDir, deleteOnExit); + diskAttribute.setMaxSize(maxSize); + if (((MemoryAttribute) attribute).getByteBuf() != null) { + diskAttribute.addContent(((MemoryAttribute) attribute) + .getByteBuf(), false); + } + attribute = diskAttribute; } - attribute = diskAttribute; + } catch (IOException e) { + buffer.release(); + throw e; } } attribute.addContent(buffer, last); @@ -199,7 +204,12 @@ public class MixedAttribute implements Attribute { @Override public void setContent(ByteBuf buffer) throws IOException { - checkSize(buffer.readableBytes()); + try { + checkSize(buffer.readableBytes()); + } catch (IOException e) { + buffer.release(); + throw e; + } if (buffer.readableBytes() > limitSize) { if (attribute instanceof MemoryAttribute) { // change to Disk diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedFileUpload.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedFileUpload.java index b5d5e673ce..547a363983 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedFileUpload.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedFileUpload.java @@ -83,22 +83,27 @@ public class MixedFileUpload implements FileUpload { public void addContent(ByteBuf buffer, boolean last) throws IOException { if (fileUpload instanceof MemoryFileUpload) { - checkSize(fileUpload.length() + buffer.readableBytes()); - if (fileUpload.length() + buffer.readableBytes() > limitSize) { - DiskFileUpload diskFileUpload = new DiskFileUpload(fileUpload - .getName(), fileUpload.getFilename(), fileUpload - .getContentType(), fileUpload - .getContentTransferEncoding(), fileUpload.getCharset(), - definedSize, baseDir, deleteOnExit); - diskFileUpload.setMaxSize(maxSize); - ByteBuf data = fileUpload.getByteBuf(); - if (data != null && data.isReadable()) { - diskFileUpload.addContent(data.retain(), false); - } - // release old upload - fileUpload.release(); + try { + checkSize(fileUpload.length() + buffer.readableBytes()); + if (fileUpload.length() + buffer.readableBytes() > limitSize) { + DiskFileUpload diskFileUpload = new DiskFileUpload(fileUpload + .getName(), fileUpload.getFilename(), fileUpload + .getContentType(), fileUpload + .getContentTransferEncoding(), fileUpload.getCharset(), + definedSize, baseDir, deleteOnExit); + diskFileUpload.setMaxSize(maxSize); + ByteBuf data = fileUpload.getByteBuf(); + if (data != null && data.isReadable()) { + diskFileUpload.addContent(data.retain(), false); + } + // release old upload + fileUpload.release(); - fileUpload = diskFileUpload; + fileUpload = diskFileUpload; + } + } catch (IOException e) { + buffer.release(); + throw e; } } fileUpload.addContent(buffer, last); @@ -181,7 +186,12 @@ public class MixedFileUpload implements FileUpload { @Override public void setContent(ByteBuf buffer) throws IOException { - checkSize(buffer.readableBytes()); + try { + checkSize(buffer.readableBytes()); + } catch (IOException e) { + buffer.release(); + throw e; + } if (buffer.readableBytes() > limitSize) { if (fileUpload instanceof MemoryFileUpload) { FileUpload memoryUpload = fileUpload; diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpDataTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpDataTest.java new file mode 100644 index 0000000000..17f90e70fa --- /dev/null +++ b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpDataTest.java @@ -0,0 +1,135 @@ +/* + * Copyright 2021 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: + * + * https://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.multipart; + +import io.netty.buffer.ByteBuf; +import io.netty.buffer.PooledByteBufAllocator; +import io.netty.util.CharsetUtil; +import org.assertj.core.api.ThrowableAssert; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import java.io.IOException; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.util.Random; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; + +class HttpDataTest { + private static final byte[] BYTES = new byte[64]; + + @Retention(RetentionPolicy.RUNTIME) + @Target(ElementType.METHOD) + @ParameterizedTest(name = "{displayName}({0})") + @MethodSource("data") + @interface ParameterizedHttpDataTest { + } + + static HttpData[] data() { + return new HttpData[]{ + new MemoryAttribute("test", 10), + new MemoryFileUpload("test", "", "text/plain", null, CharsetUtil.UTF_8, 10), + new MixedAttribute("test", 10, -1), + new MixedFileUpload("test", "", "text/plain", null, CharsetUtil.UTF_8, 10, -1), + new DiskAttribute("test", 10), + new DiskFileUpload("test", "", "text/plain", null, CharsetUtil.UTF_8, 10) + }; + } + + @BeforeAll + static void setUp() { + Random rndm = new Random(); + rndm.nextBytes(BYTES); + } + + @ParameterizedHttpDataTest + void testAddContentEmptyBuffer(HttpData httpData) throws IOException { + ByteBuf content = PooledByteBufAllocator.DEFAULT.buffer(); + httpData.addContent(content, false); + assertThat(content.refCnt()).isEqualTo(0); + } + + @Test + void testAddContentExceedsDefinedSizeDiskFileUpload() { + doTestAddContentExceedsSize( + new DiskFileUpload("test", "", "application/json", null, CharsetUtil.UTF_8, 10), + "Out of size: 64 > 10"); + } + + @Test + void testAddContentExceedsDefinedSizeMemoryFileUpload() { + doTestAddContentExceedsSize( + new MemoryFileUpload("test", "", "application/json", null, CharsetUtil.UTF_8, 10), + "Out of size: 64 > 10"); + } + + @ParameterizedHttpDataTest + void testAddContentExceedsMaxSize(final HttpData httpData) { + httpData.setMaxSize(10); + doTestAddContentExceedsSize(httpData, "Size exceed allowed maximum capacity"); + } + + @ParameterizedHttpDataTest + void testSetContentExceedsDefinedSize(final HttpData httpData) { + doTestSetContentExceedsSize(httpData, "Out of size: 64 > 10"); + } + + @ParameterizedHttpDataTest + void testSetContentExceedsMaxSize(final HttpData httpData) { + httpData.setMaxSize(10); + doTestSetContentExceedsSize(httpData, "Size exceed allowed maximum capacity"); + } + + private static void doTestAddContentExceedsSize(final HttpData httpData, String expectedMessage) { + final ByteBuf content = PooledByteBufAllocator.DEFAULT.buffer(); + content.writeBytes(BYTES); + + assertThatExceptionOfType(IOException.class) + .isThrownBy(new ThrowableAssert.ThrowingCallable() { + + @Override + public void call() throws Throwable { + httpData.addContent(content, false); + } + }) + .withMessage(expectedMessage); + + assertThat(content.refCnt()).isEqualTo(0); + } + + private static void doTestSetContentExceedsSize(final HttpData httpData, String expectedMessage) { + final ByteBuf content = PooledByteBufAllocator.DEFAULT.buffer(); + content.writeBytes(BYTES); + + assertThatExceptionOfType(IOException.class) + .isThrownBy(new ThrowableAssert.ThrowingCallable() { + + @Override + public void call() throws Throwable { + httpData.setContent(content); + } + }) + .withMessage(expectedMessage); + + assertThat(content.refCnt()).isEqualTo(0); + } +}