search

mozilla / fxa-auth-server

DEPRECATED - Migrated to https://github.com/mozilla/fxa
Mozilla Public License 2.0
399 stars 121 forks source link

refactor(fxa-auth-server): Added semicolons #2986

Closed hritvi closed 5 years ago

hritvi commented 5 years ago

fixes #2953.

Commands used to fix: eslint --fix .

Also added complexity: 0 and no-useless-escape: 0 because some errors were occuring due to the use of regex.

Have also added //eslint disable-next-line in validators.js, authorization.js, token.js and added /*eslint complexity: [2, 11] */ in summary.js manually because the errors no-useless-escape and complexity were still present.

Please review @shane-tomlinson.

hritvi commented 5 years ago

As suggested here, versions js-yaml prior to 3.13.0 are vulnerable to Denial of Service. By parsing a carefully-crafted YAML file, the node process stalls and may exhaust system resources leading to a Denial of Service.

Therefore upgrading the js-yaml in fxa-content-server-l10n/package-lock.json to 3.13.0 might resolve the error. May I proceed with this?

vladikoff commented 5 years ago

@hritvi see #2984

hritvi commented 5 years ago

So I suppose with the merging of that PR, the tests would also run correctly.

hritvi commented 5 years ago

As #2984 is now merged, could you please re-run the tests @vladikoff. Thanks :slightly_smiling_face:

philbooth commented 5 years ago

@hritvi, you can rebase your branch to get it to include latest changes from master. Either in one step with git pull --rebase master, or you can pull master separately and then just git rebase master instead.

hritvi commented 5 years ago

@philbooth could you please review it now, as the tests have passed.

philbooth commented 5 years ago

Just to continue the conversation we were having in IRC...

Seeing as this PR touches 23,000 lines and we're taking the hit for everything in git blame, we may as well use this opportunity to do other things that eslint --fix can do for us. Let's do each one in a separate commit, all in this PR. Off the top of my head, how about:

?

philbooth commented 5 years ago

prefer-destructuring

@hritvi, fyi I've been playing with these locally and prefer-destructuring does not apply cleanly, so it's probably easier if you leave that one out. The others all worked well here though.

hritvi commented 5 years ago

@philbooth could you please review it now. I have added all the eslint fixes.

dannycoates commented 5 years ago

🙈

vladikoff commented 5 years ago

yay2

Semicolons finally won!