[pve-devel] [PATCH v2 http-server 0/2] fix #4344: ignore unused multipart headers

[pve-devel] [PATCH v2 http-server 0/2] fix #4344: ignore unused multipart headers

[John Hollowell]

This fixes an issue where an upload request without a Content-Type in
the file's multipart part would prevent the upload and throw
missleading errors. This patch removes the requirement and ignores
all multipart headers once the needed information has been extracted.

I have tested these changes against a 7.2-11 server and both a previously
broken upload method (without the Content-Type) and using the webUI in
Chrome (which includes a Content-Type) correctly uploads the file.

Changes since v1:
* remove `xx` and escaping of spaces from regex

John Hollowell (2):
  fix #4344: http-server: ignore unused multipart headers
  Remove whitespace ignore from regex

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

--
2.30.2

[pve-devel] [PATCH v2 http-server 1/2] fix #4344: http-server: ignore unused multipart headers

[John Hollowell]

Signed-off-by: John Hollowell <jhollowe@johnhollowell.com>
---
 src/PVE/APIServer/AnyEvent.pm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
index f397a8c..d958642 100644
--- a/src/PVE/APIServer/AnyEvent.pm
+++ b/src/PVE/APIServer/AnyEvent.pm
@@ -1215,15 +1215,15 @@ sub file_upload_multipart {
 	    $extract_form_disposition->('checksum');

 	    if ($hdl->{rbuf} =~
-		s/^${delim_re}
-		Content-Disposition:\ (.*?);\ name="(.*?)";\ filename="([^"]+)"${newline_re}
-		Content-Type:\ \S*\s+
-		//sxx
+		s/^${delim_re}Content-Disposition:\ (.*?);\ name="(.*?)";\ filename="([^"]+)"//sxx
 	    ) {
 		assert_form_disposition($1);
 		die "wrong field name '$2' for file upload, expected 'filename'" if $2 ne "filename";
 		$rstate->{phase} = 2;
 		$rstate->{params}->{filename} = trim($3);
+
+		# remove any remaining multipart "headers" like Content-Type
+		$hdl->{rbuf} =~ s/^.*?${newline_re}{2}//s
 	    }
 	}

--
2.30.2

[pve-devel] [PATCH v2 http-server 2/2] Remove whitespace ignore from regex

[John Hollowell]

Signed-off-by: John Hollowell <jhollowe@johnhollowell.com>
---
 src/PVE/APIServer/AnyEvent.pm | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
index d958642..ed1321d 100644
--- a/src/PVE/APIServer/AnyEvent.pm
+++ b/src/PVE/APIServer/AnyEvent.pm
@@ -1214,9 +1214,7 @@ sub file_upload_multipart {
 	    $extract_form_disposition->('checksum-algorithm');
 	    $extract_form_disposition->('checksum');

-	    if ($hdl->{rbuf} =~
-		s/^${delim_re}Content-Disposition:\ (.*?);\ name="(.*?)";\ filename="([^"]+)"//sxx
-	    ) {
+	    if ($hdl->{rbuf} =~ s/^${delim_re}Content-Disposition: (.*?); name="(.*?)"; filename="([^"]+)"//s) {
 		assert_form_disposition($1);
 		die "wrong field name '$2' for file upload, expected 'filename'" if $2 ne "filename";
 		$rstate->{phase} = 2;
--
2.30.2

Re: [pve-devel] [PATCH v2 http-server 0/2] fix #4344: ignore unused multipart headers

[Matthias Heiserer]

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

On 18.11.2022 02:39, John Hollowell wrote:
> This fixes an issue where an upload request without a Content-Type in
> the file's multipart part would prevent the upload and throw
> missleading errors. This patch removes the requirement and ignores
> all multipart headers once the needed information has been extracted.
> 
> I have tested these changes against a 7.2-11 server and both a previously
> broken upload method (without the Content-Type) and using the webUI in
> Chrome (which includes a Content-Type) correctly uploads the file.
> 
> Changes since v1:
> * remove `xx` and escaping of spaces from regex
> 
> John Hollowell (2):
>    fix #4344: http-server: ignore unused multipart headers
>    Remove whitespace ignore from regex
> 
>   src/PVE/APIServer/AnyEvent.pm | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)
> 
> --
> 2.30.2
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 

[pve-devel] applied-series: [PATCH v2 http-server 0/2] fix #4344: ignore unused multipart headers

[Thomas Lamprecht]

Am 18/11/2022 um 02:39 schrieb John Hollowell:
> This fixes an issue where an upload request without a Content-Type in
> the file's multipart part would prevent the upload and throw
> missleading errors. This patch removes the requirement and ignores
> all multipart headers once the needed information has been extracted.
> 
> I have tested these changes against a 7.2-11 server and both a previously
> broken upload method (without the Content-Type) and using the webUI in
> Chrome (which includes a Content-Type) correctly uploads the file.
> 
> Changes since v1:
> * remove `xx` and escaping of spaces from regex
> 
> John Hollowell (2):
>   fix #4344: http-server: ignore unused multipart headers
>   Remove whitespace ignore from regex
> 
>  src/PVE/APIServer/AnyEvent.pm | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> --
> 2.30.2
> 

this went a bit under our radar, applied series now with some small merge conflict
resolved, thanks!