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

Updated the docs related to AJV plugins #3189

Merged
merged 5 commits into from Oct 20, 2021

Conversation

amis-shokoohi
Copy link
Contributor

  • Added a note for ajv-errors plugin
  • Updated the link to AJV options page

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Jul 11, 2021
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

docs/Server.md Outdated Show resolved Hide resolved
Comment on lines 604 to 613
Below is an example showing how to add **custom error messages for each property** of a schema by supplying custom AJV options.
Inline comments in the schema below describe how to configure it to show a different error message for each case:

```js
const fastify = Fastify({
ajv: {
customOptions: { jsonPointers: true },
customOptions: {
jsonPointers: true,
allErrors: true
},
Copy link
Member

Choose a reason for hiding this comment

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

Oof. We need to update this example with a warning that implementing it opens the service up to a denial of service attack.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I just add the warning as a comment in front of the option:

customOptions: {
  jsonPointers: true,
  allErrors: true // Warning: Enabling this option may open the service up to a denial of service attack
},

Copy link
Member

Choose a reason for hiding this comment

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

I would add a note box with the link to the CVE
https://www.cvedetails.com/cve/CVE-2020-8192/

@mcollina
Copy link
Member

What's the status of this? Should we land some of this or should we close for now?

Copy link
Member

@Eomm Eomm left a comment

Choose a reason for hiding this comment

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

The suggestion should be addressed because the proposed link, points to ajv8 and it confuses users

amis-shokoohi and others added 2 commits September 12, 2021 13:02
Co-authored-by: Matteo Collina <matteo.collina@gmail.com>
Co-authored-by: Manuel Spigolon <behemoth89@gmail.com>
Co-authored-by: Manuel Spigolon <behemoth89@gmail.com>
@mcollina mcollina merged commit 505780b into fastify:main Oct 20, 2021
@github-actions
Copy link

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants