making DefaultHttpDataFactory able to configure basedir and deleteonexit (#10146)

Motivation: 

currently (http) disk based attributes or uploads are globally configured in a single directory and can also only globally be deleted on exit or not. it does not fit well multiple cases, in particular the case you have multiple servers in the same JVM.

Modification: 
make it configurable per attribute/fileupload.

Result:

This PR duplicates Disk* constructor to add basedir and deleteonexit parameters and wires it in default http daa factory.
This commit is contained in:
Romain Manni-Bucau 2020-03-30 13:11:46 +02:00 committed by GitHub
parent aed5a74e09
commit 84ebf47515
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 168 additions and 26 deletions

View File

@ -19,6 +19,7 @@ import io.netty.handler.codec.http.DefaultHttpRequest;
import io.netty.handler.codec.http.HttpConstants;
import io.netty.handler.codec.http.HttpRequest;
import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
import java.util.ArrayList;
@ -59,6 +60,10 @@ public class DefaultHttpDataFactory implements HttpDataFactory {
private Charset charset = HttpConstants.DEFAULT_CHARSET;
private String baseDir;
private boolean deleteOnExit; // false is a good default cause true leaks
/**
* Keep all {@link HttpData}s until cleaning methods are called.
* We need to use {@link IdentityHashMap} because different requests may be equal.
@ -111,6 +116,25 @@ public class DefaultHttpDataFactory implements HttpDataFactory {
this.charset = charset;
}
/**
* Override global {@link DiskAttribute#baseDirectory} and {@link DiskFileUpload#baseDirectory} values.
*
* @param baseDir directory path where to store disk attributes and file uploads.
*/
public void setBaseDir(String baseDir) {
this.baseDir = baseDir;
}
/**
* Override global {@link DiskAttribute#deleteOnExitTemporaryFile} and
* {@link DiskFileUpload#deleteOnExitTemporaryFile} values.
*
* @param deleteOnExit true if temporary files should be deleted with the JVM, false otherwise.
*/
public void setDeleteOnExit(boolean deleteOnExit) {
this.deleteOnExit = deleteOnExit;
}
@Override
public void setMaxLimit(long maxSize) {
this.maxSize = maxSize;
@ -131,14 +155,14 @@ public class DefaultHttpDataFactory implements HttpDataFactory {
@Override
public Attribute createAttribute(HttpRequest request, String name) {
if (useDisk) {
Attribute attribute = new DiskAttribute(name, charset);
Attribute attribute = new DiskAttribute(name, charset, baseDir, deleteOnExit);
attribute.setMaxSize(maxSize);
List<HttpData> list = getList(request);
list.add(attribute);
return attribute;
}
if (checkSize) {
Attribute attribute = new MixedAttribute(name, minSize, charset);
Attribute attribute = new MixedAttribute(name, minSize, charset, baseDir, deleteOnExit);
attribute.setMaxSize(maxSize);
List<HttpData> list = getList(request);
list.add(attribute);
@ -152,14 +176,14 @@ public class DefaultHttpDataFactory implements HttpDataFactory {
@Override
public Attribute createAttribute(HttpRequest request, String name, long definedSize) {
if (useDisk) {
Attribute attribute = new DiskAttribute(name, definedSize, charset);
Attribute attribute = new DiskAttribute(name, definedSize, charset, baseDir, deleteOnExit);
attribute.setMaxSize(maxSize);
List<HttpData> list = getList(request);
list.add(attribute);
return attribute;
}
if (checkSize) {
Attribute attribute = new MixedAttribute(name, definedSize, minSize, charset);
Attribute attribute = new MixedAttribute(name, definedSize, minSize, charset, baseDir, deleteOnExit);
attribute.setMaxSize(maxSize);
List<HttpData> list = getList(request);
list.add(attribute);
@ -186,11 +210,11 @@ public class DefaultHttpDataFactory implements HttpDataFactory {
if (useDisk) {
Attribute attribute;
try {
attribute = new DiskAttribute(name, value, charset);
attribute = new DiskAttribute(name, value, charset, baseDir, deleteOnExit);
attribute.setMaxSize(maxSize);
} catch (IOException e) {
// revert to Mixed mode
attribute = new MixedAttribute(name, value, minSize, charset);
attribute = new MixedAttribute(name, value, minSize, charset, baseDir, deleteOnExit);
attribute.setMaxSize(maxSize);
}
checkHttpDataSize(attribute);
@ -199,7 +223,7 @@ public class DefaultHttpDataFactory implements HttpDataFactory {
return attribute;
}
if (checkSize) {
Attribute attribute = new MixedAttribute(name, value, minSize, charset);
Attribute attribute = new MixedAttribute(name, value, minSize, charset, baseDir, deleteOnExit);
attribute.setMaxSize(maxSize);
checkHttpDataSize(attribute);
List<HttpData> list = getList(request);
@ -222,7 +246,7 @@ public class DefaultHttpDataFactory implements HttpDataFactory {
long size) {
if (useDisk) {
FileUpload fileUpload = new DiskFileUpload(name, filename, contentType,
contentTransferEncoding, charset, size);
contentTransferEncoding, charset, size, baseDir, deleteOnExit);
fileUpload.setMaxSize(maxSize);
checkHttpDataSize(fileUpload);
List<HttpData> list = getList(request);
@ -231,7 +255,7 @@ public class DefaultHttpDataFactory implements HttpDataFactory {
}
if (checkSize) {
FileUpload fileUpload = new MixedFileUpload(name, filename, contentType,
contentTransferEncoding, charset, size, minSize);
contentTransferEncoding, charset, size, minSize, baseDir, deleteOnExit);
fileUpload.setMaxSize(maxSize);
checkHttpDataSize(fileUpload);
List<HttpData> list = getList(request);

View File

@ -37,6 +37,10 @@ public class DiskAttribute extends AbstractDiskHttpData implements Attribute {
public static final String postfix = ".att";
private String baseDir;
private boolean deleteOnExit;
/**
* Constructor used for huge Attribute
*/
@ -44,16 +48,40 @@ public class DiskAttribute extends AbstractDiskHttpData implements Attribute {
this(name, HttpConstants.DEFAULT_CHARSET);
}
public DiskAttribute(String name, String baseDir, boolean deleteOnExit) {
this(name, HttpConstants.DEFAULT_CHARSET);
this.baseDir = baseDir == null ? baseDirectory : baseDir;
this.deleteOnExit = deleteOnExit;
}
public DiskAttribute(String name, long definedSize) {
this(name, definedSize, HttpConstants.DEFAULT_CHARSET, baseDirectory, deleteOnExitTemporaryFile);
}
public DiskAttribute(String name, long definedSize, String baseDir, boolean deleteOnExit) {
this(name, definedSize, HttpConstants.DEFAULT_CHARSET);
this.baseDir = baseDir == null ? baseDirectory : baseDir;
this.deleteOnExit = deleteOnExit;
}
public DiskAttribute(String name, Charset charset) {
this(name, charset, baseDirectory, deleteOnExitTemporaryFile);
}
public DiskAttribute(String name, Charset charset, String baseDir, boolean deleteOnExit) {
super(name, charset, 0);
this.baseDir = baseDir == null ? baseDirectory : baseDir;
this.deleteOnExit = deleteOnExit;
}
public DiskAttribute(String name, long definedSize, Charset charset) {
this(name, definedSize, charset, baseDirectory, deleteOnExitTemporaryFile);
}
public DiskAttribute(String name, long definedSize, Charset charset, String baseDir, boolean deleteOnExit) {
super(name, charset, definedSize);
this.baseDir = baseDir == null ? baseDirectory : baseDir;
this.deleteOnExit = deleteOnExit;
}
public DiskAttribute(String name, String value) throws IOException {
@ -61,8 +89,15 @@ public class DiskAttribute extends AbstractDiskHttpData implements Attribute {
}
public DiskAttribute(String name, String value, Charset charset) throws IOException {
this(name, value, charset, baseDirectory, deleteOnExitTemporaryFile);
}
public DiskAttribute(String name, String value, Charset charset,
String baseDir, boolean deleteOnExit) throws IOException {
super(name, charset, 0); // Attribute have no default size
setValue(value);
this.baseDir = baseDir == null ? baseDirectory : baseDir;
this.deleteOnExit = deleteOnExit;
}
@Override
@ -136,12 +171,12 @@ public class DiskAttribute extends AbstractDiskHttpData implements Attribute {
@Override
protected boolean deleteOnExit() {
return deleteOnExitTemporaryFile;
return deleteOnExit;
}
@Override
protected String getBaseDirectory() {
return baseDirectory;
return baseDir;
}
@Override
@ -193,7 +228,7 @@ public class DiskAttribute extends AbstractDiskHttpData implements Attribute {
@Override
public Attribute replace(ByteBuf content) {
DiskAttribute attr = new DiskAttribute(getName());
DiskAttribute attr = new DiskAttribute(getName(), baseDir, deleteOnExit);
attr.setCharset(getCharset());
if (content != null) {
try {

View File

@ -37,6 +37,10 @@ public class DiskFileUpload extends AbstractDiskHttpData implements FileUpload {
public static final String postfix = ".tmp";
private final String baseDir;
private final boolean deleteOnExit;
private String filename;
private String contentType;
@ -44,11 +48,19 @@ public class DiskFileUpload extends AbstractDiskHttpData implements FileUpload {
private String contentTransferEncoding;
public DiskFileUpload(String name, String filename, String contentType,
String contentTransferEncoding, Charset charset, long size) {
String contentTransferEncoding, Charset charset, long size, String baseDir, boolean deleteOnExit) {
super(name, charset, size);
setFilename(filename);
setContentType(contentType);
setContentTransferEncoding(contentTransferEncoding);
this.baseDir = baseDir == null ? baseDirectory : baseDir;
this.deleteOnExit = deleteOnExit;
}
public DiskFileUpload(String name, String filename, String contentType,
String contentTransferEncoding, Charset charset, long size) {
this(name, filename, contentType, contentTransferEncoding,
charset, size, baseDirectory, deleteOnExitTemporaryFile);
}
@Override
@ -132,12 +144,12 @@ public class DiskFileUpload extends AbstractDiskHttpData implements FileUpload {
@Override
protected boolean deleteOnExit() {
return deleteOnExitTemporaryFile;
return deleteOnExit;
}
@Override
protected String getBaseDirectory() {
return baseDirectory;
return baseDir;
}
@Override
@ -190,7 +202,8 @@ public class DiskFileUpload extends AbstractDiskHttpData implements FileUpload {
@Override
public FileUpload replace(ByteBuf content) {
DiskFileUpload upload = new DiskFileUpload(
getName(), getFilename(), getContentType(), getContentTransferEncoding(), getCharset(), size);
getName(), getFilename(), getContentType(), getContentTransferEncoding(), getCharset(), size,
baseDir, deleteOnExit);
if (content != null) {
try {
upload.setContent(content);

View File

@ -27,6 +27,8 @@ import java.nio.charset.Charset;
* Mixed implementation using both in Memory and in File with a limit of size
*/
public class MixedAttribute implements Attribute {
private String baseDir;
private boolean deleteOnExit;
private Attribute attribute;
private final long limitSize;
@ -41,24 +43,45 @@ public class MixedAttribute implements Attribute {
}
public MixedAttribute(String name, long limitSize, Charset charset) {
this(name, limitSize, charset, DiskAttribute.baseDirectory, DiskAttribute.deleteOnExitTemporaryFile);
}
public MixedAttribute(String name, long limitSize, Charset charset, String baseDir, boolean deleteOnExit) {
this.limitSize = limitSize;
attribute = new MemoryAttribute(name, charset);
this.baseDir = baseDir;
this.deleteOnExit = deleteOnExit;
}
public MixedAttribute(String name, long definedSize, long limitSize, Charset charset) {
this(name, definedSize, limitSize, charset,
DiskAttribute.baseDirectory, DiskAttribute.deleteOnExitTemporaryFile);
}
public MixedAttribute(String name, long definedSize, long limitSize, Charset charset,
String baseDir, boolean deleteOnExit) {
this.limitSize = limitSize;
attribute = new MemoryAttribute(name, definedSize, charset);
this.baseDir = baseDir;
this.deleteOnExit = deleteOnExit;
}
public MixedAttribute(String name, String value, long limitSize) {
this(name, value, limitSize, HttpConstants.DEFAULT_CHARSET);
this(name, value, limitSize, HttpConstants.DEFAULT_CHARSET,
DiskAttribute.baseDirectory, DiskFileUpload.deleteOnExitTemporaryFile);
}
public MixedAttribute(String name, String value, long limitSize, Charset charset) {
this(name, value, limitSize, charset,
DiskAttribute.baseDirectory, DiskFileUpload.deleteOnExitTemporaryFile);
}
public MixedAttribute(String name, String value, long limitSize, Charset charset,
String baseDir, boolean deleteOnExit) {
this.limitSize = limitSize;
if (value.length() > this.limitSize) {
try {
attribute = new DiskAttribute(name, value, charset);
attribute = new DiskAttribute(name, value, charset, baseDir, deleteOnExit);
} catch (IOException e) {
// revert to Memory mode
try {
@ -74,6 +97,8 @@ public class MixedAttribute implements Attribute {
throw new IllegalArgumentException(e);
}
}
this.baseDir = baseDir;
this.deleteOnExit = deleteOnExit;
}
@Override
@ -100,7 +125,7 @@ public class MixedAttribute implements Attribute {
checkSize(attribute.length() + buffer.readableBytes());
if (attribute.length() + buffer.readableBytes() > limitSize) {
DiskAttribute diskAttribute = new DiskAttribute(attribute
.getName(), attribute.definedLength());
.getName(), attribute.definedLength(), baseDir, deleteOnExit);
diskAttribute.setMaxSize(maxSize);
if (((MemoryAttribute) attribute).getByteBuf() != null) {
diskAttribute.addContent(((MemoryAttribute) attribute)
@ -178,7 +203,7 @@ public class MixedAttribute implements Attribute {
if (buffer.readableBytes() > limitSize) {
if (attribute instanceof MemoryAttribute) {
// change to Disk
attribute = new DiskAttribute(attribute.getName(), attribute.definedLength());
attribute = new DiskAttribute(attribute.getName(), attribute.definedLength(), baseDir, deleteOnExit);
attribute.setMaxSize(maxSize);
}
}
@ -191,7 +216,7 @@ public class MixedAttribute implements Attribute {
if (file.length() > limitSize) {
if (attribute instanceof MemoryAttribute) {
// change to Disk
attribute = new DiskAttribute(attribute.getName(), attribute.definedLength());
attribute = new DiskAttribute(attribute.getName(), attribute.definedLength(), baseDir, deleteOnExit);
attribute.setMaxSize(maxSize);
}
}
@ -202,7 +227,7 @@ public class MixedAttribute implements Attribute {
public void setContent(InputStream inputStream) throws IOException {
if (attribute instanceof MemoryAttribute) {
// change to Disk even if we don't know the size
attribute = new DiskAttribute(attribute.getName(), attribute.definedLength());
attribute = new DiskAttribute(attribute.getName(), attribute.definedLength(), baseDir, deleteOnExit);
attribute.setMaxSize(maxSize);
}
attribute.setContent(inputStream);

View File

@ -27,6 +27,10 @@ import java.nio.charset.Charset;
*/
public class MixedFileUpload implements FileUpload {
private final String baseDir;
private final boolean deleteOnExit;
private FileUpload fileUpload;
private final long limitSize;
@ -37,6 +41,13 @@ public class MixedFileUpload implements FileUpload {
public MixedFileUpload(String name, String filename, String contentType,
String contentTransferEncoding, Charset charset, long size,
long limitSize) {
this(name, filename, contentType, contentTransferEncoding,
charset, size, limitSize, DiskFileUpload.baseDirectory, DiskFileUpload.deleteOnExitTemporaryFile);
}
public MixedFileUpload(String name, String filename, String contentType,
String contentTransferEncoding, Charset charset, long size,
long limitSize, String baseDir, boolean deleteOnExit) {
this.limitSize = limitSize;
if (size > this.limitSize) {
fileUpload = new DiskFileUpload(name, filename, contentType,
@ -46,6 +57,8 @@ public class MixedFileUpload implements FileUpload {
contentTransferEncoding, charset, size);
}
definedSize = size;
this.baseDir = baseDir;
this.deleteOnExit = deleteOnExit;
}
@Override
@ -76,7 +89,7 @@ public class MixedFileUpload implements FileUpload {
.getName(), fileUpload.getFilename(), fileUpload
.getContentType(), fileUpload
.getContentTransferEncoding(), fileUpload.getCharset(),
definedSize);
definedSize, baseDir, deleteOnExit);
diskFileUpload.setMaxSize(maxSize);
ByteBuf data = fileUpload.getByteBuf();
if (data != null && data.isReadable()) {
@ -177,7 +190,7 @@ public class MixedFileUpload implements FileUpload {
.getName(), memoryUpload.getFilename(), memoryUpload
.getContentType(), memoryUpload
.getContentTransferEncoding(), memoryUpload.getCharset(),
definedSize);
definedSize, baseDir, deleteOnExit);
fileUpload.setMaxSize(maxSize);
// release old upload
@ -199,7 +212,7 @@ public class MixedFileUpload implements FileUpload {
.getName(), memoryUpload.getFilename(), memoryUpload
.getContentType(), memoryUpload
.getContentTransferEncoding(), memoryUpload.getCharset(),
definedSize);
definedSize, baseDir, deleteOnExit);
fileUpload.setMaxSize(maxSize);
// release old upload
@ -219,7 +232,7 @@ public class MixedFileUpload implements FileUpload {
.getName(), fileUpload.getFilename(), fileUpload
.getContentType(), fileUpload
.getContentTransferEncoding(), fileUpload.getCharset(),
definedSize);
definedSize, baseDir, deleteOnExit);
fileUpload.setMaxSize(maxSize);
// release old upload

View File

@ -29,6 +29,7 @@ import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
import static io.netty.handler.codec.http.multipart.HttpPostBodyUtil.DEFAULT_TEXT_CONTENT_TYPE;
import static io.netty.util.CharsetUtil.UTF_8;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
@ -57,6 +58,21 @@ public class DefaultHttpDataFactoryTest {
factory.cleanAllHttpData();
}
@Test
public void customBaseDirAndDeleteOnExit() {
final DefaultHttpDataFactory defaultHttpDataFactory = new DefaultHttpDataFactory(true);
final String dir = "target/DefaultHttpDataFactoryTest/customBaseDirAndDeleteOnExit";
defaultHttpDataFactory.setBaseDir(dir);
defaultHttpDataFactory.setDeleteOnExit(true);
final Attribute attr = defaultHttpDataFactory.createAttribute(req1, "attribute1");
final FileUpload fu = defaultHttpDataFactory.createFileUpload(
req1, "attribute1", "f.txt", "text/plain", null, null, 0);
assertEquals(dir, DiskAttribute.class.cast(attr).getBaseDirectory());
assertEquals(dir, DiskFileUpload.class.cast(fu).getBaseDirectory());
assertTrue(DiskAttribute.class.cast(attr).deleteOnExit());
assertTrue(DiskFileUpload.class.cast(fu).deleteOnExit());
}
@Test
public void cleanRequestHttpDataShouldIdentifiesRequestsByTheirIdentities() throws Exception {
// Create some data belonging to req1 and req2

View File

@ -18,12 +18,28 @@ package io.netty.handler.codec.http.multipart;
import io.netty.buffer.Unpooled;
import org.junit.Test;
import java.io.File;
import java.io.IOException;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
public class DiskFileUploadTest {
@Test
public void testSpecificCustomBaseDir() throws IOException {
File baseDir = new File("target/DiskFileUploadTest/testSpecificCustomBaseDir");
baseDir.mkdirs(); // we don't need to clean it since it is in volatile files anyway
DiskFileUpload f =
new DiskFileUpload("d1", "d1", "application/json", null, null, 100,
baseDir.getAbsolutePath(), false);
f.setContent(Unpooled.EMPTY_BUFFER);
assertTrue(f.getFile().getAbsolutePath().startsWith(baseDir.getAbsolutePath()));
assertTrue(f.getFile().exists());
assertEquals(0, f.getFile().length());
f.delete();
}
@Test
public final void testDiskFileUploadEquals() {