From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Friedrich Weber <f.weber@proxmox.com>
Subject: [pve-devel] applied: [PATCH http-server] multipart upload: properly parse file parts without Content-Type
Date: Tue, 11 Apr 2023 14:43:46 +0200 [thread overview]
Message-ID: <676c2cee-5e3c-7195-e848-3eae16e47f7b@proxmox.com> (raw)
In-Reply-To: <20230411121947.124433-1-f.weber@proxmox.com>
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@proxmox.com>
> ---
> src/PVE/APIServer/AnyEvent.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>
applied, thanks!
prev parent reply other threads:[~2023-04-11 12:44 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-11 12:19 [pve-devel] " Friedrich Weber
2023-04-11 12:43 ` Thomas Lamprecht [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=676c2cee-5e3c-7195-e848-3eae16e47f7b@proxmox.com \
--to=t.lamprecht@proxmox.com \
--cc=f.weber@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.