fastify icon indicating copy to clipboard operation
fastify copied to clipboard

Empty response when validation is used

Open vasilianeugen opened this issue 3 years ago • 8 comments

Prerequisites

  • [X] I have written a descriptive issue title
  • [X] I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.0.2

Plugin version

3.0.1

Node.js version

16.14.2

Operating system

Windows

Operating system version (i.e. 20.04, 11.3, 10)

10

Description

I have a schema that previously worked on Fastify 3.x

export const userLogInSchema: FastifySchema = { tags: ['user'], description: '[Cognito] - User login', body: S.oneOf([S.ref('UserLoginRequest'), S.ref('RefreshTokenRequest')]), response: { 200: S.oneOf([S.ref('AuthResponse'), S.ref('MFASetupResponse'), S.ref('MFALoginResponse')]) } }

I get now an empty JSON as response {} If I remove the response validation part, the then I get again a value as it worked before. The response is valid as it is conform to the MFALoginResponse schema. Anyway if there is a validation problem there should be an validation error.

The S is from the 'fluent-json-schema' lib

Steps to Reproduce

Example found in description.

In handler code the JSON returned value is correct and I use reply.send([JSON]) to send it.

Expected Behavior

The endpoint returns the JSON value that is put in reply.send()

vasilianeugen avatar Jun 14 '22 13:06 vasilianeugen

Thanks for reporting!

Can you provide steps to reproduce? We often need a reproducible example, e.g. some code that allows someone else to recreate your problem by just copying and pasting it. If it involves more than a couple of different file, create a new repository on GitHub and add a link to that.

mcollina avatar Jun 14 '22 14:06 mcollina

@mcollina It seems that the use of oneOf in response is the problem. If I use only one validation it works. Also other endpoints that have simple response validations work correctly.

This one works: export const userLogInSchema: FastifySchema = { tags: ['user'], description: '[Cognito] - User login', body: S.oneOf([S.ref('UserLoginRequest'), S.ref('RefreshTokenRequest')]), response: { 200: S.ref('MFALoginResponse') } }

vasilianeugen avatar Jun 14 '22 14:06 vasilianeugen

@mcollina I have further found this: I used only two items in oneOf that have a common field called "Session". As a result the response contains only the "Session" field. In my initial case there was no common field for the 3 of the schemas. So it seems it returns only the common fields of all the schemas in the oneOf list. Kind of intersection of the fields from schemas

vasilianeugen avatar Jun 14 '22 14:06 vasilianeugen

Thanks for the investigation. But we need a repro to confirm if it is a bug or expected behavior.

For serialization, oneOf actually do the same job as anyOf. It test the input and schema one by one in order. If any of them match, it serialize as that schema.

From your description, it seems like fast-json-stringify do not match the first two of your schema and serialize as the last one.

climba03003 avatar Jun 14 '22 15:06 climba03003

Thanks for the investigation. But we need a repro to confirm if it is a bug or expected behavior.

For serialization, oneOf actually do the same job as anyOf. It test the input and schema one by one in order. If any of them match, it serialize as that schema.

From your description, it seems like fast-json-stringify do not match the first two of your schema and serialize as the last one.

No, it serializes as the common properties of the schemas. If there are no common properties it returns an empty JSON. Previously in 3.x it worked correctly and returned the schema that matched.

vasilianeugen avatar Jun 14 '22 17:06 vasilianeugen

To replicate your issue we need your schemas or a minimal reproducible code that we can run. We do not have the whole schema here:

body: S.oneOf([S.ref('UserLoginRequest'), S.ref('RefreshTokenRequest')]),
response: {
200: S.oneOf([S.ref('AuthResponse'), S.ref('MFASetupResponse'), S.ref('MFALoginResponse')])

the new version adheres more strictly to the JSON schema spec, so I suspect that you have some $ids to update

Eomm avatar Jun 14 '22 19:06 Eomm

@climba03003

My first schema is

export const mfaLoginResponseJsonSchema = S.object()
    .id('MFALoginResponse')
    .prop('ChallengeName', S.string())
    .prop('Session', S.string()) 

And when I looked into code it seems that if the first schema validates it retunrs {} MFALoginResponse dose not have required fields so I guess it validates and returns {} because of that.

 function main (input) {
      let json = ''      
            if(ajv.validate("MFALoginResponse", input))
              json += anonymous0(input)
          
            else if(ajv.validate("AuthResponse", input))
              json += anonymous1(input)
          
          else throw new Error(`The value ${JSON.stringify(input)} does not match schema definition.`)
        
      return json
    }

It would be good that for oneOf we would not have if ... else but instead check the next schema in case the first one returns empty json {} and so on

vasilianeugen avatar Jun 21 '22 15:06 vasilianeugen

Previously in 3.x it worked correctly and returned the schema that matched.

It was a bug, actually 😄 the old v2 (included in fastify v3) returns all the properties regardless of the schema

Here an example: https://runkit.com/embed/dotem4i1p7gl

MFALoginResponse dose not have required fields so I guess it validates and returns {} because of that.

Yes, indeed

It would be good that for oneOf we would not have if ... else but instead check the next schema in case the first one returns empty json {} and so on

Not sure about it. What if a user would like an empty object instead? I think you need to define your schemas to don't overlap them, or you should arrange your oneOf array to keep the schema with fewer constraints in the latter position.

This behaviour is described on the docs: https://github.com/fastify/fast-json-stringify#anyof-and-oneof

// without "required" validation any object will match


Talking about the standard, the oneOf keyword must match only one element of the array, so we should think if we must run all the validations and throw an error accordingly

As ajv explains:

The data is valid if it matches exactly one JSON Schema from this array. Validators have to validate data against all schemas to establish validity according to this keyword.

https://ajv.js.org/json-schema.html#oneof

My opinion on that, is that fast-json-stringify is not a validator

Eomm avatar Jul 28 '22 12:07 Eomm

Things have changed and fast-json-stringify has evolved.

Closing as stale

Feel free to open a new issue with a Minimal, Reproducible Example

Eomm avatar Aug 26 '23 16:08 Eomm