#30199: tor-android-service: Review 2019/04/16 -------------------------------------------------+------------------------- Reporter: sysrqb | Owner: tbb- | team Type: defect | Status: new Priority: Medium | Milestone: Component: Applications/Tor Browser | Version: Severity: Normal | Resolution: Keywords: tbb-mobile, tbb-8.5, | Actual Points: TorBrowserTeam201906 | Parent ID: | Points: Reviewer: | Sponsor: -------------------------------------------------+-------------------------
Comment (by sisbell): I wasn't able to follow the exact commits in the descriptions (I can't seem to locate the hashes) but I have a general idea of what some of them are addressing: Replying to [ticket:30199 sysrqb]: > Overall, nicely done. This review is on master (`6a9314aff4418a4edac33ff39fae266b097cf000`): > > the initial import (`36f9873ff075253f4c1c9e394c91031fd4ba9d4a`), do you know the git hash of the orbot commit you used? The original branch point was from commit: 8ad7668013a7291b98cf8689cd99961db69f8ed3 > > `1d635a925ca1728542067ee7bf34ff532d623a3f` - The renaming is Tor Browser specific, so we should probably carry this as a patch in tor-browser- build, instead of hard coding "Tor Browser" within a general purpose "Tor service" library, right? I did update the resources to be the latest ones in Orbot. We will need to provide a patch to add 'Tor Browser' text now. > > `45244c49fbe382e97655b8f8d8f482e54f95ed07` - I wonder if we should carry a patch for this too. If tor-android-services+TOPL is intended as a general Android Tor library, then we probably shouldn't make it Tor Browser specific. > > `f3b3df4e66630a68c9987d52d93580549a0acaf4` - this package doesn't exist yet, but it will exist in a few weeks, so leaving it is probably okay. > > `1a2121e2a7ee8a6c0f3b5edd150b38eaae8bef304` - hasCookieAuthentication() and runAsDaemon() seem out of place in this commit I was not able to locate the 3 commits above to review > > `784919d8eb19083cf761b3e7314c49d8befc00cd` - custom tor, the binaries should be compiled as dependencies by tor-browser-build and injected into the build, TorService.TOR_VERSION should be patched at build time I'll open an issue for this. I think a good approach would be to place the version in a property file rather than using static fields. Orbot handles things a little differently now as well and pulls a binary artifact that contains the correct version in the meta-data. > > `6a9314aff4418a4edac33ff39fae266b097cf000` - In the future, we should take the bridges used on desktop (currently specified in Firefox's pref format and injected at build time by tor-browser-build), and pre-process them for inclusion on Android, too. I did see sysrqb added this in commit d9b049c8cee225b4e7bb6f0191093f543d0f9f65 -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/30199#comment:8> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online
_______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs