fix(HTTP): error boundary in HTTP-Header, Content-Type

[HR1025]

see https://www.rfc-editor.org/rfc/rfc2046#section-5.1

Poco carries extra ''", which can cause nginx to return 'Malformed multipart message', upload fail.

[HR1025]

[HR1025]

[matejk]

@HR1025 , can you please correct unit tests to match the code, please?

[HR1025]

Of course. The test cases have been corrected

[HR1025]

The way Poco handles this might be correct and compliant with the standard (by adding "). According to the compatibility description in RFC 2046, when the boundary contains special characters, it is necessary to add ". The relevant description is as follows:

   WARNING TO IMPLEMENTORS:  The grammar for parameters on the Content-
   type field is such that it is often necessary to enclose the boundary
   parameter values in quotes on the Content-type line.  This is not
   always necessary, but never hurts. Implementors should be sure to
   study the grammar carefully in order to avoid producing invalid
   Content-type fields.  Thus, a typical "multipart" Content-Type header
   field might look like this:

     Content-Type: multipart/mixed; boundary=gc0p4Jq0M2Yt08j34c0p

   But the following is not valid:

     Content-Type: multipart/mixed; boundary=gc0pJq0M:08jU534c0p

   (because of the colon) and must instead be represented as

     Content-Type: multipart/mixed; boundary="gc0pJq0M:08jU534c0p"

However, Poco's generated boundary does not include any special characters, so removing the " would cause no harm.
It is possible that different HTTP services have varying implementations of the standard.

[matejk]

@HR1025 Does this mean that problem is not in Poco at all because it follows the RFC.

[HR1025]

@HR1025 Does this mean that problem is not in Poco at all because it follows the RFC.
I checked and found that the exception originates from an Nginx plugin, nginx-upload-module.
It can be observed here that an exception does occur if "" is present.
Perhaps my understanding of the standard is still somewhat flawed, or there might be other RFCs that provide additional descriptions for such edge cases.
In any case, it can be confirmed that this exception comes from the following code.
However, in another branch of logic, I noticed there is handling for "quoted boundaries," while the above code does not have such handling. This is puzzling.

[HR1025]

https://github.com/fdintino/nginx-upload-module/blob/master/ngx_http_upload_module.c

static ngx_int_t upload_parse_request_headers(ngx_http_upload_ctx_t *upload_ctx, ngx_http_headers_in_t *headers_in) { /* {{{ */
    ngx_str_t                 *content_type, s;
    ngx_list_part_t           *part;
    ngx_table_elt_t           *header;
    ngx_uint_t                 i;
    u_char                    *mime_type_end_ptr;
    u_char                    *boundary_start_ptr, *boundary_end_ptr;
    ngx_atomic_uint_t          boundary;
    ngx_http_upload_loc_conf_t *ulcf;

    ulcf = ngx_http_get_module_loc_conf(upload_ctx->request, ngx_http_upload_module);

    // Check whether Content-Type header is missing
    if(headers_in->content_type == NULL) {
        ngx_log_error(NGX_LOG_ERR, upload_ctx->log, ngx_errno,
                      "missing Content-Type header");
        return NGX_HTTP_BAD_REQUEST;
    }

    content_type = &headers_in->content_type->value;

    if(ngx_strncasecmp(content_type->data, (u_char*) MULTIPART_FORM_DATA_STRING,
        sizeof(MULTIPART_FORM_DATA_STRING) - 1)) {

        if(!ulcf->resumable_uploads) {
            ngx_log_error(NGX_LOG_ERR, upload_ctx->log, 0,
                "Content-Type is not multipart/form-data and resumable uploads are off: %V", content_type);
            return NGX_HTTP_UNSUPPORTED_MEDIA_TYPE;
        }
        /*
         * Content-Type is not multipart/form-data,
         * look for Content-Disposition header now
         */
        part = &headers_in->headers.part;
        header = part->elts;

        for (i = 0;;i++) {
            if (i >= part->nelts) {
                if (part->next == NULL) {
                  break;
                }

                part = part->next;
                header = part->elts;
                i = 0;
            }

            if(!strncasecmp(CONTENT_DISPOSITION_STRING, (char*)header[i].key.data, sizeof(CONTENT_DISPOSITION_STRING) - 1 - 1)) {
                if(upload_parse_content_disposition(upload_ctx, &header[i].value)) {
                    ngx_log_error(NGX_LOG_INFO, upload_ctx->log, 0,
                        "invalid Content-Disposition header");
                    return NGX_ERROR;
                }

                upload_ctx->is_file = 1;
                upload_ctx->unencoded = 1;
                upload_ctx->raw_input = 1;
        
                upload_ctx->data_handler = upload_process_raw_buf;
            }else if(!strncasecmp(SESSION_ID_STRING, (char*)header[i].key.data, sizeof(SESSION_ID_STRING) - 1 - 1)
                || !strncasecmp(X_SESSION_ID_STRING, (char*)header[i].key.data, sizeof(X_SESSION_ID_STRING) - 1 - 1))
            {
                if(header[i].value.len == 0) {
                    ngx_log_debug0(NGX_LOG_DEBUG_CORE, upload_ctx->log, 0,
                                   "empty Session-ID in header");
                    return NGX_ERROR;
                }

                if(ngx_http_upload_validate_session_id(&header[i].value) != NGX_OK) {
                    ngx_log_debug0(NGX_LOG_DEBUG_CORE, upload_ctx->log, 0,
                                   "invalid Session-ID in header");
                    return NGX_ERROR;
                }

                upload_ctx->session_id = header[i].value;

                ngx_log_debug1(NGX_LOG_DEBUG_CORE, upload_ctx->log, 0,
                               "session id %V", &upload_ctx->session_id);
            }else if(!strncasecmp(CONTENT_RANGE_STRING, (char*)header[i].key.data, sizeof(CONTENT_RANGE_STRING) - 1 - 1) 
                || !strncasecmp(X_CONTENT_RANGE_STRING, (char*)header[i].key.data, sizeof(X_CONTENT_RANGE_STRING) - 1 - 1))
            {
                if(header[i].value.len == 0) {
                    ngx_log_debug0(NGX_LOG_DEBUG_CORE, upload_ctx->log, 0,
                                   "empty Content-Range in part header");
                    return NGX_ERROR;
                }

                if(strncasecmp((char*)header[i].value.data, BYTES_UNIT_STRING, sizeof(BYTES_UNIT_STRING) - 1)) {
                    ngx_log_debug0(NGX_LOG_DEBUG_CORE, upload_ctx->log, 0,
                                   "unsupported range unit");
                    return NGX_ERROR;
                }

                s.data = (u_char*)(char*)header[i].value.data + sizeof(BYTES_UNIT_STRING) - 1;
                s.len = header[i].value.len - sizeof(BYTES_UNIT_STRING) + 1;

                if(ngx_http_upload_parse_range(&s, &upload_ctx->content_range_n) != NGX_OK) {
                    ngx_log_debug2(NGX_LOG_DEBUG_CORE, upload_ctx->log, 0,
                                   "invalid range %V (%V)", &s, &header[i].value);
                    return NGX_ERROR;
                }

                ngx_log_debug3(NGX_LOG_DEBUG_CORE, upload_ctx->log, 0,
                               "partial content, range %O-%O/%O", upload_ctx->content_range_n.start, 
                               upload_ctx->content_range_n.end, upload_ctx->content_range_n.total);

                if(ulcf->max_file_size != 0 && upload_ctx->content_range_n.total > ulcf->max_file_size) {
                    ngx_log_error(NGX_LOG_ERR, upload_ctx->log, 0,
                                  "entity length is too big");
                    return NGX_HTTP_REQUEST_ENTITY_TOO_LARGE;
                }

                if( (upload_ctx->content_range_n.end - upload_ctx->content_range_n.start + 1)
                    != headers_in->content_length_n) 
                {
                    ngx_log_error(NGX_LOG_ERR, upload_ctx->log, 0,
                                  "range length is not equal to content length");
                    return NGX_HTTP_RANGE_NOT_SATISFIABLE;
                }

                upload_ctx->partial_content = 1;
            }
        }

        if(!upload_ctx->unencoded) {
            ngx_log_error(NGX_LOG_ERR, upload_ctx->log, 0,
                           "Content-Type is not multipart/form-data and no Content-Disposition header found");
            return NGX_HTTP_UNSUPPORTED_MEDIA_TYPE;
        }

        upload_ctx->content_type = *content_type;

        boundary = ngx_next_temp_number(0);

        content_type->data =
            ngx_pnalloc(upload_ctx->request->pool,
                        sizeof(MULTIPART_FORM_DATA_STRING "; boundary=") - 1
                        + NGX_ATOMIC_T_LEN);

        if (content_type->data == NULL) {
            return NGX_ERROR;
        }

        content_type->len =
                       ngx_sprintf(content_type->data,
                                   MULTIPART_FORM_DATA_STRING "; boundary=%0muA",
                                   boundary)
                       - content_type->data;

        boundary_start_ptr = content_type->data + sizeof(MULTIPART_FORM_DATA_STRING "; boundary=") - 1;
        boundary_end_ptr = content_type->data + content_type->len;
    }
    else{
        // Find colon in content type string, which terminates mime type
        mime_type_end_ptr = (u_char*) ngx_strchr(content_type->data, ';');

        upload_ctx->boundary.data = 0;

        if(mime_type_end_ptr == NULL) {
            ngx_log_debug0(NGX_LOG_DEBUG_CORE, upload_ctx->log, 0,
                           "no boundary found in Content-Type");
            return NGX_UPLOAD_MALFORMED;
        }

        boundary_start_ptr = ngx_strstrn(mime_type_end_ptr, BOUNDARY_STRING, sizeof(BOUNDARY_STRING) - 2);

        if(boundary_start_ptr == NULL) {
            ngx_log_debug0(NGX_LOG_DEBUG_CORE, upload_ctx->log, 0,
                           "no boundary found in Content-Type");
            return NGX_UPLOAD_MALFORMED; // No boundary found
        }

        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");
            return NGX_UPLOAD_MALFORMED;
        }
    }

    // Allocate memory for entire boundary plus \r\n-- plus terminating character
    upload_ctx->boundary.len = boundary_end_ptr - boundary_start_ptr + 4;
    upload_ctx->boundary.data = ngx_palloc(upload_ctx->request->pool, upload_ctx->boundary.len + 1);

    if(upload_ctx->boundary.data == NULL)
        return NGX_HTTP_INTERNAL_SERVER_ERROR;

    ngx_cpystrn(upload_ctx->boundary.data + 4, boundary_start_ptr,
        boundary_end_ptr - boundary_start_ptr + 1);
    
    // Prepend boundary data by \r\n--
    upload_ctx->boundary.data[0] = '\r'; 
    upload_ctx->boundary.data[1] = '\n'; 
    upload_ctx->boundary.data[2] = '-'; 
    upload_ctx->boundary.data[3] = '-'; 

    /*
     * NOTE: first boundary doesn't start with \r\n. Here we
     * advance 2 positions forward. We will return 2 positions back 
     * later
     */
    upload_ctx->boundary_start = upload_ctx->boundary.data + 2;
    upload_ctx->boundary_pos = upload_ctx->boundary_start;

    return NGX_OK;
} /* }}} */

[matejk]

@HR1025, I assume that there is nothing to be done in Poco for this matter, right?