Support for string format binary e.g. application/octet-stream

[mirekgw]

Hi,
Maybe I'm doing something wrong but I'm trying to have an endpoint that is used for sending binary data, with PUT. The API definition is like this:
requestBody:
description: Binary Data Stream
content:
application/octet-stream:
schema:
type: string
format: binary
It looks ok according to this: https://swagger.io/docs/specification/describing-request-body/file-upload/

When I try to execute the request, I get following error:
"message": "request.body should be string",
"errors": [
{
"path": ".body",
"message": "should be string",
"errorCode": "type.openapi.validation"
}
]

I did a quick peek at the code of your lib, and it seems you don't support binary strings. Is that correct or am I doing something wrong? If it's not implemented what is a chance you could enhance the lib with that feature?
Thanks

[cdimascio]

@mirekgw thanks for the issue. this sounds like a bug / missing feature. either way, we'll certainly add support.
if you know where things went awry, feel free to submit a PR. they are most welcome.
if not, i'll try to take a look this weekend.

[cdimascio]

@mirekgw this should be fixed in version 3.8.0
here is the test case
please give it a try. let me know how it goes.
thanks again!

[cdimascio]

this fix should honor format: binary

[cdimascio]

closing this out. please reopen if the issue persists

[mirekgw]

Guys,
First of all thank you very much for super fast response. It's great to see you care for you product better than most paid frameworks :)

I took latest version of lib (3.8.0) and unfortunately the issue is not fixed for me. I did some debugging and here is the result. In my case the request still fails validation. The problem is that buildMiddleware() function in openapi.request.validator.js has this code: const valid = validator(Object.assign(Object.assign({}, req), { cookies })); and that code is returning false for my case.
I printed the validator function and interesting part of it is below:

    var data1 = data.body;
    if (data1 !== undefined) {
        var errs_1 = errors;
        if (typeof data1 !== "string") {
            var dataType1 = typeof data1;
            var coerced1 = undefined;
            if (dataType1 == 'number' || dataType1 == 'boolean') coerced1 = '' + data1;
            else if (data1 === null) coerced1 = '';
            if (coerced1 === undefined) {
                var err = { keyword: 'type', dataPath: (dataPath || '') + '.body', schemaPath: '#/properties/body/type', params: { type: 'string' }, message: 'should be string' };
                if (vErrors === null) vErrors = [err];
                else vErrors.push(err); errors++;
            }
            else { data1 = coerced1; data['body'] = coerced1; }
        }
        if (typeof data1 === "string") {
            if (!formats.binary(data1)) {
                var err = { keyword: 'format', dataPath: (dataPath || '') + '.body', schemaPath: '#/properties/body/format', params: { format: 'binary' }, message: 'should match format "binary"' };
                if (vErrors === null) vErrors = [err];
                else vErrors.push(err); errors++;
            }
        }
        var valid1 = errors === errs_1;
    }

The issue in my case is that req.body type is Object if I don't use any raw body parser. When I use BodyParser.raw() the req.body is Buffer type. So as you can see from the code in my case typeof data1 !== "string" and it's also not a number nor boolean, so coerced1 is undefined. because of that functions return error: "body should be string".

I think the issue is that OpenApi spec defines binary data to be represented by string for some weird reason. Because of that this part of validation needs to be handled in a special way. I would assume you should first check if the string type is format: binary and just pass the validation if that's the case.

@cdimascio I hope my comment makes sense. Could you please look again at the issue?
Thanks

[mirekgw]

I believe I know what the issue is. The problem is that ajv library is for JSON validation and as we all know it's text only format. OpenApi introduced strange construct i.e. string with format binary. It's not compatible with JSON so basically we can't use it (or at least we can't validate that with ajv). Not sure how to proceed here, I'm not expecting JSON spec will be extended with "binary" format for string.

I suppose for me the best solution is to ignore validation of endpoints that use binary data. The question is how to achieve that with your library, as ignorePaths config option doesn't do what I would expect it to do? I will play more with ignorePaths and most likely create another issue when I'm sure why it's not working as expected.

[cdimascio]

@mirekgw, thanks for digging in an investigating this. this is very helpful. if you don't mind sharing a canonical version of your spec and express app (perhaps via a public repo on github). I'll certainly have a look. It will help to have a reproducible example, one to explore possible workarounds/solutions

[mirekgw]

I will prepare sample app to reproduce.

As to the issue, I believe I have found an easy solution and I will create PR later today. Hopefully you will accept the idea, that is not verifying body at all if the content-type is 'application/octet-stream'. I believe verifying octet-stream content don't make any sense as it's just bunch of bytes. I quickly patched .js file containing buildMiddleware() with following code and it seems to be working fine for me:

    const isOctetStream = contentType.contentType === 'application/octet-stream';
    const properties = Object.assign(Object.assign({}, parameters), isOctetStream ? {} : { body: body });
    const required = (body.required && !isOctetStream) ? ['body'] : [];

[mirekgw]

I prepared repo with sample app that reproduces the issue, sent invite to @cdimascio. I will create PR with proposed fix in few minutes.

Could you reopen the issue, as it's not fixed for me, and I don't have rights to reopen.
Thanks

[mirekgw]

Don't have rights to create branch, so I'll type here what I would put in PR.
As I mention before my idea is that there is no point to validate request's body if content-type is application/octet-stream. So I propose to change src/middlewares/openapi.request.validator.ts file in following way:
In private buildMiddleware() function , add new line at line 94 and modify 95 & 96:

const isOctetStream = contentType.contentType === 'application/octet-stream';

const properties: ValidationSchema = { ...parameters, body: ( isOctetStream ? {} : body) };
const required = ((<SchemaObject>body).required  && !isOctetStream) ? ['body'] : [];

I did this modification and all test pass exactly like without it.
Is it OK to include this into the library?

[cdimascio]

Thanks for your efforts, @mirekgw. They are greatly appreciated. I'll be sure to try your repo within the next few days. Please feel free to submit a PR. I'll definitely have a look. Thanks again!

[cdimascio]

@mirekgw note that you create a PR even though you can't creat a branch in this repo here is how

tldr
Find a project you want to contribute to
Fork it
Clone it to your local system
Make a new branch
Make your changes
Push it back to your repo
Click the Compare & pull request button
Click Create pull request to open a new pull request

Detailed steps with example: https://opensource.com/article/19/7/create-pull-request-github

[cdimascio]

Also if u submit the PR and we merge your change to master, you will show up on this project's contributors page :)

[mirekgw]

Created PR

[cdimascio]

fix is in v3.9.3