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

docs: warn users about bad practice #1094

Merged
merged 1 commit into from Sep 28, 2021

Conversation

hariprasadkc
Copy link
Contributor

@hariprasadkc hariprasadkc commented Sep 28, 2021

Description

Fixes #1087
https://express-validator.github.io/docs/custom-validators-sanitizers.html
The above documentation promotes a couple of bad practices that might affect the quality of the documentation.

  • Illustration of accessing data layer directly from validation layer
  • Promoting the usage of unvalidated data

@fedeci fedeci changed the title #1087 - Examples promoting bad practice docs: warn users about bad practice Sep 28, 2021
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling e9af187 on hariprasadkc:master into 05a905a on express-validator:master.

Copy link
Member

@fedeci fedeci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

### Example: checking if password confirmation matches password

```js
const { body } = require('express-validator');

app.post(
'/user',
body('password').isLength({ min: 5 }),
Copy link
Member

@fedeci fedeci Sep 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't add that more layer of security because the custom validator will run even if the previous validators failed. We should probably change this behaviour in v7. (ref #990)

@fedeci fedeci merged commit 94d5426 into express-validator:master Sep 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs have an example that promotes bad practice
3 participants