all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH v2 http-server 1/2] fix multipart upload: ignore additional headers
@ 2022-12-12 13:14 Matthias Heiserer
  2022-12-12 13:14 ` [pve-devel] [PATCH v2 http-server 2/2] multipart upload: don't require trailing newline Matthias Heiserer
  0 siblings, 1 reply; 2+ messages in thread
From: Matthias Heiserer @ 2022-12-12 13:14 UTC (permalink / raw)
  To: pve-devel

Reported in the forum: https://forum.proxmox.com/threads/image-upload-fails-after-upgrading-from-7-1-to-7-3.119051/#post-516517

When additional headers existed in the request body, the upload failed.
With this patch, all additional headers get ignored.

Example: The following upload would fail because no
headers were expected after Content-Disposition.

```
--EPIHyQJFC5ftgoXHMe8-Jc6E7FqA4oMb0QBfOTz
Content-Disposition: form-data; name="content"
Content-Type: text/plain; charset=ISO-8859-1

iso
```
would fail. These headers now also get ignored, as we don't use them.

Also, upload now works when the Content-Disposition header isn't the first, i.e.
```
--XVH95dt1-A3J8mWiLCmHCW4roSC7-gBntjATBy--
Content-Type: text/plain; charset=ISO-8859-1
Content-Disposition: form-data; name="content"
```

Fixed upload was tested using
* Curl
* GUI
* Apache HttpClient 5

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---

Changes from v1:
* wait for delimiter before handling data, as reported in forum
* squash ignore order of headers patch

 src/PVE/APIServer/AnyEvent.pm | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
index f397a8c..63b61a9 100644
--- a/src/PVE/APIServer/AnyEvent.pm
+++ b/src/PVE/APIServer/AnyEvent.pm
@@ -1201,11 +1201,19 @@ sub file_upload_multipart {
 	    $rstate->{phase} = 1;
 	}
 
+	my $remove_until_data = sub {
+	    my ($hdl) = @_;
+	    # remove any remaining multipart "headers" like Content-Type
+	    $hdl->{rbuf} =~ s/^.*?${newline_re}{2}//s;
+	};
+
 	my $extract_form_disposition = sub {
 	    my ($name) = @_;
-	    if ($hdl->{rbuf} =~ s/^${delim_re}Content-Disposition: (.*?); name="$name"(.*?)($delim_re)/$3/s) {
+	    if ($hdl->{rbuf} =~ s/^${delim_re}.*?Content-Disposition: (.*?); name="$name"(.*?${delim_re})/$2/s) {
 		assert_form_disposition($1);
-		$rstate->{params}->{$name} = trim($2);
+		$remove_until_data->($hdl);
+		$hdl->{rbuf} =~ s/^(.*?)(${delim_re})/$2/s;
+		$rstate->{params}->{$name} = trim($1);
 	    }
 	};
 
-- 
2.30.2





^ permalink raw reply	[flat|nested] 2+ messages in thread

* [pve-devel] [PATCH v2 http-server 2/2] multipart upload: don't require trailing newline
  2022-12-12 13:14 [pve-devel] [PATCH v2 http-server 1/2] fix multipart upload: ignore additional headers Matthias Heiserer
@ 2022-12-12 13:14 ` Matthias Heiserer
  0 siblings, 0 replies; 2+ messages in thread
From: Matthias Heiserer @ 2022-12-12 13:14 UTC (permalink / raw)
  To: pve-devel

RFC 152 mandates that data after the final boundary
must be ignored. Some software (e.g. postman) sends
their request without a trailing newline, which resulted
in failed uploads.

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---
 src/PVE/APIServer/AnyEvent.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
index 63b61a9..5d210d0 100644
--- a/src/PVE/APIServer/AnyEvent.pm
+++ b/src/PVE/APIServer/AnyEvent.pm
@@ -1193,7 +1193,7 @@ sub file_upload_multipart {
 
 	my $newline_re = qr/\015?\012/;
 	my $delim_re = qr/--\Q$boundary\E${newline_re}/;
-	my $close_delim_re = qr/--\Q$boundary\E--${newline_re}/;
+	my $close_delim_re = qr/--\Q$boundary\E--/;
 
 	# Phase 0 - preserve boundary, but remove everything before
 	if ($rstate->{phase} == 0 && $hdl->{rbuf} =~ s/^.*?($delim_re)/$1/s) {
-- 
2.30.2





^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-12-12 13:14 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-12 13:14 [pve-devel] [PATCH v2 http-server 1/2] fix multipart upload: ignore additional headers Matthias Heiserer
2022-12-12 13:14 ` [pve-devel] [PATCH v2 http-server 2/2] multipart upload: don't require trailing newline Matthias Heiserer

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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal