Handle quotes problems in Content-Type and Content-Disposition

fdintino · Apr 12, 2020 · 1ac57ac · 1ac57ac
1 parent aa42509
commit 1ac57ac
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 20 deletions.
diff --git a/ngx_http_upload_module.c b/ngx_http_upload_module.c
@@ -55,8 +55,8 @@ typedef ngx_md5_t MD5_CTX;
 #define X_SESSION_ID_STRING                     "X-Session-ID:"
 #define FORM_DATA_STRING                        "form-data"
 #define ATTACHMENT_STRING                       "attachment"
-#define FILENAME_STRING                         "filename=\""
-#define FIELDNAME_STRING                        "name=\""
+#define FILENAME_STRING                         "filename="
+#define FIELDNAME_STRING                        "name="
 #define BYTES_UNIT_STRING                       "bytes "
 
 #define NGX_UPLOAD_MALFORMED    -11
@@ -3521,16 +3521,14 @@ static ngx_int_t upload_parse_content_disposition(ngx_http_upload_ctx_t *upload_
 
     if(filename_start != 0) {
 
-        filename_start += sizeof(FILENAME_STRING)-1;
+        filename_start += sizeof(FILENAME_STRING) - 1;
 
-        filename_end = filename_start + strcspn(filename_start, "\"");
-
-        if(*filename_end != '\"') {
-            ngx_log_debug0(NGX_LOG_DEBUG_CORE, upload_ctx->log, 0,
-                           "malformed filename in part header");
-            return NGX_UPLOAD_MALFORMED;
+        if (*filename_start == '\"') {
+            filename_start++;
         }
 
+        filename_end = filename_start + strcspn(filename_start, "\";");
+
         /*
          * IE sends full path, strip path from filename 
          * Also strip all UNIX path references
@@ -3557,16 +3555,14 @@ static ngx_int_t upload_parse_content_disposition(ngx_http_upload_ctx_t *upload_
 //    }while((fieldname_start != 0) && (fieldname_start + sizeof(FIELDNAME_STRING) - 1 == filename_start));
 
     if(fieldname_start != 0) {
-        fieldname_start += sizeof(FIELDNAME_STRING)-1;
+        fieldname_start += sizeof(FIELDNAME_STRING) - 1;
 
-        if(fieldname_start != filename_start) {
-            fieldname_end = fieldname_start + strcspn(fieldname_start, "\"");
+        if (*fieldname_start == '\"') {
+            fieldname_start++;
+        }
 
-            if(*fieldname_end != '\"') {
-                ngx_log_error(NGX_LOG_ERR, upload_ctx->log, 0,
-                               "malformed fieldname in part header");
-                return NGX_UPLOAD_MALFORMED;
-            }
+        if(fieldname_start != filename_start) {
+            fieldname_end = fieldname_start + strcspn(fieldname_start, "\";");
 
             upload_ctx->field_name.len = fieldname_end - fieldname_start;
             upload_ctx->field_name.data = ngx_pcalloc(upload_ctx->request->pool, upload_ctx->field_name.len + 1);
@@ -3941,6 +3937,12 @@ static ngx_int_t upload_parse_request_headers(ngx_http_upload_ctx_t *upload_ctx,
         boundary_start_ptr += sizeof(BOUNDARY_STRING) - 1;
         boundary_end_ptr = boundary_start_ptr + strcspn((char*)boundary_start_ptr, " ;\n\r");
 
+        // Handle quoted boundaries
+        if ((boundary_end_ptr - boundary_start_ptr) >= 2 && boundary_start_ptr[0] == '"' && *(boundary_end_ptr - 1) == '"') {
+          boundary_start_ptr++;
+          boundary_end_ptr--;
+        }
+
         if(boundary_end_ptr == boundary_start_ptr) {
             ngx_log_debug0(NGX_LOG_DEBUG_CORE, upload_ctx->log, 0,
                            "boundary is empty");

diff --git a/t/lib/Test/Nginx/UploadModule/TestServer.pm b/t/lib/Test/Nginx/UploadModule/TestServer.pm
@@ -111,7 +111,7 @@ sub process_body {
     my $content_type = $req->header('Content-Type');
     my $data = {};
     if ($content_type) {
-        if ($content_type =~ /multipart\/form\-data; boundary=(.+?)$/i) {
+        if ($content_type =~ /multipart\/form\-data; boundary="?(.+?)"?$/i) {
             my $boundary = quotemeta($1);
             return $self->process_multipart($content, $boundary);
         }
@@ -241,4 +241,4 @@ if (!caller) {
     $server->run();
 }
 
-1;
+1;
diff --git a/t/upload.t b/t/upload.t
@@ -5,7 +5,7 @@ use File::Basename qw(dirname);
 
 use lib dirname(__FILE__) . "/lib";
 
-use Test::Nginx::Socket tests => 27;
+use Test::Nginx::Socket tests => 33;
 use Test::Nginx::UploadModule;
 
 
@@ -179,3 +179,53 @@ upload_tmp_path = ${ENV{TEST_NGINX_UPLOAD_PATH}}/store/5/0000000005
 }]
 --- upload_file_like eval
 qr/^(??{'a' x 131072 . 'b' x 131072})$/
+
+=== TEST 6: multipart upload
+--- config eval: $::config
+--- more_headers
+Content-Type: multipart/form-data; boundary=BOUNDARY
+--- request eval
+"POST /upload/
+--BOUNDARY
+Content-Disposition: form-data; name=\"file\"; filename=\"test.txt\"\r
+Content-Type: text/plain\r
+\r
+test\r
+--BOUNDARY--\r
+"
+--- error_code: 200
+--- response_body eval
+qq{upload_content_range = bytes 0-0/0
+upload_content_type = text/plain
+upload_field_name = file
+upload_file_name = test.txt
+upload_file_number = 1
+upload_tmp_path = ${ENV{TEST_NGINX_UPLOAD_PATH}}/store/8/0000123458
+}
+--- upload_file_like eval
+qr/^test$/
+
+=== TEST 7: multipart upload with quoted boundary and unquoted names
+--- config eval: $::config
+--- more_headers
+Content-Type: multipart/form-data; boundary="BOUNDARY"
+--- request eval
+"POST /upload/
+--BOUNDARY
+Content-Disposition: form-data; name=file; filename=test.txt\r
+Content-Type: text/plain\r
+\r
+test\r
+--BOUNDARY--\r
+"
+--- error_code: 200
+--- response_body eval
+qq{upload_content_range = bytes 0-0/0
+upload_content_type = text/plain
+upload_field_name = file
+upload_file_name = test.txt
+upload_file_number = 1
+upload_tmp_path = ${ENV{TEST_NGINX_UPLOAD_PATH}}/store/5/0000246915
+}
+--- upload_file_like eval
+qr/^test$/