Option to align route params with schema at startup

[avele]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

Introduce startup validation in Fastify to check consistency between route parameters in path strings and schema definitions.

Motivation

Ensuring route parameter consistency to prevent runtime errors

Example

// Inconsistent route params and schema could lead to unnoticed issues until runtime
fastify.get('/item/:itemId', {
  schema: {
    params: {
      type: 'object',
      properties: {
        id: { type: 'string' } // Potential issue: 'itemId' in route vs 'id' in schema
      }
    }
  },
  handler: (request, reply) => { /* ... */ }
});

A pre-runtime check could catch these mismatches early, enhancing developer experience and application stability.

[metcoder95]

👋
I wouldn't add this to the core as it can make users make wrong assumptions and noise while working with the framework and setting its routes.

Rather this sounds like a good idea for a plugin, would you like to create one?

[Eomm]

We have something similar here:

• feat: generate path params definition when missing fastify-swagger#761
  But it does not throw if there is a mismatch.

The hardest thing is to list the path parameters because they are a router's internal info

[sf3ris]

👋 Hi,
Was taking a look at this issue since I had the same problem a few times and it has been quite time-consuming to debug.
Since the parameters are router's internal info, do you think could be feasible to validate right before adding the route within the route.addNewRoute function? Maybe passing a flag if we dont want to be too strict?

I guess to make a standalone plugin we need somehow to leak the router's internal or expose them.
What you think? 🤔

[metcoder95]

I'd say that leaking the router as is can be pretty dangerous as it's easy to alter it and make fastify fail, causing issues to spawn within the project (unless we assess is not a problem).

Potentially, have we already considered exposing an abstraction of fmw.findRoute?

I can see potential use cases there (like this plugin). Although I've not tested if it returns the parameters of the route path as well.

[sf3ris]

Makes sense, Ill try to give it a shot to see whether we can abstract it so that other plugins can use it.
So, just to recap, do you think that the reported case above should not throw on the fastify core without any plugin?
The plugin should only check that the parameters declared on the schema should match the ones in the URL?

[sf3ris]

Tried a quick implementation for opening a discussion wether is better to extract into a standalone plugin (what scope would it add? just to validate routes with params?)
see: #5230

[metcoder95]

The PR LGTM overall, but I'm over the fence with having the validation within core. This seems to have pretty specific use cases, and the harm of unexpected behaviour can cause friction.

That's one of the main reasons to suggest develop a community plugin instead

[sf3ris]

Should this be closed due to #5230 or do we want to keep it open for the plugin discussion?

[Uzlopak]

I think it should be in core. It feels somehow wrong to load a plugin, to "just" check if routes are registered correctly.

[mcollina]

We support multiple validators, and we can't safely implement this.

[metcoder95]

Should this be closed due to #5230 or do we want to keep it open for the plugin discussion?

I believe we should keep it open for plugin discussion

[sf3ris]

To me, a nice plugin I would use to validate the parameters would be:

Plugin proposal adding a new hook

• Fastify core could add a new event like onRegister emitted every time a new route is registered (If its already something similar, better)
• The plugin, after being registered, will automatically add a new hook onRegister that will validate the route parameters.

Advantages

• The onRegister hook can be used by any other plugin that wants to validate or performs any action after every route is registered (logging, etc ..)
• The user of the plugin doesn't need to validate each route every time or calling like a plugin.validateAll method that could have lots of responsibility like:
  • how to retrieve all the routes
  • how to validate all the routes

Cons

• One more hook to add on the fastify core

Plugin proposal using existent hooks

• The plugin, after being registered, will automatically add a new hook preHandler that will validate the route parameters before executing the route.
  E.g. if I call GET /artists/123 during the preHandler the plugin will check if the /artists/:id will match a :id parameter inside the schema

Advantages

• No need to implement anything on the core
• Easy enough to use and the error will be meaningful for the user
• Only the not valid routes will throw while other ones will still keep working

Cons

• The route will be validated only once called, not right after registration.
• The server will boot even if invalid routes

[Uzlopak]

We have already onRoute hook, which is triggered when a route is added.

[sf3ris]

Ouch, you're right. Thanks for the quick reply and the insight.
Then I think option one using the onRoute hook would fit the abovementioned perfectly.

What you think giving it a shot? Do you have any other community plugins that are responsible for validating routes just check them out?

[sf3ris]

@Uzlopak I tried to create the abovementioned plugin but I've encountered an issue, the onRoute hook is called before the actual registration.
In fact, doing the following steps will return null inside the hook.

fastify.addHook('onRoute', (routeOpts) => {
    const {method, url} = routeOpts.
    const route = fastify.findRoute({method, url});
    console.log(route); // null
})

fastify.get('/index', (req, reply) => reply.send("OK"));

Any specific reason why the hooks are called here and the actual registration happens here ?

To overcome this without touching the core, I could parse the routeOptions.url and matching against routeOptions.schema.params but the fmw route object returned from the fastify.findRoute is more reliable and already has the parsed params on the object.

[Uzlopak]

I think it would be better to extract the parameter determination functionality in find-my-way

https://github.com/delvedor/find-my-way/blob/a4d92627b818b42cb682fb2c808e1ca694b215f5/index.js#L171

and then use it.

But I wonder what you can do with regex values for route?!

[sf3ris]

But I wonder what you can do with regex values for route?!

The reported issue here was that there are no checks if the registered params inside the route is the same as the one inside the declared schema.
E.g.

fastify.get('/artists/:artistId', {
   schema: { params: { id: { type: 'integer' } } }
}, handler);

In this case the application would start correctly but hiding silent errors due to mismatch between declared params inside URL and the one declared in the schema.

The plugin to me, should check that the parsed params are the same as the ones declared inside the schema.
Does it make sense to you?

I think it would be better to extract the parameter determination functionality in find-my-way

You mean exposing the functionality from fmw that will be exposed from fastify core (to avoid dependency leaks)?

[metcoder95]

Hmm, I think we are going down the rabbit hole; setting fmw aside to make the plugin fit the use case will be slightly more problematic than we thought.

Have you considered doing the registration by yourself using fwm?
i.e. the onRoute hook intercepts the registering, registers the route against a custom instance of fmw, obtains the route definition, does the validation and throws if mismatches.

You can also use onReady to throw away the fwm instance to avoid having dangling objects so it can be gc'ed.

[mcollina]

Any specific reason why the hooks are called here and the actual registration happens here ?

Yes.

---

You don't need to call fmw. You have all the schema inside routeOptions.

[sf3ris]

You don't need to call fmw. You have all the schema inside routeOptions.

Yes, you have the schema but you don't have the parsed params from the route path.
For example, the fmw route object has the following fields that make the checks easier

{ 
    schema: { params: { id: 'integer' } },
    params: { id: ':id' },
   ...otherFmwRouteFields
}

Hmm, I think we are going down the rabbit hole; setting fmw aside to make the plugin fit the use case will be slightly more problematic than we thought.

I totally agree with this and I think is getting much more complicated than the problem it tries to solve.
Just wanted to give it a shot to close the loop of the reported issue but for me is totally fine to keep it as it is.

[climba03003]

Wouldn't it as simple as the following check?

fastify.addHook('onRoute', function(option) {
  // check only when schema provided
  if(option.schema && option.schema.params) {
    for(const param of Object.keys(option.schema.params)) {
      // here is simple, named param actually always start with colon and follow the param name
      // so, what we need to do is check by includes
      if(!option.path.includes(`:${param}`)) throw Error(`${option.path} does not include param ${param}`)
    }
  }
})

Even better for bidirectional check.

const COLON = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_'
function extractParamUrl (str) {
  const params = []
  let i, char
  let state = 'skip'
  let param = ''
  let level = 0
  // count for regex if no param exist
  let regexp = 0
  for (i = 0; i < str.length; i++) {
    char = str[i]
    switch (state) {
      case 'colon': {
        // we only accept a-zA-Z0-9_ in param
        if (COLON.indexOf(char) !== -1) {
          param += char
        } else if (char === '(') {
          state = 'regexp'
          level++
        } else {
          // end
          state = 'skip'
          params.push(param)
          param = ''
        }
        break
      }
      case 'regexp': {
        if (char === '(') {
          level++
        } else if (char === ')') {
          level--
        }
        // we end if the level reach zero
        if (level === 0) {
          state = 'skip'
          if (param === '') {
            // no idea how to handle non-named param
            param = String(regexp)
          }
          params.push(param)
          param = ''
        }
        break
      }
      default: {
        // we check if we need to change state
        if (char === ':' && str[i + 1] === ':') {
          // double colon -> single colon
          path += char
          // skip one more
          i++
        } else if (char === ':') {
          // single colon -> state colon
          state = 'colon'
        } else if (char === '(') {
          state = 'regexp'
          level++
        } else if (char === '*') {
          // * -> {*}
          // should be exist once only
          params.push('*')
        }
      }
    }
  }
  // clean up
  if (state === 'colon' && param !== '') {
    params.push(param)
  }
  return params
}

fastify.addHook('onRoute', function(option) {
  // check only when schema provided
  if(option.schema && option.schema.params) {
    const urlParams = extractParamUrl(option.path)
    const schemaParams = Object.keys(option.schema.params)
    for(const param of schemaParams ) {
      if(!urlParams.includes(param)) throw Error(`${option.path} does not include param ${param}`)
    }
    for(const param of urlParams) {
      if(!schemaParams.includes(param)) throw Error(`${param} does not include schema but exist inside path ${option.path}`)
    }
    
  }
})

[sf3ris]

Yes, this was exactly what we proposed along the thread but I fear that there will be a dependency leak between our param extraction and the one used by fmw internally.
Doing the parsing by ourselves will work and "probably" we would never introduce breaking changes, but a change/fix on the fmw parameter parsing (the real one then used to retrieve the value from a named parameter) could break projects using this validator.

To me, to create a stable plugin, the params parsed from the fmw should be exactly the same that we use to check against the schema.
Do you think could be an option to:

• add the fmw param extraction to the fmw interface
• when creating the routeOptions for the onRoute event, adding the parsed params to the object?
  e.g.

       const routeOptions = {
           ...routeOptions,
           params: fmw.extractParams(path)
       }

But, if you think could be worth to implement another extraction algorithm I can try to write a plugin to solve the issue 🙌

[climba03003]

@mcollina
I though about it a while, but #5230 give too much power to the users.
It provide the whole route information including even the context within fastify.
It also means that the read-only of context will be useless after this PR.
Follow up in #5253

We need to re-considerate should be we expose all the information on route and hidden field on fastify.

Anyway, @sf3ris you can achieve it by

const kRoutes = Symbol('routes')
fastify.decorate(kRoutes, [])

fastify.addHook('onRoute', function(options) {
  fastify[kRoutes].push(options)
})

fastify.addHook('onReady', function() {
  // we can now check each routes
  for(const option of fastify[kRoutes]) {
    const schema = option.schema
    const route = fastify.findRoute({
      url: option.url,
      method: option.method,
      constraints: option.constraints
    })
    if(schema && schema.params) {
      // we check only when params schema exists
      const schemaParams = Object.keys(schema.params.properties)
      const urlParams = Object.keys(route.params)
      for(const param of schemaParams ) {
        if(!urlParams.includes(param)) throw Error(`${option.path} does not include param ${param}`)
      }
      for(const param of urlParams) {
        if(!schemaParams.includes(param)) throw Error(`${param} does not include schema but exist inside path ${option.path}`)
      }
    }
  }
})