Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contextsSymbol prevents proper encapsulation of middleware #813

Closed
maxarndt opened this issue Dec 16, 2019 · 4 comments
Closed

contextsSymbol prevents proper encapsulation of middleware #813

maxarndt opened this issue Dec 16, 2019 · 4 comments
Labels

Comments

@maxarndt
Copy link

maxarndt commented Dec 16, 2019

I wrote an npm package to reuse my express validation code.

This package allows me to inject error handling as a middleware into my express app using the .use() method.
The validation for arbitrary routes can be called as shown in the following code snippet of my router:

const SampleController = require('./controller')
const { check } = require('express-validator')
const { throwIfFails, validators } = require('@maxarndt/some-package')

module.exports = function (router) {
  router.post('/testOriginalValidators', throwIfFails(check('username').isEmail(), check('something').isAlphanumeric()), SampleController.testHandler)
  router.post('/testCustomValidator', throwIfFails(check('deviceId').custom(validators.deviceId)), SampleController.testHandler)
}

@maxarndt/some-package looks like the following:

const { validationResult } = require('express-validator')
const ValidationError = require('./ValidationError')
const formatError = require('./errorFormatter')

module.exports.validationErrorHandler = function (err, req, res, next) {
  if (err instanceof ValidationError) {
    const formattedError = formatError(err)
    return res.status(formattedError.statusCode).send(formattedError.error)
  }
  next(err)
}

module.exports.throwIfFails = function (...checks) {
  return [...checks, throwIfError]
}

function throwIfError (req, res, next) {
  try {
    validationResult(req).throw()
    return next()
  } catch (error) {
    throw new ValidationError('Validation error', error.errors)
  }
}

Problem

While this works like a charm in my component tests in the package project it fails when I use this npm package in another application.
The validationResult(req) function does not extract any errors because the contextsSymbol does not match. (Link to the line in validation-result.ts)

My problem seems to be, that I require express-validator@6.3.0 in my private package and additionally in the application which uses my own npm package.

I can avoid this error by exporting your check function and using this one. But I didn't want to wrap your whole module.

Is this an intended behavior? And if that's the case why is it necessary to use such a symbol?
I would have expected the validation to work properly.

@gustavohenke
Copy link
Member

Hey!
That's definitely not an intended problem. I just never really thought of it being used like this.

I guess reverting back to string keys should make it.
In the mean time, you can try deduping the packages to fix the issue.

@gustavohenke
Copy link
Member

...well, I actually fixed it already. v6.3.1 is out.

@GerbenRampaart
Copy link

Lol, we encountered this bug when we decided to modularize our header validation literally days ago. Talk about good timing.

@lock
Copy link

lock bot commented Mar 17, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants