Skip to content

Commit

Permalink
Correctly clean up memory attributes and file uploads. Fixes netty#8640
Browse files Browse the repository at this point in the history
  • Loading branch information
@jameskleeh
jameskleeh committed Oct 15, 2019
1 parent 2e5dd28 commit c70254e
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 65 deletions.
104 changes: 39 additions & 65 deletions codec-http/src/main/java/io/netty/handler/codec/http/multipart/DefaultHttpDataFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,43 +130,33 @@ private List<HttpData> getList(HttpRequest request) {

@Override
public Attribute createAttribute(HttpRequest request, String name) {
Attribute attribute;
if (useDisk) {
Attribute attribute = new DiskAttribute(name, charset);
attribute.setMaxSize(maxSize);
List<HttpData> list = getList(request);
list.add(attribute);
return attribute;
attribute = new DiskAttribute(name, charset);
} else if (checkSize) {
attribute = new MixedAttribute(name, minSize, charset);
} else {
attribute = new MemoryAttribute(name);
}
if (checkSize) {
Attribute attribute = new MixedAttribute(name, minSize, charset);
attribute.setMaxSize(maxSize);
List<HttpData> list = getList(request);
list.add(attribute);
return attribute;
}
MemoryAttribute attribute = new MemoryAttribute(name);
attribute.setMaxSize(maxSize);
List<HttpData> list = getList(request);
list.add(attribute);
return attribute;
}

@Override
public Attribute createAttribute(HttpRequest request, String name, long definedSize) {
Attribute attribute;
if (useDisk) {
Attribute attribute = new DiskAttribute(name, definedSize, charset);
attribute.setMaxSize(maxSize);
List<HttpData> list = getList(request);
list.add(attribute);
return attribute;
}
if (checkSize) {
Attribute attribute = new MixedAttribute(name, definedSize, minSize, charset);
attribute.setMaxSize(maxSize);
List<HttpData> list = getList(request);
list.add(attribute);
return attribute;
attribute = new DiskAttribute(name, definedSize, charset);
} else if (checkSize) {
attribute = new MixedAttribute(name, definedSize, minSize, charset);
} else {
attribute = new MemoryAttribute(name, definedSize);
}
MemoryAttribute attribute = new MemoryAttribute(name, definedSize);
attribute.setMaxSize(maxSize);
List<HttpData> list = getList(request);
list.add(attribute);
return attribute;
}

Expand All @@ -183,65 +173,49 @@ private static void checkHttpDataSize(HttpData data) {

@Override
public Attribute createAttribute(HttpRequest request, String name, String value) {
Attribute attribute;
if (useDisk) {
Attribute attribute;
try {
attribute = new DiskAttribute(name, value, charset);
attribute.setMaxSize(maxSize);
} catch (IOException e) {
// revert to Mixed mode
attribute = new MixedAttribute(name, value, minSize, charset);
attribute.setMaxSize(maxSize);
}
checkHttpDataSize(attribute);
List<HttpData> list = getList(request);
list.add(attribute);
return attribute;
}
if (checkSize) {
Attribute attribute = new MixedAttribute(name, value, minSize, charset);
attribute.setMaxSize(maxSize);
checkHttpDataSize(attribute);
List<HttpData> list = getList(request);
list.add(attribute);
return attribute;
}
try {
MemoryAttribute attribute = new MemoryAttribute(name, value, charset);
attribute.setMaxSize(maxSize);
checkHttpDataSize(attribute);
return attribute;
} catch (IOException e) {
throw new IllegalArgumentException(e);
} else if (checkSize) {
attribute = new MixedAttribute(name, value, minSize, charset);
} else {
try {
attribute = new MemoryAttribute(name, value, charset);
} catch (IOException e) {
throw new IllegalArgumentException(e);
}
}
attribute.setMaxSize(maxSize);
checkHttpDataSize(attribute);
List<HttpData> list = getList(request);
list.add(attribute);
return attribute;
}

@Override
public FileUpload createFileUpload(HttpRequest request, String name, String filename,
String contentType, String contentTransferEncoding, Charset charset,
long size) {
FileUpload fileUpload;
if (useDisk) {
FileUpload fileUpload = new DiskFileUpload(name, filename, contentType,
fileUpload = new DiskFileUpload(name, filename, contentType,
contentTransferEncoding, charset, size);
fileUpload.setMaxSize(maxSize);
checkHttpDataSize(fileUpload);
List<HttpData> list = getList(request);
list.add(fileUpload);
return fileUpload;
}
if (checkSize) {
FileUpload fileUpload = new MixedFileUpload(name, filename, contentType,
} else if (checkSize) {
fileUpload = new MixedFileUpload(name, filename, contentType,
contentTransferEncoding, charset, size, minSize);
fileUpload.setMaxSize(maxSize);
checkHttpDataSize(fileUpload);
List<HttpData> list = getList(request);
list.add(fileUpload);
return fileUpload;
} else {
fileUpload = new MemoryFileUpload(name, filename, contentType,
contentTransferEncoding, charset, size);
}
MemoryFileUpload fileUpload = new MemoryFileUpload(name, filename, contentType,
contentTransferEncoding, charset, size);
fileUpload.setMaxSize(maxSize);
checkHttpDataSize(fileUpload);
List<HttpData> list = getList(request);
list.add(fileUpload);
return fileUpload;
}

Expand Down
38 changes: 38 additions & 0 deletions ...-http/src/test/java/io/netty/handler/codec/http/multipart/DefaultHttpDataFactoryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,4 +146,42 @@ public void removeHttpDataFromCleanShouldIdentifiesDataByTheirIdentities() throw
assertEquals(0, attribute2.refCnt());
assertEquals(0, file2.refCnt());
}

@Test
public void testMemoryUploadsAndAttributesAreCleanedUp() throws Exception {
DefaultHttpDataFactory factory = new DefaultHttpDataFactory(false);
Attribute attribute1 = factory.createAttribute(req1, "attribute1");
Attribute attribute2 = factory.createAttribute(req1, "attribute2", 5L);
Attribute attribute3 = factory.createAttribute(req1, "attribute3", "value");
attribute1.setContent(Unpooled.copiedBuffer("value", UTF_8));
attribute2.setContent(Unpooled.copiedBuffer("value", UTF_8));
FileUpload file1 = factory.createFileUpload(
req1, "file1", "file1.txt",
DEFAULT_TEXT_CONTENT_TYPE, IDENTITY.toString(), UTF_8, 123
);
file1.setContent(Unpooled.copiedBuffer("file1 content", UTF_8));

// Assert that they are not deleted
assertNotNull(attribute1.getByteBuf());
assertNotNull(attribute2.getByteBuf());
assertNotNull(attribute3.getByteBuf());
assertNotNull(file1.getByteBuf());
assertEquals(1, attribute1.refCnt());
assertEquals(1, attribute2.refCnt());
assertEquals(1, attribute3.refCnt());
assertEquals(1, file1.refCnt());

// Clean up by req1
factory.cleanRequestHttpData(req1);

// Assert that data belonging to req1 has been cleaned up
assertNull(attribute1.getByteBuf());
assertNull(attribute2.getByteBuf());
assertNull(attribute3.getByteBuf());
assertNull(file1.getByteBuf());
assertEquals(0, attribute1.refCnt());
assertEquals(0, attribute2.refCnt());
assertEquals(0, attribute3.refCnt());
assertEquals(0, file1.refCnt());
}
}

0 comments on commit c70254e

Please sign in to comment.