feat: add message validation

[zekth]

fix: https://github.com/fastify/fastify-websocket/issues/190

First implementation. Basically the use would be:

const Fastify = require('fastify')
const fastifyWebsocket = require('.')
const { ValidateWebSocket } = require('.')
const fastify = Fastify({ logger: true })

fastify.register(fastifyWebsocket, { options: { WebSocket: ValidateWebSocket } })

fastify.get('/', {
    websocket: true,
    schema: {
        body: {
            oneOf:
                [
                    { type: 'null' }, // This is required because of the upgrade
                    {
                        type: 'object',
                        properties: {
                            foo: {
                                type: 'string'
                            }
                        },
                        required: [
                            'foo'
                        ],
                        additionalProperties: false
                    }
                ]

        }
    }
}, (connection, request) => {
    connection.socket.on('message', () => {
        connection.socket.send('ok')
    })
})

fastify.listen(3000, '0.0.0.0', (err) => {
})

This also adds 2 way of handling the validation errors with the strictMode. If is set to true it closes the connection with a 1003 code, otherwise it sends the validator errors in the payload.

Kudos to @climba03003 who helped a lot

Checklist

• [ ] run npm run test and npm run benchmark
• [ ] tests and/or benchmarks are included
• [ ] documentation is changed or added
• [ ] commit message and code follows the Developer's Certification of Origin and the Code of conduct

[zekth]

Could you emit an event in case a packet is dropped?

In a case of strictMode set to false? Yes sure

[airhorns]

@zekth I see why you'd want to do this but I am wondering if this could be done completely in userland or in a separate package. Not super opposed to merging this, but this implementation has to make a few big assumptions:

• that a user wants to validate every message
• that a developer wants to validate according to the same schema the whole time
• that messages to validate are JSON
• that a developer wants adopt this mode approach to errors.

I can contrive situations where each one of those things is not true. If any of those are not true, the developer has to go back to square one and write validation code themselves -- dunno how likely they are or how likely the happy path is where this thing helps. But because it hasn't come up a lot, I am a little hesitant to stick it right in the mainline code. It also requires some kinda ugly stuff like setting those properties on the socket.

One other thing we could do would be to add the facilities to make this possible and nice in userland, and then add examples that show how you might validation json yourself to the docs, as well as showing how you might validate msgpack or something like that too!

[zekth]

that a user wants to validate every message

I don't get why some people would want to validate only a subset of messages

that a developer wants to validate according to the same schema the whole time

Would you change your route schema on the fly?

that messages to validate are JSON

currently indeed it validates only the JSON, but using the schema definition we can validate any type, this case juste needs to be handled too.

that a developer wants adopt this mode approach to errors.

What would be a better approach?

One other thing we could do would be to add the facilities to make this possible and nice in userland, and then add examples that show how you might validation json yourself to the docs, as well as showing how you might validate msgpack or something like that too!

How would you achieve that? Seems like a bad DX as the user won't have the benefit of using the tools that fastify provides. The idea in here is to have only validated messages to be propagated inside the server message event.

Main idea behind this feature is to be able to build AsyncApi specs like: https://www.asyncapi.com/blog/websocket-part2 We have api contracts for Rest and GraphQl and i don't get why we won't have for websockets.

[airhorns]

I don't get why some people would want to validate only a subset of messages

Sometimes other systems are already doing validation and hand you back strings to send, like y.js or graphql-ws

Would you change your route schema on the fly?

No, but for stateful connections like a websocket, you might enter different phases of the connection that only accept certain message types at certain times. Lots of wire protocols have modes where they flip from some structured message format to a binary stream for a while, like Postgres' COPY instruction

currently indeed it validates only the JSON, but using the schema definition we can validate any type, this case juste needs to be handled too.

That's kinda what I am saying -- this is a maximalist approach, do we want that? I am ok if we do, it just increases the maintenance burden here for a potentially narrow set of use cases which require it.

What would be a better approach?

try / catch in the message handler for super explicitness. error event handlers kinda suck IMO because you need to remember to register them, otherwise the errors just go into the void. Stack based error handling at least interrupts the rest of your code's execution and is loud. This approach doesn't really jive with fastify-websocket doing the validation though. Another approach would be adding an onError callback option, that's a bit more explicit and we could make it required, and another higher level approach would be adding a higher order function like withMessageValidation(yourExistingHandler, options} that wrapped the handler to do validation outside fastify-websocket itself.

[zekth]

Lots of wire protocols have modes where they flip from some structured message format to a binary stream for a while, like Postgres' COPY instruction

https://github.com/fastify/fastify-websocket/pull/191/files#diff-e727e4bdf3657fd1d798edcd6b099d6e092f8573cba266154583a746bba0f346R218

narrow set of use cases which require it.

I think we are in a chicken vs egg problem in here. Is there a reason not to push forward api contracts and message validation over websocket?

try / catch in the message handler for super explicitness.

With this approach there is no need for this feature in the end, because anybody can use their own validator in the end.

another higher level approach would be adding a higher order function like withMessageValidation(yourExistingHandler, options} that wrapped the handler to do validation outside fastify-websocket itself.

But the logic is in ValidateWebSocket which is not really inside fastify-websocket. That means any developer can extend it or replace it. Having another package just to handle the validation makes no sense to me. We can isolate this part: https://github.com/fastify/fastify-websocket/pull/191/files#diff-e727e4bdf3657fd1d798edcd6b099d6e092f8573cba266154583a746bba0f346R62-R64 which in the end in a case of a non ValidatedWebsocket option there will be no change inside the fastify-websocket plugin.

[zekth]

Any thoughts @mcollina ?