Skip to content

Conversation

@malmstein
Copy link
Contributor

@malmstein malmstein commented Mar 3, 2021

Task/Issue URL: https://app.asana.com/0/1157893581871903/1199944591737638

Description:
F-Droid has flagged https_mobile_v2_bloom.bin in their build system. We can't have an embedded binary as part of the apk we send them.

This means that, anyone installing the app from F-Droid won't get the initial Http to Https upgrade until the data is downloaded in the background.

Play Store users will still get the initial protection the binary brings.

To solve this, we are creating a separate HttpsDataPersister for the FDroid flavour, which will not store the upgrades embedded in the apk.


Internal references:

Software Engineering Expectations
Technical Design Template

@malmstein malmstein requested a review from subsymbolic March 3, 2021 16:37
@malmstein
Copy link
Contributor Author

@subsymbolic can you take a look at this when you have the chance? Let me know if I'm missing something. Installing both variants has worked for me and all https upgrades happen as expected

@subsymbolic
Copy link
Contributor

I like your approach here! I wonder if we could split the logic at a more relevant spot though to avoid duplicating code, as it would be easy for someone to update the main implementation and forget about the FDroid one. So rather than duplicating the HttpsDataPersister class which has a bunch of logic that we want in both flavors, could we create a new HttpsEmbeddedDataManager (or whatever name works) with just the persistEmbeddedData method and have that be the item that gets two flavor implementations?

Does that make sense?

@malmstein
Copy link
Contributor Author

Oh that's actually a great idea @subsymbolic! Makes sense that the only difference is the embedding part, so I'll make that the variant between FDroid and Play.

@malmstein
Copy link
Contributor Author

@subsymbolic I think this looks cleaner now, let me know what you think

@malmstein malmstein changed the title added a httpsdatapersister that does not persist embedded data for froid Moved bloom filter binary to Play flavor Mar 4, 2021
Copy link
Contributor

@subsymbolic subsymbolic left a comment

Choose a reason for hiding this comment

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

Great work @malmstein. I added a couple of log statement suggestions. No need to rereview when they're done, just merge when you're ready.

@malmstein malmstein merged commit a13c3e2 into develop Mar 8, 2021
@malmstein malmstein deleted the feature/david/fdroid-bloom-filter-removal branch March 8, 2021 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants