[PATCH http-server 0/2] apiserver: make multipart upload parsing more RFC compliant

[PATCH http-server 0/2] apiserver: make multipart upload parsing more RFC compliant

[Michal Fox]

two small fixes for the upload parser, both for things the RFCs allow
and real clients send: a quoted boundary (#6887) and unquoted
name/filename values, which .NET and PowerShell use (#7389).

tested by running curl, quoted-boundary and .NET style bodies through
file_upload_multipart() with different chunk sizes. curl behaves as
before, the other two now parse and the file comes out intact

Michal Fox (2):
  fix #6887: apiserver: accept quoted multipart boundary
  fix #7389: apiserver: accept unquoted multipart parameter values

 src/PVE/APIServer/AnyEvent.pm | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

-- 
2.43.0

[PATCH http-server 1/2] fix #6887: apiserver: accept quoted multipart boundary

[Michal Fox]

RFC 2046 allows the boundary parameter of a multipart Content-Type to
be a quoted-string, and even requires it if the boundary contains
characters like a colon. RFC 7578 notes that quoting it is often
necessary for multipart/form-data.

parse_content_type() took everything after the equal sign verbatim, so
for 'boundary="foo"' the quotes became part of the boundary. The
delimiter lines of the body, correctly written as '--foo', never
matched and the upload failed.

Strip the quotes of a quoted boundary.

Signed-off-by: Michal Fox <me@dualfroz.com>
---
 src/PVE/APIServer/AnyEvent.pm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
index 915d678..bb49a8f 100644
--- a/src/PVE/APIServer/AnyEvent.pm
+++ b/src/PVE/APIServer/AnyEvent.pm
@@ -1382,8 +1382,9 @@ sub parse_content_type {
     my ($ct, @params) = split(/\s*[;,]\s*/o, $ctype);
 
     foreach my $v (@params) {
-        if ($v =~ m/^\s*boundary\s*=\s*(\S+?)\s*$/o) {
-            return wantarray ? ($ct, $1) : $ct;
+        # the boundary may be given as quoted-string, see RFC 2046, section 5.1.1
+        if ($v =~ m/^\s*boundary\s*=\s*(?:"([^"]+)"|(\S+?))\s*$/o) {
+            return wantarray ? ($ct, $1 // $2) : $ct;
         }
     }
 
-- 
2.43.0

[PATCH http-server 2/2] fix #7389: apiserver: accept unquoted multipart parameter values

[Michal Fox]

In the Content-Disposition header of a multipart/form-data part, the
values of the name and filename parameters can be either a token or a
quoted-string, see RFC 7578, section 4.2, and RFC 6266. The upload
parser only matched the quoted form.

Browsers, curl and Python always quote these values, but the .NET
HttpClient, and with it PowerShell's Invoke-RestMethod -Form, uses the
token form for simple ASCII values, e.g.:

  Content-Disposition: form-data; name=filename; filename=foo.iso;
    filename*=utf-8''foo.iso

For such requests, neither the 'content' parameter nor the file part
got recognized and the upload failed.

Accept both forms. For the named parameters, make sure the unquoted
name is not just a prefix of a longer one, like 'checksum' for
'checksum-algorithm'.

Signed-off-by: Michal Fox <me@dualfroz.com>
---
 src/PVE/APIServer/AnyEvent.pm | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
index bb49a8f..f4ee9f2 100644
--- a/src/PVE/APIServer/AnyEvent.pm
+++ b/src/PVE/APIServer/AnyEvent.pm
@@ -1303,8 +1303,9 @@ sub file_upload_multipart {
 
         my $extract_form_disposition = sub {
             my ($name) = @_;
+            # parameter values can be a token or a quoted-string, see RFC 7578, section 4.2
             if ($hdl->{rbuf} =~
-                s/^${delim_re}.*?Content-Disposition: (.*?); name="$name"(.*?${delim_re})/$2/s
+                s/^${delim_re}.*?Content-Disposition: (.*?); name=(?:"\Q$name\E"|\Q$name\E(?=[;\s]))(.*?${delim_re})/$2/s
             ) {
                 assert_form_disposition($1);
                 $remove_until_data->($hdl);
@@ -1319,13 +1320,14 @@ sub file_upload_multipart {
             $extract_form_disposition->('checksum');
 
             if ($hdl->{rbuf} =~
-                s/^${delim_re}Content-Disposition: (.*?); name="(.*?)"; filename="([^"]+)"//s
+                s/^${delim_re}Content-Disposition: (.*?); name=(?:"(.*?)"|([^\s;"]+)); filename=(?:"([^"]+)"|([^\s;"]+))//s
             ) {
-                assert_form_disposition($1);
-                die "wrong field name '$2' for file upload, expected 'filename'"
-                    if $2 ne "filename";
+                my ($disposition, $field, $filename) = ($1, $2 // $3, $4 // $5);
+                assert_form_disposition($disposition);
+                die "wrong field name '$field' for file upload, expected 'filename'"
+                    if $field ne "filename";
                 $rstate->{phase} = 2;
-                $rstate->{params}->{filename} = trim($3);
+                $rstate->{params}->{filename} = trim($filename);
                 $remove_until_data->($hdl); # any remaining multipart "headers" like Content-Type
             }
         }
-- 
2.43.0