Hi, I've spent a large part of this week battling with an obscure file upload bug on StackHub. A customer was receiving validation errors when submitting a form, but ONLY with one particular .pod file!?

I was able to finally pin-point it to WebUtil.parseMultiPart() (used in the multi-part form upload process) where the InStream for the File part was reading too much data and also consumed the next part!

The following failing test creates a multi-part form with x 2 parts. The final verify just checks that x 2 parts were read, but it fails because the 2nd part vanishes.

using web

class TestMultipartInStream : Test {

    Void testMultipart() {
        bound    := "XXXXX"
        buf      := Buf()
        out      := buf.out

        // GIVEN I have a MultiPart form with x2 parts
        out.print("--").print(bound).print("\r\n")
        out.print("name: foo1\r\n")    // Part1 = foo1
        out.print("\r\n")
        1023.times { out.write(0) }    // 1023 is the MAGIC bad number!
        out.print("\r\n")

        out.print("--").print(bound).print("\r\n")
        out.print("name: foo2\r\n")    // Part2 = foo2
        out.print("\r\n")
        out.print("Data-Data-Data")
        out.print("\r\n")

        out.print("--").print(bound).print("--\r\n")

        // WHEN I parse it with WebUtil
        names    := Str[,]
        WebUtil.parseMultiPart(buf.flip.in, bound) |headers, InStream in| {
            names.add(headers["name"])
            in.readAllBuf    // drain the part stream
        }
        
        // THEN I should have BOTH field values!
        verifyEq(names, ["foo1", "foo2"])

        // But...
        // Test failed: [foo1] != [foo1, foo2] !!!
    }
}

And here's a patch for a fix. There was a very specific edge case whereby should the final read of a part be exactly 1023 bytes - then the boundary prefix of \r\n gets missed resulting in the InStream also gobbling up the next part.

diff --git a/src/web/fan/WebUtil.fan b/src/web/fan/WebUtil.fan
index 9965a08..74c5649 100644
--- a/src/web/fan/WebUtil.fan
+++ b/src/web/fan/WebUtil.fan
@@ -750,6 +750,14 @@ internal class MultiPartInStream : InStream
       if (c == '\n') break
     }
 
+    // test boundary condition - make sure \r is available to read on next iteration
+    if (curLine.size == 1024 && curLine[-1] == '\r') {
+      in.unread(curLine[-1])
+      curLine.size = curLine.size - 1
+      curLine.seek(0)
+      return true
+    }
+
     // if not a property \r\n newline then keep chugging
     if (curLine.size < 2 || curLine[-2] != '\r') { curLine.seek(0); return true }

Have fun!

Steve.

Thanks Steve. I reworked your fix to move that check into the read loop itself and added that test case and pushed a changeset.