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

Move up the size check in AbstractDiskHttpData.setContent. (#10222)

Browse Source 
Motivation:

`AbstractHttpData.checkSize` may throw an IOException if we set the max size limit via `AbstractHttpData.setMaxSize`. However, if this exception happens, the `AbstractDiskHttpData.file` and the `AbstractHttpData.size` are still be modified. In other words, it may break the failure atomicity here.

Modification:

Just move up the size check.

Result:

Keep the failure atomicity even if `AbstractHttpData.checkSize` fails.
This commit is contained in:
feijermu 2020-04-28 15:34:33 +08:00 committed by GitHub
parent 4f72cdf233
commit 9751bb3ebc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 42 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -198,12 +198,13 @@ public abstract class AbstractDiskHttpData extends AbstractHttpData {
@Override
public void setContent(File file) throws IOException {
long size = file.length();
checkSize(size);
this.size = size;
if (this.file != null) {
delete();
}
this.file = file;
size = file.length();
checkSize(size);
isRenamed = true;
setCompleted();
}

39
codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java
View File

@ -19,13 +19,16 @@ import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufInputStream;
import io.netty.buffer.Unpooled;
import io.netty.util.CharsetUtil;
import io.netty.util.internal.PlatformDependent;
import org.junit.Test;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.util.UUID;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
@ -33,6 +36,7 @@ import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
public class DiskFileUploadTest {
@Test
@ -215,4 +219,39 @@ public class DiskFileUploadTest {
assertFalse(tmpFile.exists());
}
}
@Test
public void setSetContentFromFileExceptionally() throws Exception {
final long maxSize = 4;
DiskFileUpload f1 = new DiskFileUpload("file5", "file5", "application/json", null, null, 0);
f1.setMaxSize(maxSize);
try {
f1.setContent(Unpooled.wrappedBuffer(new byte[(int) maxSize]));
File originalFile = f1.getFile();
assertNotNull(originalFile);
assertEquals(maxSize, originalFile.length());
assertEquals(maxSize, f1.length());
byte[] bytes = new byte[8];
PlatformDependent.threadLocalRandom().nextBytes(bytes);
File tmpFile = File.createTempFile(UUID.randomUUID().toString(), ".tmp");
tmpFile.deleteOnExit();
FileOutputStream fos = new FileOutputStream(tmpFile);
try {
fos.write(bytes);
fos.flush();
} finally {
fos.close();
}
try {
f1.setContent(tmpFile);
fail("should not reach here!");
} catch (IOException e) {
assertNotNull(f1.getFile());
assertEquals(originalFile, f1.getFile());
assertEquals(maxSize, f1.length());
}
} finally {
f1.delete();
}
}
}