andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Ensure HttpData#addContent/setContent releases the buffer before throwing IOException (#11621)

Browse Source 
Motivation:
When the ByteBuf size exceeds the max limit/defined size, IOException is thrown.
HttpData#addContent/setContent should release the buffer in such cases otherwise memory leak will happen.

Modification:
- Release the provided ByteBuf before throwing IOException
- Add unit tests

Result:
Fixes #11618
This commit is contained in:
Violeta Georgieva 2021-09-02 23:59:44 +03:00 committed by GitHub
parent d58d8a1df8
commit 47bababfc7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 210 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpData.java
View File

@ -50,8 +50,14 @@ public abstract class AbstractMemoryHttpData extends AbstractHttpData {
public void setContent(ByteBuf buffer) throws IOException {
ObjectUtil.checkNotNull(buffer, "buffer");
long localsize = buffer.readableBytes();
try {
checkSize(localsize);
} catch (IOException e) {
buffer.release();
throw e;
}
if (definedSize > 0 && definedSize < localsize) {
buffer.release();
throw new IOException("Out of size: " + localsize + " > " +
definedSize);
}
@ -99,8 +105,14 @@ public abstract class AbstractMemoryHttpData extends AbstractHttpData {
throws IOException {
if (buffer != null) {
long localsize = buffer.readableBytes();
try {
checkSize(size + localsize);
} catch (IOException e) {
buffer.release();
throw e;
}
if (definedSize > 0 && definedSize < size + localsize) {
buffer.release();
throw new IOException("Out of size: " + (size + localsize) +
" > " + definedSize);
}

5
codec-http/src/main/java/io/netty/handler/codec/http/multipart/DiskAttribute.java
View File

@ -126,7 +126,12 @@ public class DiskAttribute extends AbstractDiskHttpData implements Attribute {
@Override
public void addContent(ByteBuf buffer, boolean last) throws IOException {
final long newDefinedSize = size + buffer.readableBytes();
try {
checkSize(newDefinedSize);
} catch (IOException e) {
buffer.release();
throw e;
}
if (definedSize > 0 && definedSize < newDefinedSize) {
definedSize = newDefinedSize;
}

4
codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpData.java
View File

@ -43,12 +43,13 @@ public interface HttpData extends InterfaceHttpData, ByteBufHolder {
/**
* Check if the new size is not reaching the max limit allowed.
* The limit is always computed in term of bytes.
* The limit is always computed in terms of bytes.
*/
void checkSize(long newSize) throws IOException;
/**
* Set the content from the ChannelBuffer (erase any previous data)
* <p>{@link ByteBuf#release()} ownership of {@code buffer} is transferred to this {@link HttpData}.
*
* @param buffer
* must be not null
@ -58,6 +59,7 @@ public interface HttpData extends InterfaceHttpData, ByteBufHolder {
/**
* Add the content from the ChannelBuffer
* <p>{@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

5
codec-http/src/main/java/io/netty/handler/codec/http/multipart/MemoryAttribute.java
View File

@ -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();
try {
checkSize(size + localsize);
} catch (IOException e) {
buffer.release();
throw e;
}
if (definedSize > 0 && definedSize < size + localsize) {
definedSize = size + localsize;
}

10
codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedAttribute.java
View File

@ -122,6 +122,7 @@ public class MixedAttribute implements Attribute {
@Override
public void addContent(ByteBuf buffer, boolean last) throws IOException {
if (attribute instanceof MemoryAttribute) {
try {
checkSize(attribute.length() + buffer.readableBytes());
if (attribute.length() + buffer.readableBytes() > limitSize) {
DiskAttribute diskAttribute = new DiskAttribute(attribute
@ -133,6 +134,10 @@ public class MixedAttribute implements Attribute {
}
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 {
try {
checkSize(buffer.readableBytes());
} catch (IOException e) {
buffer.release();
throw e;
}
if (buffer.readableBytes() > limitSize) {
if (attribute instanceof MemoryAttribute) {
// change to Disk

10
codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedFileUpload.java
View File

@ -83,6 +83,7 @@ public class MixedFileUpload implements FileUpload {
public void addContent(ByteBuf buffer, boolean last)
throws IOException {
if (fileUpload instanceof MemoryFileUpload) {
try {
checkSize(fileUpload.length() + buffer.readableBytes());
if (fileUpload.length() + buffer.readableBytes() > limitSize) {
DiskFileUpload diskFileUpload = new DiskFileUpload(fileUpload
@ -100,6 +101,10 @@ public class MixedFileUpload implements FileUpload {
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 {
try {
checkSize(buffer.readableBytes());
} catch (IOException e) {
buffer.release();
throw e;
}
if (buffer.readableBytes() > limitSize) {
if (fileUpload instanceof MemoryFileUpload) {
FileUpload memoryUpload = fileUpload;

135
codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpDataTest.java Normal file
View File

@ -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);
}
}