Closed ghost closed 7 years ago
Nice. Seems they decided to make it free software again.
Trying to remember how my emoji generator works :+1: will send a PR probably if and when I do :)
Anything else needs to be done here? Anything a simple human being with almost no experience in mobile dev could help with?
@iamale well it's probably not that complicated to merge the changes but it will take at least a few hours of work.
What I'm more concerned about is the fact that nobody is realistically able to do any code review on this on giant commit. At least it would take ages.
This project is supposed to be a privacy friendly version of telegram. We don't yet know what changed in this regard.
So what do we do? Release an unvetted version ass soon as possible and look at the privacy implications in the weeks to come? Or do we carefully look at all new code first and release a new version much later?
I can take a first look at what's needed to build the new version tomorrow night.
I would say get it merged asap, and put an apk somewhere in the comments here, but not a release, so people who really want it can download it. Then review the code superficially, then do a release with a disclaimer, then continue reviewing code in more detail and remove the disclaimer? I'm maybe overthinking this
Telegram stores data on it's server. So you only loose secret chats by reinstalling it.
On 04/02/2017 02:48 PM, Edward M. wrote:
An apk file which is not signed by F-Droid will not be able to get installed on a device the F-Droid version is installed on. The user would have to reinstall Telegram. That's not quite nice! Of course with root you could backup the application data previously, but what about devices without root?
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/slp/Telegram-FOSS/issues/131#issuecomment-290977255, or mute the thread https://github.com/notifications/unsubscribe-auth/ADfTtkdl0fpbTB3U4jzkAA5bgCKw-Omgks5rr3YRgaJpZM4MvJsk.
What I see are two new dependencies:
Ignoring assets, drawables and deletions(those shouldn't be important to review for privacy reasons), there are:
A big part of the patch is actually the migration from exoplayer to exoplayer2 - 372 new files I believe. Mostly, it's the original Google's source code, but with changed package names.
Guys, who else is digging in review, let's divide it somehow. I'll continue crossing out exoplayer for now, diff's will follow
My proposal is to merge it in parts and to actually do the review, in the end it will be less shocking.
I don't think merging the deletions immediately without review is a good plan, deletions could do nefarious stuff as well.
@Matttter how? Under the assumption that the previous source code was kind of known-to-be-ok, what could go wrong if some parts of it are deleted? I'm having fun with it in my fork, anyway a bunch of separated commits will be a lot better then nothing https://github.com/thermatk/Telegram-FOSS/commits/master
EDIT: I see, I meant only when the full files are deleted :)
{
do normal things <--- not bad
other normal things <--- not bad
more normal things <--- not bad
}
if (user_press_bug_report_button){
send all the info to the overlords <--- not bad, requires user agreement
}
Could be made into :
{
do normal things <--- not bad
other normal things <--- not bad
more normal things <--- not bad
~~} if (user_press_bug_report_button){~~
send all the info to the overlords <--- really bad
}
Something like this, I would say. I have to say, I don't know very much about how the Telegram app works, so I'm just putting on my paranoid hat here
I'm finished with what seemed to be the easy part, added a [REVIEW] to the commit one should pay attention to then: https://github.com/thermatk/Telegram-FOSS/commit/cf046e0fe9fb24e92104b2a0e8796d3f89f953d9
@thermatk very good suggestion in merging this step by step. But I agree we need to pay attention to the deletions too.
And thanks for starting to look into the code.
if it makes life easier, use this branch: https://github.com/thermatk/Telegram-FOSS/commits/original3.13-to-3.18 It goes from upstream 3.13 to 3.18, but I've crossed out everything that one shouldn't check for malicious code. Everything else is still pretty big, but split into several commits with "[REVIEW]". As I'm not a security guy, I don't know if there's anything else I can do.
These commits are so huge that it's nearly impossible to see them on github.com on my laptop. Even on my desktop I have to let it sit for 5 minutes to be able to scroll... If only they would develop in the open, or at least push all the separate commits to the repo instead of making one giant one. This just isn't behaviour compatible with what free software is about for me: trust and transparency.
Soooooo
Ah, ok, i know why :) the jni thing! They only provide native prebuilts for armv7, and my emulator was x86
everything except jni is done then
I'd love to test it on a real phone if you want. Just put up an apk for me to test out
there you go, as it is a debug build you can install it alongside the usual app https://github.com/thermatk/Telegram-FOSS/releases/tag/3.18FOSS-BETA everything works as expected for me
Yup works great, have a couple of questions about this. The "call quality" rating after a call, where does that go? It should be removed IMO in the FOSS build.
Is the call connection p2p or does it go through the server? (probably doesn't matter now that I think about it, encryption and stuff)
@Matttter This call quality thing must be in the [REVIEW] parts, I didn't do anything new in this sense except merging previous patches.
Ok, thought so. I hope someone into the native stuff gives us a hand here
@Matttter Actually, I would be strongly against the removal of the call quality rating, but IFF (if and only if) it is implemented in the right way. For example, I didn't remove Giphy search either, because I don't see how harmful a network request done with the open source code provided can be, when you should explicitly go and tap "Search Giphy" for it to do something.
Let this really be about privacy and FOSS, optional features properly implemented upstream(not doing anything in a shady way without explicit click) shouldn't really be touched, because why??? :+1: Bing is gone, because of API key.
@Matttter Re: p2p vs server: https://telegram.org/blog/calls#super-fast
@Bubu @Matttter a cool and easy feature to do would be to show whether current call is p2p or server, maybe just add a one-liner toast :)
@thermatk Thanks for all the work! Can you open Pull Requests against this repo with the changes I should merge into this repo?
Or maybe it's easier to just do it in one PR, I don't know. We'll figure something out.
Oh and regarding giphy/call rating, etc. I'm with you. If it is an explicit option and optional it can very well stay in the app. patching it out only oncreases the maintenance overhead.
(I'm stuck in a train without a proper dev environment, so I can't actually build anything until later tonight.)
@Bubu be careful what you wish for :+1: https://github.com/slp/Telegram-FOSS/pull/133 my browser couldn't handle the pull request edit page until i disabled JS :)
As a first step I just updated openssl to 1.0.2k. Openssl 1.0.1 is no longer maintained and we need openssl to properly build in fdroid. So this was a required step anyway.
I'll continue tomorrow, working through the changes.
Can someone maybe gather up all the changelog entries from the intermediate releases?
@iamale @Matttter Ping.
Not sure if this is what you mean @Bubu
What's New in Telegram v3.18.0
What's New in Telegram v3.17.1
What's New in Telegram v3.17.0
What's New in Telegram v3.16.1
What's New in Telegram v3.16.0
What's New in Telegram v3.15.0
What's New in Telegram v3.14.0
What's New in Telegram v3.13.2
What's New in Telegram v3.13.1
What's New in Telegram v3.13.0
@Bubu @mastad0n https://github.com/slp/Telegram-FOSS/pull/135
@mastad0n @thermatk Merged, thanks.
@Bubu as far as i can see from jni/, the sources for libWebRtcAec library are not there. The original webrtc sources are here: https://chromium.googlesource.com/external/webrtc/+/master/webrtc
Yet I don't understand how to get from this to libWebRtcAec, and it seems that some source files have to be edited for that to happen, as the header files here are edited compared to upstream https://github.com/grishka/libtgvoip/tree/public/external/include/webrtc
As per last comment in https://github.com/grishka/libtgvoip/issues/1 , the developer is aware of missing sources for the libraries and had the intention of uploading them, 29 days ago :+1: I'm opening a bug upstream
Ok @Bubu https://github.com/slp/Telegram-FOSS/pull/136 is done The missing libwebrtcaec sources will be there this week, as promised by the upstream developer in response to my request
@thermatk @others What do you say about the SmsListener + related capability? I find that feature actually useful and we can check the code is doing nothing nefarious.
Do we add it back?
@Bubu I would add it back, since compared to some other messengers it still gives you the option to fill in the code manually :) This is important to me, since I frequently use Telegram on a smartphone with a different sim.
It's just a speed-up.
Reviewing the code gives us just one entry point, SmsListener.java, which forwards the text using NotificationCenter.didReceiveSmsCode
. There are 20 usages of this notification, all of which seem legit to me, in 3 files:
The sms messages don't seem to be transferred to some shady internet service :+1:
Yup I agree with this, if it does not send stuff to our corporate overlords I would think it's a useful feature.
Accept #138 and then #137 Only the libwebrtcaec will be remaining, but it doesn't depend on us.
The sources were uploaded! @thermatk
If someone wants to play with it, with the latest sources compiled and included in #138 , here is the arm sdk23+ debug apk:
Since it's a debug build, you can install it alongside usual Telegram and it will have the name "Telegram Beta". Calls are working fine here :+1:
EDIT: Outdated report. I had a local (Android-level) firewall which made the internet unavailable for Telegram Beta.
Does the current Telegram server API allow receiving a confirmation code NOT through SMS? The desktop version and the previous FOSS mobile version allowed to receive the code on older instances of telegram, without the use of SMS at all.
Can we do that for the latest version? The new UI freezes when trying to confirm my code, and nothing is being sent to my other (already connected) devices.
EDIT: the UI actually dead-freezes while trying to receive the SMS. Can't press "back" or enter another number. Had to force-kill the app. This actually is kind of an issue for me, because I do not really have the SIM I've registered with anymore. Previosly I got around that fact by sending a code I've received on my other devices.
@vn971 it does, although it chooses kind of randomly between sending to other devices, calling and sms. You can force it to send the code to other device with higher probability if you take out the sim for the time you set up the app
@vn971 I had a similar issue but i don't know if this trick will help you. Try to kill all "old unused sessions" in your current session and try to reconnect your device.
Beware not to remove "used sessions" since you don't have you SIM wich is needed for the first connection !
@thermatk I've tested your apk for the last 8 hours without any issue. Only the sms auto-detection hasn't worked for me. Nice job, colleague, a beer for you.
@thermatk Good job indeed! I also experienced no issues while using your apk.
Works perfect! No issues here either! Thank you @thermatk !!
Yup fine with me too! Let's get this on F-Droid I would say :)
Great idea! What are we still waiting for?
Looks like its updated :+1: :tada:
https://github.com/DrKLO/Telegram
Pavel Durov tweet: https://twitter.com/durov/status/847454485882355713
Telegram 3.18 blog: https://telegram.org/blog/calls
:tada: