From 9751bb3ebc5593dfd369c86800d0cfb1319cda97 Mon Sep 17 00:00:00 2001
From: feijermu <seedeed@qq.com>
Date: Tue, 28 Apr 2020 15:34:33 +0800
Subject: [PATCH] Move up the size check in AbstractDiskHttpData.setContent.
 (#10222)

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.
---
 .../http/multipart/AbstractDiskHttpData.java  |  5 ++-
 .../http/multipart/DiskFileUploadTest.java    | 39 +++++++++++++++++++
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java
index ce578efca7..e0a16b352a 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java
@@ -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();
     }
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java
index 410877b70e..7a9429ba30 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java
@@ -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();
+        }
+    }
 }