working-plusplus icon indicating copy to clipboard operation
working-plusplus copied to clipboard

Fix sql injection and add multiple mentions

Open Alex-Vol-SV opened this issue 5 years ago • 4 comments

This combined PR adds the fix to allow multiple mentions in the same message to work for plus/minus actions. I do not find a good reason why other kinds of actions should be combined.

The handleSelfPlus code was merged into the handlePlusMinus code as it made the operation able to use with other plus/minus operations.

The usage did not change, all that happens is each ++/-- will be added as a line in a multiline message where before only the very first ++/-- would be considered.

See my other PR for some comments relating to the SQL injection fix.

This fixes issue #8

Alex-Vol-SV avatar Dec 23 '19 20:12 Alex-Vol-SV

Hi @Alex-Vol-SV,

Just wanted to drop in quickly to thank you very much for your PRs. I’m currently travelling for an extended period so I’m not going to have a chance to review these for at least the next couple of months, just didn’t want you to not hear anything at all for that long! I’ll let you know once I’ve been able to have a proper look.

tdmalone avatar Dec 23 '19 20:12 tdmalone

@tdmalone I may take a stab at fixing your .travis-postgres10.sh script that seems to fail to correctly upgrade postgresql 10 and caused the PR builds to fail as a result.

Alex-Vol-SV avatar Dec 23 '19 20:12 Alex-Vol-SV

Pull Request Test Coverage Report for Build 127

  • 26 of 26 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.5%) to 72.436%

Totals Coverage Status
Change from base Build 89: 0.5%
Covered Lines: 262
Relevant Lines: 337

💛 - Coveralls

coveralls avatar Dec 23 '19 21:12 coveralls

I was able to fix the issues with the new base images. It was simple once I realized the problem was that PG 10 is now properly supported.

I created 3 PR with each building on the previous one, this one combines the commits from the other two. Feel free to merge this and ignore the other two or just merge one at a time starting with the Travis fix first, then the SQL injection and finally this one.

If you prefer three separate PR for each issue I can break it up but until master has the Travis CI configuration fixes the other PR fail t build.

Alex-Vol-SV avatar Dec 23 '19 22:12 Alex-Vol-SV