Further work on message validation #1758

Kaz-ookid · 2024-02-26T11:02:59Z

Addressing #1751

This PR will hold new message data object validation, to help the effort of complete message validation depicted in #1751.

Note to myself: remember to update the list of validated data in #1751, after this PR gets approved.

CLAassistant · 2024-02-26T11:03:05Z

All committers have signed the CLA.

github-actions · 2024-02-26T11:03:15Z

Pull reviewers stats

Stats of the last 30 days for popstellar:

User	Total reviews	Time to review	Total comments
pierluca 🥇	6 ▀▀▀▀	6d 7h 35m ▀▀▀▀▀▀	10 ▀▀▀▀▀▀▀
MariemBaccari 🥈	4 ▀▀▀	1d 17h 15m ▀▀	1 ▀
jbsv 🥉	2 ▀	1d 7h 33m ▀	0
sgueissa	1 ▀	1h 17m	3 ▀▀
ljankoschek	1 ▀	20h 9m ▀	0
K1li4nL	1 ▀	18m	0
emonnin-epfl	1 ▀	6h 45m	1 ▀

…ng valid format

…alidations in ElectionQuestion

ElectionQuestion : added validation at Question creation + added tests

Kaz-ookid · 2024-02-29T15:41:51Z

Hint for reviewers:

The construction of a Question is now validated. I also am checking the question handed in the ElectionQuestion for the exact same constraints. This means we are validating the question twice, thus reducing coverage (since we can't construct any invalid question to hand to the ElectionQuestion constructor). It may not be needed to validate the question in ElectionQuestion, but I stil did just in case. I am open to suggestion about it.

matteosz

Really good work! Just some picky comments

...a/com/github/dedis/popstellar/model/network/method/message/data/election/ElectionQuestion.kt

fe2-android/app/src/main/java/com/github/dedis/popstellar/utility/MessageValidator.kt

.../java/com/github/dedis/popstellar/model/network/method/message/data/election/CastVoteTest.kt

…ed inputs CastVoteTest : actually using defined vote variables to avoid hardcoded inputs

Kaz-ookid · 2024-03-03T17:34:16Z

Thanks a lot @matteosz for your insightful review 👍

matteosz · 2024-03-03T23:04:17Z

Just a reminder also for future PRs (you can skip it for this one): try to keep commits number as low as possible.
I just saw you did a commit formatting and then runktmftm Format, plus probably 16 commits for a 300 lines change is a bit too much (but it's just my subjective opinion obv).

Commits are a great mechanism for tracking history, but if multiple commits have the same purpose this simply increases the entropy of the codebase and "obfuscate" a good commit history. So try to squash together all the commits that serve to the same purpose.

Simply run git rebase -i HEAD~<number_of_commits> and then manually go through commits and decide whether to squash or keep them. Keep in mind that if you squash commits already pushed you'd need to force push, so be careful about it.

matteosz · 2024-03-03T23:08:26Z

...a/com/github/dedis/popstellar/model/network/method/message/data/election/ElectionQuestion.kt

+  init {
+    verify().isNotEmptyBase64(electionId, "election ID").validQuestions(listOf(question))


Saw that you did a commit named ElectionQuestion : removed overlapping Question validations but it's actually an empty commit (just some formatting), so I guess you forgot to remove the validQuestions check here

I actually removed the redundant checks performed in validQuestions, line 245 of MessageValidator (calling validQuestion for each Question in the list). validQuestions is still used, as it checks for duplicates and requires the list not to be empty. Maybe it makes it not relevant anymore to be a function in MessageValidator tho.

Yeah the point was: why are you still calling .validQuestions(listOf(question)) in ElectionQuestion? The fact that the list is not empty and without duplicates is given by the construction of it (a list of 1 element)

oh yes that makes a lot of sense, I overlooked

…ed inputs CastVoteTest : actually using defined vote variables to avoid hardcoded inputs

formatting

…on' into work-fe2-maxime-message-validation

quadcopterman

Seems OK to me

matteosz

LGTM!

sonarcloud · 2024-03-05T15:29:35Z

Quality Gate passed for 'PoP - PoPCHA-Web-Client'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

sonarcloud · 2024-03-05T15:31:29Z

Quality Gate passed for 'PoP - Be1-Go'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

sonarcloud · 2024-03-05T15:31:52Z

Quality Gate passed for 'PoP - Be2-Scala'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

sonarcloud · 2024-03-05T15:36:47Z

Quality Gate passed for 'PoP - Fe2-Android'

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
98.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

sonarcloud · 2024-03-05T15:36:58Z

Quality Gate passed for 'PoP - Fe1-Web'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Kaz-ookid added 2 commits February 23, 2024 21:50

Added isValidWriteIn in MessageValidator.kt

9df081e

EncryptedVote validations

01ea364

Kaz-ookid added 6 commits February 26, 2024 15:48

EncryptedVote validations : Previous tests now using valid formats

5c476b7

EncryptedVote : added tests for validations

67d5d15

EncryptedVote : added tests for validations + More previous tests usi…

ed2baf8

…ng valid format

ElectionQuestion : added validQuestions in MessageValidator + added v…

f5d56d6

…alidations in ElectionQuestion

fixed format

151cf30

ElectionQuestion : added validation at Question creation + added tests

07d495b

ElectionQuestion : added validation at Question creation + added tests

matteosz added the fe2-android label Feb 27, 2024

Kaz-ookid self-assigned this Feb 29, 2024

Kaz-ookid marked this pull request as ready for review February 29, 2024 15:32

Kaz-ookid requested a review from a team as a code owner February 29, 2024 15:32

Kaz-ookid requested a review from quadcopterman February 29, 2024 15:32

Kaz-ookid linked an issue Feb 29, 2024 that may be closed by this pull request

Complete message validation #1751

Open

54 tasks

Kaz-ookid removed a link to an issue Feb 29, 2024

Complete message validation #1751

Open

54 tasks

matteosz requested changes Mar 3, 2024

View reviewed changes

Kaz-ookid added 6 commits March 3, 2024 18:16

ElectionQuestion : removed overlapping Question validations

e83a468

ElectionQuestion : fixed wrong hashCode concatenation

65a7caf

MessageValidator : removed unnecessary function isValidWriteIn

f88cfaa

MessageValidator : moved validVotingMethods as a static field

fed6cb7

MessageValidator : forEach instead of useless map use

7bbf22c

CastVoteTest : actually using defined vote variables to avoid hardcod…

9768d91

…ed inputs CastVoteTest : actually using defined vote variables to avoid hardcoded inputs

Kaz-ookid added 2 commits March 3, 2024 18:37

formatting

fed176f

formatting : run ktfmtFormat

a819878

matteosz self-requested a review March 3, 2024 23:07

matteosz reviewed Mar 3, 2024

View reviewed changes

Kaz-ookid added 5 commits March 4, 2024 15:33

ElectionQuestion : Addressing Matteo's comments

eb92351

MessageValidator : Addressing Matteo's comments

2aed118

CastVoteTest : actually using defined vote variables to avoid hardcod…

a7c0acb

…ed inputs CastVoteTest : actually using defined vote variables to avoid hardcoded inputs

formatting : run ktfmtFormat

b41f82f

formatting

Merge remote-tracking branch 'origin/work-fe2-maxime-message-validati…

a7e5a5f

…on' into work-fe2-maxime-message-validation

quadcopterman previously approved these changes Mar 4, 2024

View reviewed changes

ElectionQuestion : removed useless validation function (validQuestions)

e2d820d

Kaz-ookid dismissed quadcopterman’s stale review via e2d820d March 4, 2024 18:34

matteosz self-requested a review March 5, 2024 09:23

matteosz previously approved these changes Mar 5, 2024

View reviewed changes

Merge branch 'master' into work-fe2-maxime-message-validation

ef01bc7

Kaz-ookid dismissed matteosz’s stale review via ef01bc7 March 5, 2024 15:27

matteosz self-requested a review March 5, 2024 17:25

matteosz approved these changes Mar 5, 2024

View reviewed changes

Kaz-ookid merged commit ec20ed0 into master Mar 5, 2024
18 checks passed

Kaz-ookid deleted the work-fe2-maxime-message-validation branch March 5, 2024 17:45

matteosz mentioned this pull request Apr 29, 2024

First parts of UI for the federation project on FE2 #1810

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Further work on message validation #1758

Further work on message validation #1758

Kaz-ookid commented Feb 26, 2024 •

edited

Loading

CLAassistant commented Feb 26, 2024 •

edited

Loading

github-actions bot commented Feb 26, 2024

Kaz-ookid commented Feb 29, 2024

matteosz left a comment

Kaz-ookid commented Mar 3, 2024

matteosz commented Mar 3, 2024

matteosz Mar 3, 2024

Kaz-ookid Mar 4, 2024 •

edited

Loading

matteosz Mar 4, 2024

Kaz-ookid Mar 4, 2024

quadcopterman left a comment

matteosz left a comment

sonarcloud bot commented Mar 5, 2024

sonarcloud bot commented Mar 5, 2024

sonarcloud bot commented Mar 5, 2024

sonarcloud bot commented Mar 5, 2024

sonarcloud bot commented Mar 5, 2024

		init {
		verify().isNotEmptyBase64(electionId, "election ID").validQuestions(listOf(question))

Further work on message validation #1758

Further work on message validation #1758

Conversation

Kaz-ookid commented Feb 26, 2024 • edited Loading

CLAassistant commented Feb 26, 2024 • edited Loading

github-actions bot commented Feb 26, 2024

Pull reviewers stats

Kaz-ookid commented Feb 29, 2024

matteosz left a comment

Choose a reason for hiding this comment

Kaz-ookid commented Mar 3, 2024

matteosz commented Mar 3, 2024

matteosz Mar 3, 2024

Choose a reason for hiding this comment

Kaz-ookid Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

matteosz Mar 4, 2024

Choose a reason for hiding this comment

Kaz-ookid Mar 4, 2024

Choose a reason for hiding this comment

quadcopterman left a comment

Choose a reason for hiding this comment

matteosz left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Mar 5, 2024

Quality Gate passed for 'PoP - PoPCHA-Web-Client'

sonarcloud bot commented Mar 5, 2024

Quality Gate passed for 'PoP - Be1-Go'

sonarcloud bot commented Mar 5, 2024

Quality Gate passed for 'PoP - Be2-Scala'

sonarcloud bot commented Mar 5, 2024

Quality Gate passed for 'PoP - Fe2-Android'

sonarcloud bot commented Mar 5, 2024

Quality Gate passed for 'PoP - Fe1-Web'

Kaz-ookid commented Feb 26, 2024 •

edited

Loading

CLAassistant commented Feb 26, 2024 •

edited

Loading

Kaz-ookid Mar 4, 2024 •

edited

Loading