all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH http-server 1/2] fix multipart upload: ignore additional headers
@ 2022-12-06 15:16 Matthias Heiserer
  2022-12-06 15:16 ` [pve-devel] [PATCH http-server 2/2] fix multipart upload: header order Matthias Heiserer
  2022-12-07 10:52 ` [pve-devel] [PATCH http-server 1/2] fix multipart upload: ignore additional headers Matthias Heiserer
  0 siblings, 2 replies; 3+ messages in thread
From: Matthias Heiserer @ 2022-12-06 15:16 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
```

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

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

diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
index f397a8c..db89ff2 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"(.*)$/$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);
 	    }
 	};
 
@@ -1224,6 +1232,7 @@ sub file_upload_multipart {
 		die "wrong field name '$2' for file upload, expected 'filename'" if $2 ne "filename";
 		$rstate->{phase} = 2;
 		$rstate->{params}->{filename} = trim($3);
+		$remove_until_data->($hdl);
 	    }
 	}
 
-- 
2.30.2





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

* [pve-devel] [PATCH http-server 2/2] fix multipart upload: header order
  2022-12-06 15:16 [pve-devel] [PATCH http-server 1/2] fix multipart upload: ignore additional headers Matthias Heiserer
@ 2022-12-06 15:16 ` Matthias Heiserer
  2022-12-07 10:52 ` [pve-devel] [PATCH http-server 1/2] fix multipart upload: ignore additional headers Matthias Heiserer
  1 sibling, 0 replies; 3+ messages in thread
From: Matthias Heiserer @ 2022-12-06 15:16 UTC (permalink / raw)
  To: pve-devel

When the Content-Disposition header wasn't the first, i.e. upload of
```
--XVH95dt1-A3J8mWiLCmHCW4roSC7-gBntjATBy--
Content-Type: text/plain; charset=ISO-8859-1
Content-Disposition: form-data; name="content"
```
would fail. These headers now also get ignored, as we don't use them.

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

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 db89ff2..c26a439 100644
--- a/src/PVE/APIServer/AnyEvent.pm
+++ b/src/PVE/APIServer/AnyEvent.pm
@@ -1209,7 +1209,7 @@ sub file_upload_multipart {
 
 	my $extract_form_disposition = sub {
 	    my ($name) = @_;
-	    if ($hdl->{rbuf} =~ s/^${delim_re}Content-Disposition: (.*?); name="$name"(.*)$/$2/s) {
+	    if ($hdl->{rbuf} =~ s/^${delim_re}.*?Content-Disposition: (.*?); name="$name"(.*)$/$2/s) {
 		assert_form_disposition($1);
 		$remove_until_data->($hdl);
 		$hdl->{rbuf} =~ s/^(.*?)(${delim_re})/$2/s;
-- 
2.30.2





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

* Re: [pve-devel] [PATCH http-server 1/2] fix multipart upload: ignore additional headers
  2022-12-06 15:16 [pve-devel] [PATCH http-server 1/2] fix multipart upload: ignore additional headers Matthias Heiserer
  2022-12-06 15:16 ` [pve-devel] [PATCH http-server 2/2] fix multipart upload: header order Matthias Heiserer
@ 2022-12-07 10:52 ` Matthias Heiserer
  1 sibling, 0 replies; 3+ messages in thread
From: Matthias Heiserer @ 2022-12-07 10:52 UTC (permalink / raw)
  To: pve-devel

I was advised in the forum that there's a bug in this patch - I'll send 
a v2.




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

end of thread, other threads:[~2022-12-07 10:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-06 15:16 [pve-devel] [PATCH http-server 1/2] fix multipart upload: ignore additional headers Matthias Heiserer
2022-12-06 15:16 ` [pve-devel] [PATCH http-server 2/2] fix multipart upload: header order Matthias Heiserer
2022-12-07 10:52 ` [pve-devel] [PATCH http-server 1/2] fix multipart upload: ignore additional headers 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