Nested file property in requestBody results in unexpected errors

[ajmanlove]

I'm experiencing some issues when using file properties nested within a requestBody. I'm new to this library, so feel free to point out if I'm just neglecting to consider something basic or obvious -- it's entirely possible this is my own openapi-fu lacking here.

Using:
Ruby: 3.4.1
Rails: 8.0.1
Rspec: 3.13, Rspec Rails: 7.1.1
openapi_first: 2.2.3

OpenAPI 3.0

Use Case 1: No Nesting

openapi.yaml

openapi: 3.0.3
# ...
paths:
    '/api/v1/signup':
      $ref: 'paths/signup.yaml'

paths/signup.yaml

patch:
  requestBody:
    content:
      multipart/form-data:
        schema:
          type: object
          properties:
            # ...
            avatar:
              type: string
              format: binary
                
  responses:
    # ...

Spec:

it 'should PATCH with avatar file' do
    body = {
      avatar: fixture_file_upload('image.jpg')
    }
    patch user_registration_path,
        params: body,
        headers: {
          'Content-Type': 'multipart/form-data'
        }
end

No middleware error, works fine.

Use Case 2: Nested file property

paths/signup.yaml

patch:
  requestBody:
    content:
      multipart/form-data:
        schema:
          type: object
          properties:
            user:
              type: object
              properties:
                # ...
                avatar:
                  type: string
                  format: binary
  responses:
    # ...

Spec:

it 'should PATCH with avatar file' do
    body = {
      user: {
        avatar: fixture_file_upload('image.jpg')
      }
    }
    patch user_registration_path,
      params: body,
      headers: {
        'Content-Type': 'multipart/form-data'
      }
end

Results in error response:

{
    errors: [
        {
            code: "string",
            source: {
                pointer: "/user/avatar"
            },
            status: "400",
            title: "value at `/user/avatar` is not a string"
        },
        {
            code: "format",
            source: {
                pointer: "/user/avatar"
            },
            status: "400",
            title: "value at `/user/avatar` does not match format: binary"
        }
    ]
}

Use Case 3: Array of Files

paths/signup.yaml

patch:
  requestBody:
    content:
      multipart/form-data:
        schema:
          type: object
          properties:
            # ...
            filesArray:
                type: array
                items:
                  type: string
                  format: binary
                
  responses:
    # ...

Spec:

it 'should PATCH with avatar file' do
    body = {
      filesArray: [
        fixture_file_upload('image.jpg')
      ]
    }
    patch user_registration_path,
      params: body,
      headers: {
        'Content-Type': 'multipart/form-data'
      }
end

Results in error:

{
    errors: [
        {
            code: "string",
            source: {
                pointer: "/filesArray/0"
            },
            status: "400",
            title: "value at `/filesArray/0` is not a string"
        },
        {
            code: "format",
            source: {
                pointer: "/filesArray/0"
            },
            status: "400",
            title: "value at `/filesArray/0` does not match format: binary"
        }
    ]
}

Use Case Four, Array of Objects with with file property

paths/signup.yaml

patch:
  requestBody:
    content:
      multipart/form-data:
        schema:
          type: object
          properties:
            # ...
            filesObjArray:
              type: array
              items:
                type: object
                properties:
                  file:
                    type: string
                    format: binary
                
  responses:
    # ...

Spec:

it 'should PATCH with avatar file' do
    body = {
      filesObjArray: [
        { avatar: fixture_file_upload('image.jpg') }
      ]
    }
    patch user_registration_path,
      params: body,
      headers: {
        'Content-Type': 'multipart/form-data'
      }
end

Results in error response:

{
    errors: [
        {
            code: "string",
            source: {
                pointer: "/filesObjArray/0/file"
            },
            status: "400",
            title: "value at `/filesObjArray/0/file` is not a string"
        },
        {
            code: "format",
            source: {
                pointer: "/filesObjArray/0/file"
            },
            status: "400",
            title: "value at `/filesObjArray/0/file` does not match format: binary"
        }
    ]
}

OpenAPI 3.1

If I keep using format: binary, I'll get mostly the same errors, minus the does not match format: binary error, which makes some sense.

If I replace the use of format: binary with something like contentMediaType: application/octet-stream or image/jpeg, I get a different thrown error:

JSONSchemer::UnknownContentMediaType:
  application/octet-stream

I can also use the empty {} for typing to avoid the errors, but this seems incorrect since it's tacitly an unknown/any type.

Considerations

In all of the above, if I remove the OpenapiFirst::Middlewares::RequestValidation middleware and the underlying controller code supports the body structure, actual functionality works fine in spec, postman, javascript client, etc

[ahx]

Hey @ajmanlove ,

thank you for the report. I think all your examples make sense and should just work.

can also use the empty {} for typing to avoid the errors
I guess right now this or using 'format: binary', without the "'type: string'" is an okay workaround to remove the blocker on your side until this issue is fixed (I have a PR almost ready).

I am not really satisfied with how the gem works around validating type: 'string', format: 'binary' in multipart/form-data request bodies. Here is what the library currently does:

1. Have Rack parse the multipart request body.
2. Go through the parsed body and put uploaded file contents to where the "string" value is expected. This means it reads the uploaded file.
3. This makes the type: string validation succeed, because now it is just a string (binary or not).

The reason why this approach currently works only for your Use Case 1 is that it does not handle nested values, which is a bug that is easy to fix.

However. We should not read the uploaded before the user does tells us to, so what I would like to do instead, maye in the next major (or minor 🙈) version is something like:

1. Notice that the schema for a multipart/form-data request contains a "binary" field, which looks like a file upload. Because there is no user use case for that, right?
2. Change the validation for that specific value so that it does not check for a string.
3. The validation succeeds without us reading the file contents. The parsed request body now looks like the usual Rack::Request#POST data with inlined information about the uploaded file (name, tempfile etc.).

[ahx]

Hey again @ajmanlove,

There is a PR ready on branch fix-nested-multipart-file-upload, which just fixes parsing of nested multipart/form-data without changing the overall behavior.

Do you mind trying that before I release a new version?

[ajmanlove]

Hey @ahx , thanks for that quick response.

Understood, I'll test against that PR shortly and report back.

Regarding your proposal, that all makes sense to me and I'm inclined to agree. I would agree that reading file data unless necessary seems like overhead or if openapi_first is being utilized in the stack as a runtime contract-enforcer beyond the usage as documentation-first development tool. If a future version enabled users control over the gradations between development-first tool -> runtime enforcer middleware, that sounds powerful.

[ajmanlove]

Confirmed the PR passes all of my test cases ✅

[ahx]

A fix was released in 2.2.4