adonisjs / validator

Schema based validator for AdonisJS
MIT License
116 stars 39 forks source link

Turning on single email sanitizer turns them all on #133

Closed McSneaky closed 2 years ago

McSneaky commented 2 years ago

Package version

12.2.0

Node.js and npm version

Node: 17.1.0 npm: 8.1.2

Sample Code (to reproduce the issue)

Request payload

{
  "email": "Foo.Bar+TEST@Example.com"
}

Validator

const validated = await request.validate({
  schema: schema.create({
    email: schema.string({ trim: true }, [
      rules.email({
        sanitize: {
          lowerCase: true,
        },
      }),
    ]),
  }),
})
console.log(validated)

Expected outcome:

{ email: 'foo.bar+test@example.com' }

Actual outcome

{ email: 'foobar@example.com' }

I would expect current outcome, when sanitize is set to just true (sanitize: true) or when all keys are all set to true

Currently both

sanitize: {
  lowerCase: true,
  removeDots: true,
  removeSubaddress: true,
},

and

sanitize: {
  lowerCase: true
},

give exactly the same result

Not sure if it's like that by design? I'd kinda expect all the keys to be false by default

Willing to file in PR, to fix it, but since I'm not sure if it's bug or feature didn't do PR first 🙂

BONUS (a sample repo to reproduce the issue)

thetutlage commented 2 years ago

Yeah, makes sense to have them set to false. However, it will be a breaking change now, so can only be released in the next major release

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

thetutlage commented 2 years ago

I am going to extract the normalization of the email to its own rule and deprecate the sanitize option all together. This is how it will look in practice.

email: schema.string([
  rules.email(),
  rules.normalizeEmail({
    // pass options here
  })
])

If you decide not to pass any options, then the defaults from validator.js will be used. In other words, AdonisJS will not turn on/off any options

thetutlage commented 2 years ago

Added here. https://github.com/adonisjs/validator/commit/807e10ef62338afc251a08df0d861493ef53343f

ryanadhi commented 2 years ago

@thetutlage does this improvement make the rules.email function deprecated? because I receive deprecation warning when I am using rules.email

thetutlage commented 2 years ago

@ryanadhi You can learn more about the deprecation in the release notes. https://docs.adonisjs.com/releases/april-2022-release#email-sanitize

ryanadhi commented 2 years ago

@ryanadhi You can learn more about the deprecation in the release notes. https://docs.adonisjs.com/releases/april-2022-release#email-sanitize

does this also remove dots before the '@' ? some emails has dots before '@'