Skip to content
This repository has been archived by the owner on Jan 30, 2023. It is now read-only.

This Firefox-specific browser test was moved to a unit test in https:… #39

Merged
merged 1 commit into from
Sep 17, 2018

Conversation

rhelmer
Copy link
Contributor

@rhelmer rhelmer commented Sep 17, 2018

…//bugzilla.mozilla.org/show_bug.cgi?id=1485620 so we can remove it now

@henrycg would you mind making sure I didn't miss anything in the conversion to the gtest ^? Thanks!

@rhelmer
Copy link
Contributor Author

rhelmer commented Sep 17, 2018

Carrying over the conversation in https://phabricator.services.mozilla.com/D6049#136547:

It might be slightly cleaner to move all of the server-side validation logic in TestPrioEncoder.cpp into libprio, since the server-side logic is not what we want to test here. (Also, if we end up having to tweak the server-side APIs down the road, it would be nice if we didn't have to change the browser tests too.)

If you think that this would be a good idea, we would implement a test function in libprio that takes as input:

the PrioConfig,
the two server private keys, and
the two ciphertexts generated by the browser.

The test function would run the server-side validation logic and would then spit out the plaintext data as an array of booleans. Then in TestPrioEncoder.cpp, we would just create the two keypairs, generate the ciphertext using PrioEncoder.Encode, use a single call to the test function to decode the ciphertexts, and check that the recovered plaintext array matches the array given to PrioEncoder.

If that makes sense to you, I can put together a patch for libprio that implements this function.

I have mixed feelings - this doesn't seem like too much to me and it's well-commented so might be nice to have as an example, but having a higher-level API in libprio would be nice too

Copy link
Collaborator

@henrycg henrycg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@henrycg
Copy link
Collaborator

henrycg commented Sep 17, 2018

Let's leave it as is then. Getting browser-test out of libprio is great and if we end up needing to change the server-side API later on, we can always revisit this.

@rhelmer rhelmer merged commit 52a3348 into mozilla:master Sep 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants