[pve-devel] applied: [PATCH http-server] multipart upload: properly parse file parts without Content-Type

[Thomas Lamprecht]

Am 11/04/2023 um 14:19 schrieb Friedrich Weber:
> As reported in the forum, multipart requests are parsed incorrectly if
> the file part header contains *only* Content-Disposition, but no other
> fields (in particular, no Content-Type). As a result, uploaded files
> are mangled: In most cases, an additional carriage return and line
> feed (\r\n) is prepended to the file contents.
> 
> As an example, consider the following file part (with explicit \r\n
> for clarity):
> 
>   Content-Disposition: form-data; name=...; filename=...\r\n
>   Content-Type: application/x-iso9660-image\r\n
>   \r\n
>   file contents...
> 
> The current parsing code for file parts roughly works as follows:
> 
> 1) Consume the Content-Disposition field including the trailing \r\n
> 2) Consume and ignore everything up to and including the next \r\n\r\n
> 3) Read the file contents
> 
> This works fine in the example above. However, it has a bug in case
> Content-Disposition is the *only* header field:
> 
>   Content-Disposition: form-data; name=...; filename=...\r\n
>   \r\n
>   file contents...
> 
> Now, step 1 already consumes the first half of the \r\n\r\n sequence
> that marks the end of the part headers. As a result, step 3 starts
> reading the file at a wrong offset:
> 
> - If the remaining contents of the read buffer (currently sized 16KiB)
>   contain \r\n\r\n, step 2 consumes everything up to and including
>   this marker and step 3 starts reading file contents there. As a
>   result, the uploaded file is truncated at its beginning.
> - Otherwise, step 2 is a noop and step 3 considers the remaining
>   second half of the \r\n\r\n marker to be part of the file contents.
>   As a result, the uploaded file is prepended with an extra \r\n.
> 
> To fix this, modify step 1 to *not* consume the trailing \r\n. This
> keeps the \r\n\r\n marker intact, no matter whether additional header
> fields are present or not.
> 
> Fixes: 3e3faddb4a3792557351f1a6e9f2685e4713eb24
> Link: https://forum.proxmox.com/threads/125411/
> Signed-off-by: Friedrich Weber <f.weber at proxmox.com>
> ---
>  src/PVE/APIServer/AnyEvent.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks!