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

Skip validation for certain path(s) in Document.validate() method #10216

Closed
tbhaxor opened this issue May 7, 2021 · 14 comments
Closed

Skip validation for certain path(s) in Document.validate() method #10216

tbhaxor opened this issue May 7, 2021 · 14 comments
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue
Milestone

Comments

@tbhaxor
Copy link
Contributor

tbhaxor commented May 7, 2021

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

Unable to skip or prevalidate or bypass the validation for a path on mongoose

If the current behavior is a bug, please provide the steps to reproduce.

  1. Create a model
  2. Add required validator on a path
  3. Run the doc.$markValid("path")
  4. Try doc.validate(["path"]).catch(e => console.log(Object.keys(e.errors).length))
  5. The length would be == 1

Here I want to bypass the required validation for gwId field
image

{
  "compilerOptions": {
    "target": "es5",
    "module": "commonjs",
    "outDir": "./dist",
    "strict": true,
    "noImplicitAny": true,
    "typeRoots": ["./ctypings", "./node_modules/@types"],
    "types": ["express", "express-session", "process", "express-request-id"],
    "esModuleInterop": true,
    "skipLibCheck": true,
    "forceConsistentCasingInFileNames": true
  },
  "exclude": ["node_modules", "./node_modules", "./node_modules/*", "./node_modules/@types/node/index.d.ts"],
  "include": ["src/**/*.ts", "ctypings/**/*.ts"]
}

What is the expected behaviour?

Well, $markValid should bypass all the validators for one or more paths (except unique)

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.

Mongoose: 5.12.7

Node: v14.16.0

Mongodb: 3.6.6

@tbhaxor
Copy link
Contributor Author

tbhaxor commented May 8, 2021

Any updates @mongoose team?

@IslandRhythms
Copy link
Collaborator

Thats not how $markModified works according to the docs
https://mongoosejs.com/docs/api/document.html#document_Document-markModified

@IslandRhythms IslandRhythms added the needs clarification This issue doesn't have enough information to be actionable. Close after 14 days of inactivity label May 10, 2021
@tbhaxor
Copy link
Contributor Author

tbhaxor commented May 10, 2021

@IslandRhythms Please read my issue again

I am talking about $markValid not $markModified. Also $markModified is not any function, it's markModified

@IslandRhythms
Copy link
Collaborator

There is a discrepancy between the steps you list to reproduce the issue and the image you have provided. You are using $markModified in the steps but $markValid in the image. Please clear that up.

@tbhaxor
Copy link
Contributor Author

tbhaxor commented May 10, 2021

@IslandRhythms Done, and apologies for that.

Could you now look into this issue?

@IslandRhythms IslandRhythms added confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue and removed needs clarification This issue doesn't have enough information to be actionable. Close after 14 days of inactivity labels May 10, 2021
@IslandRhythms
Copy link
Collaborator

const mongoose = require('mongoose');
const {Schema} = mongoose;

const testSchema = new Schema({
    name: {
        type: String, 
        required: true
    },
    nickname: {
        type: String
    }
});
    const Test = new mongoose.model('Test', testSchema);

    async function run() {
        await mongoose.connect('mongodb://localhost:27017/test', {
            useNewUrlParser: true,
            useUnifiedTopology: true
          });
        
        await mongoose.connection.dropDatabase();

        const entry = await Test.create({nickname: 'Test'});
        entry.$markValid('name');
        await entry.validate('name').catch(e=> console.log('Yo', Object.keys(e.errors).length));
    }
    run();

@vkarpov15 vkarpov15 added this to the 5.12.9 milestone May 10, 2021
@tbhaxor
Copy link
Contributor Author

tbhaxor commented May 10, 2021

@IslandRhythms Is there any changelog for each mongoose release?

@vkarpov15
Copy link
Collaborator

@vkarpov15
Copy link
Collaborator

Currently this is by design, because $markValid() clears existing errors, but Mongoose doesn't run required validators until you call validate().

You should be able to set the validateModifiedOnly option, which skips paths that haven't been explicitly set:

        const entry = new Test({nickname: 'Test'});
        await entry.validate({ validateModifiedOnly: true });

The above won't quite work, we'll need to support passing options as the first parameter to validate(). And we'll add the ability to exclude paths from validation in a future minor release.

@tbhaxor
Copy link
Contributor Author

tbhaxor commented May 11, 2021

@vkarpov15 Also I have seen mongoose doesn't sanitize queries and executes malicious payloads. I have explained in this article

If you guys will help me in understanding file structure and repo (contribution guide for developers), then may be I can open few Pull requests for opened issues

@vkarpov15
Copy link
Collaborator

I'm currently working on a series of blog posts about that very subject :) First one is here: http://thecodebarbarian.com/mongoose-internals-schemas-options-models.html .

Re: sanitizing query filters, we're working on that for Mongoose 6.0. Follow #3944 for updates.

@tbhaxor
Copy link
Contributor Author

tbhaxor commented May 11, 2021

I'm currently working on a series of blog posts about that very subject :) First one is here: http://thecodebarbarian.com/mongoose-internals-schemas-options-models.html .

Re: sanitizing query filters, we're working on that for Mongoose 6.0. Follow #3944 for updates.

Nice blog, @vkarpov15 . I need more posts to be confident about sending quality PR's to the repo. How can I subscribe to the blog?

@vkarpov15
Copy link
Collaborator

RSS: http://thecodebarbarian.com/feed.xml, or my Twitter: https://twitter.com/code_barbarian . There's only one post so far, so I would recommend tinkering with the code and starting with a small bug or feature if you want to contribute.

One good place to start would be #10230, which is closely related to the problem you're trying to solve with this issue and should be about the right level of complexity for a first contribution.

@vkarpov15
Copy link
Collaborator

I'm going to close this issue for now, we made a workaround for 5.12.9, and #10230 will give us a better approach for 5.13.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue
Projects
None yet
Development

No branches or pull requests

3 participants