Multipart.foreach_entry() panics on unseen keys, is it expected?

[liufuyang]

Hi there

I am trying to use multipart in the project Tide with building an example like in this PR:
http-rs/tide#47

If you read the comments or take a look at the examples/multipart-form/main.rs file, you could see I tried to use foreach_entry method and make the lambda does nothing (only println) on a key that is not specified in the code.

It works with multipart payload with specified keys, however it panics if a key is not specified and giving error message like Unable to iterate multipart?: Custom { kind: InvalidData, error: TooLarge }

Do users are request to at least run explicitly entry.data.read_to_end() once in they function passed to foreach_entry() method? Or is this a buggy behavior that can be fixed?

Thank you :)

[abonander]

Yes, it's a bug. Every call to .read_entry() (used by foreach_entry()) should discard the previous field's data.

[liufuyang]

Is it easy to fix? Perhaps I can try help or you've got time to handle it? I am new to rust and would love some simple tasks to learn more about it :)

[abonander]

This unfortunately is going to require some debugging.

That error is being returned from here: https://github.com/abonander/multipart/blob/master/src/server/field.rs#L77

But the problem isn't with that code. The buffer isn't being emptied so it can't read the headers. The method responsible for emptying the buffer is BoundaryReader::consume_boundary() here: https://github.com/abonander/multipart/blob/master/src/server/boundary.rs#L114

But what we need is the raw request body that's triggering this behavior. If it doesn't contain any confidential data, do you mind posting it? Otherwise you should be able to replace the contents and strings in the headers with arbitrary bytes as long as the length is the same.

[liufuyang]

@abonander I see.

Basically I noticed this problem while developing an example code for the tide project:
http-rs/tide#47

You can see the error by:

git clone git@github.com:rust-net-web/tide.git
cd tide

# remove line 54-55 in examples/multipart-form/main.rs

cargo run --example multipart-form

curl -X POST http://localhost:7878/upload_file -H 'content-type: multipart/form-data' -F key1=v1, -F key2=v2,  -F key3=v3

Lines to remove to see the error:
https://github.com/rust-net-web/tide/blob/master/examples/multipart-form/main.rs#L54-L55

Let me know if this can help you see the issue on your side.

[abonander]

So what I need in this context is the request body that curl is sending. You can get that by adding the --trace flag to the command as shown here: https://curl.haxx.se/docs/manpage.html#--trace

Then attach the resulting file to a Github comment so I can inspect it.

[liufuyang]

Aha, I see, that one I can do easily, here is the output of --trace:

== Info:   Trying ::1...
== Info: TCP_NODELAY set
== Info: Connection failed
== Info: connect to ::1 port 7878 failed: Connection refused
== Info:   Trying 127.0.0.1...
== Info: TCP_NODELAY set
== Info: Connected to localhost (127.0.0.1) port 7878 (#0)
=> Send header, 219 bytes (0xdb)
0000: 50 4f 53 54 20 2f 75 70 6c 6f 61 64 5f 66 69 6c POST /upload_fil
0010: 65 20 48 54 54 50 2f 31 2e 31 0d 0a 48 6f 73 74 e HTTP/1.1..Host
0020: 3a 20 6c 6f 63 61 6c 68 6f 73 74 3a 37 38 37 38 : localhost:7878
0030: 0d 0a 55 73 65 72 2d 41 67 65 6e 74 3a 20 63 75 ..User-Agent: cu
0040: 72 6c 2f 37 2e 35 34 2e 30 0d 0a 41 63 63 65 70 rl/7.54.0..Accep
0050: 74 3a 20 2a 2f 2a 0d 0a 43 6f 6e 74 65 6e 74 2d t: */*..Content-
0060: 4c 65 6e 67 74 68 3a 20 33 33 33 0d 0a 45 78 70 Length: 333..Exp
0070: 65 63 74 3a 20 31 30 30 2d 63 6f 6e 74 69 6e 75 ect: 100-continu
0080: 65 0d 0a 63 6f 6e 74 65 6e 74 2d 74 79 70 65 3a e..content-type:
0090: 20 6d 75 6c 74 69 70 61 72 74 2f 66 6f 72 6d 2d  multipart/form-
00a0: 64 61 74 61 3b 20 62 6f 75 6e 64 61 72 79 3d 2d data; boundary=-
00b0: 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d ----------------
00c0: 2d 2d 2d 2d 2d 2d 2d 63 36 31 36 65 35 66 64 65 -------c616e5fde
00d0: 64 39 36 61 33 63 37 0d 0a 0d 0a                d96a3c7....
<= Recv header, 23 bytes (0x17)
0000: 48 54 54 50 2f 31 2e 31 20 31 30 30 20 43 6f 6e HTTP/1.1 100 Con
0010: 74 69 6e 75 65 0d 0a                            tinue..
=> Send data, 333 bytes (0x14d)
0000: 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d ----------------
0010: 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 63 36 31 36 65 35 ----------c616e5
0020: 66 64 65 64 39 36 61 33 63 37 0d 0a 43 6f 6e 74 fded96a3c7..Cont
0030: 65 6e 74 2d 44 69 73 70 6f 73 69 74 69 6f 6e 3a ent-Disposition:
0040: 20 66 6f 72 6d 2d 64 61 74 61 3b 20 6e 61 6d 65  form-data; name
0050: 3d 22 6b 65 79 31 22 0d 0a 0d 0a 76 31 2c 0d 0a ="key1"....v1,..
0060: 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d ----------------
0070: 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 63 36 31 36 65 35 ----------c616e5
0080: 66 64 65 64 39 36 61 33 63 37 0d 0a 43 6f 6e 74 fded96a3c7..Cont
0090: 65 6e 74 2d 44 69 73 70 6f 73 69 74 69 6f 6e 3a ent-Disposition:
00a0: 20 66 6f 72 6d 2d 64 61 74 61 3b 20 6e 61 6d 65  form-data; name
00b0: 3d 22 6b 65 79 32 22 0d 0a 0d 0a 76 32 2c 0d 0a ="key2"....v2,..
00c0: 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d ----------------
00d0: 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 63 36 31 36 65 35 ----------c616e5
00e0: 66 64 65 64 39 36 61 33 63 37 0d 0a 43 6f 6e 74 fded96a3c7..Cont
00f0: 65 6e 74 2d 44 69 73 70 6f 73 69 74 69 6f 6e 3a ent-Disposition:
0100: 20 66 6f 72 6d 2d 64 61 74 61 3b 20 6e 61 6d 65  form-data; name
0110: 3d 22 6b 65 79 33 22 0d 0a 0d 0a 76 33 0d 0a 2d ="key3"....v3..-
0120: 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d 2d ----------------
0130: 2d 2d 2d 2d 2d 2d 2d 2d 2d 63 36 31 36 65 35 66 ---------c616e5f
0140: 64 65 64 39 36 61 33 63 37 2d 2d 0d 0a          ded96a3c7--..
== Info: Empty reply from server
== Info: Connection #0 to host localhost left intact

[abonander]

I've added a reproduction test for this issue to the issue_114 branch along with some debug asserts to narrow down the location of the bug. It appears to be a problem with calculating consume_amt in this block: https://github.com/abonander/multipart/blob/issue_114/src/server/boundary.rs#L135-L175

The assert that triggers right now is the one in the match block at the end so it's somewhere between that and the other debug asserts. I think consume_amt is ending up too low (not consuming the entire boundary) but I have more debugging to do.

[liufuyang]

Interesting. Thanks for trying to solve it @abonander 😄

[abonander]

Fix published as 0.15.4.