AdguardTeam / AdguardForiOS

The most advanced ad blocker for iOS
https://adguard.com/
GNU General Public License v3.0
1.42k stars 200 forks source link

Need "Automatically Refresh" after adding or deleting subscriptions, especially when VPN tunnel was ever collapsed. #661

Closed lancelot-moon closed 4 years ago

lancelot-moon commented 6 years ago

@ameshkov @IvanIin @camelot-sun

Mine is iPhone 6 Plus. iOS v11.2.5. AdGuard beta v2.0.0(132).

I met an issue. I tried to have 114094 rules. Magically, VPN tunnel was not collapsed at that time. I successfully established VPN connection. (In my test, the upper limit of rules in Blocking List is around 91k.)

Screenshot: ![image](https://i.imgur.com/BSNX4lF.png)

I disabled AdGuard Protection and then clicked "Check for updates" in Subscriptions. Well, I was failed to establish VPN connection due to large numbers of rules, which is in our expectation. So I deleted 1 subscription which has large numbers of rules. It remained 90901 rules at this time, and I could successfully establish VPN connection.
Screenshot: ![image](https://i.imgur.com/rlVbLTJ.png)

However, ad domains were almost not blocked! Do you remember that I reported xxxxx.aotter.net for letting you add them in AdGuard filter? Well, we can know that they were not blocked from the DNS Requests Log. Some in filter were blocked, but some in filter were not blocked. It's in disorder.
Screenshot: ![image](https://i.imgur.com/Sl0sCnn.png)

I tried to delete more subscriptions for reducing rules. Re-connect Wi-Fi and then enable VPN. Reboot my iPhone 6 Plus. The above methods are useless. Although I could enable AdGuard Protection, every ad domain was not blocked, which became the worst. See, doubleclick.net was not blocked in the DNS Requests Log. Only trackers or analytics in Disconnect database were shown by bright red in the DNS Requests Log.
Screenshot: ![image](https://i.imgur.com/sH7HlY5.png)

Finally, I found the solution by myself. I again clicked "Check for updates" in Subscriptions and then enabled AdGuard Protection. Eventually, Ad Blocking was back! So I suggest Ivanlin to design that AdGuard Pro will do "Automatically Refresh" for subscriptions after adding or deleting subscriptions every time, which is helpful to avoid some issues in the aspect of subscriptions. And then we need to let users not set rules over the upper limit, otherwise they maybe meet some strange issues which are related to VPN tunnel and subscriptions. https://github.com/AdguardTeam/AdguardForiOS/issues/631 We indeed have some problems of logic and bugs in design for Subscriptions. These are series of issues in Subscriptions. But Ivanlin never explains about them... Pity... https://github.com/AdguardTeam/AdguardForiOS/issues/654 https://github.com/AdguardTeam/AdguardForiOS/issues/653 https://github.com/AdguardTeam/AdguardForiOS/issues/635 Do AdGuard team need the log files? I can send the bug report if you need. But I ever cleared statistic & DNS Requests Log and already closed AdGuard Pro app. Will the log files about the past data be automatically cleared after closing AdGuard Pro app? Ah... I have updated to beta v2.0.0(136) after the test. I don't know if those data are still reserved. B/R
zeezeepiggy commented 6 years ago

@lancelot-moon , sorry to piggyback in your request, can you perform your test as well in the latest beta (136)? I’m interested in the idea you presented above regarding automatice refresh as it make sense... though I am not sure the performance implication when we do this. Let’s hear from the experts.

PS: i know we have conflict of opinion in the past, and that’s normal between testers, but i think we need to reconcile and work together as a team of testers to help AdGuard Team and make the AdGuard app better. Again, Apologies. Happy Chinese New Year to you.

camelot-sun commented 6 years ago

@lancelot-moon , I don't have the code so I can't see it my self, but what I am thinking is, the probably requires a "commit" to function after a delete was done. And the "commit" was probably done during the "Check for Updates". The operation and transaction was just probably hanging in the memory until a certain process finalise it. This is just a theory and assumption.

VPN in iOS has very limited memory, and if that memory is not freed, it will not do anything (hang state) and will not perform well until it's freed for other processes to execute.

The AdGuard team should probably create a limit so that the memory will not be full (or a warning like Safari Content Blocker has which turns red when it reaches 50K) and it still has left room to execute other processes so that it will not break.

Even if iOS has limitations and security, there will always a solution.

On the other note, please reconcile with @zznosar . I've checked that users' requests and issues here in GitHUB and same as you, that user wants to have the app to be the best as possible. both of you have different opinions. I think that user has been working with the IT corporate world long enough to give those opinions and on top of that, that user is leading a team (base on that users post). Leading a team is very hard. I know what that user meant, being an IT my self. Their teams methodology is probably Agile with "New Features + Bug", "Enhancements + Bug" release approach. That user has it's point, and you have your own, i think if you and the other testers (except for the certain "moon") work together, adguard will be a very good app. Forget about the differences, work together as team; let's work together as a team to help AdGuard deliver the best for the users. It's a good feeling if you accomplish something with a team rather than being alone.

Happy Chinese New Year.

i should go to bed now, after an all nighter to meet my deadline!

lancelot-moon commented 6 years ago

I tested beta v2.0.0(136). I tried to add a subscription with many rules for letting total rules over 100k rules and didn't click "Check for Updates" at this time.

It's the same that I could establish VPN connection with 100k+ rules before clicking "Check for Updates", but some ad domains weren't blocked in the DNS Requests Log. (Setting the limit 91k (?) to let rules not over the upper limit is necessary.)

Follow on, the difference is: In beta v2.0.0(132), ad-blocking became invalid even if I deleted subscriptions for decreasing rules under the upper limit. I need to click "Check for Updates" for solving the issue.

In beta v2.0.0(136), ad-blocking would be back if I deleted subscriptions for decreasing rules under the upper limit.

I'll continue to check this for several times.

I also noted that it seems that Ivanlin changed the priority of Disconnect database and subscriptions. https://github.com/AdguardTeam/AdguardForiOS/issues/606#issuecomment-359168460 If a domain is in Disconnect database and subscriptions at the same time, now it's blocked. (In the earlier beta version, it's only processed, not blocked.) Ex: ||voicefive.com^, it's in AdGuard SDN filter. Now, sb.voicefive.com is blocked in DNS Request Log.

Honestly, I think that Ivanlin should make a changelog, which conveniently lets testers know what he modified...

And then I'm unable to discuss about the series of issues in Subscriptions which I reported with Ivanlin since he doesn't explain. Only he knows the logic and what causes the bugs in design.

--

@zznosar I don't put it on my mind. Everyone has his thought and standpoint. Every user promotes his ideas, which is normal. AdGuard team is the policy maker. Except for themselves, AdGuard team take ideas from zznosar, X8716e, other testers/users and me as long as they're good ideas. I know that zznosar is different with someone. Happy Chinese New Year to you, too.

For zznosar's friends, I can understand that they want to early use AG Pro v2.0.0. So am I. In the past, I was not a tester, too. They can go to apply for being a tester. My iPhone 6 Plus is just an old device, and then I also became a tester. Otherwise, they need to wait until AdGuard team solve bugs and think it's time to release it. I don't interfere with the release time of AG Pro v2.0.0. AdGuard team decide it by themselves. If zznosar's friends can't be testers... (though I think it's not difficult to be a tester...) When they can directly download AG Pro v2.0.0 from Apple app store, they will know the wait is worth.

--

Do I make trouble when AdGuard team uses ideas from zznosar or others, such as someone? After AdGuard team had decided to change the colors in DNS Request Log, only someone constantly makes trouble here.

Screenshot: ![image](https://i.imgur.com/7LkcOCk.png)

He wants AdGuard team to do only according to his thought. He intentionally ignores @ameshkov warning: He should stop his harassment to me. I share my thought about AdGuard app, which is not complaint. If I have complaint to AdGuard team, the under is indeed my complaint! Why does AdGuard Administrator not forbid him commenting on AdGuard GitHub? Where is the AdGuard Administrator? I stay on AdGuard GitHub for more than 1 year. Such guy who only came here for several weeks constantly harassed me. What did someone do since he came here from AdBlock by Future Mind? Oh, he did very much effort to insult and discredit me, which is really hard work for him. He spent much time on me, not AdGuard app. Until now, I start to know it's because I'm shiny, which pierces his eyes. He envies me. So he attempts to destroy me step by step. Self-righteous justice. LoL. Someone is just nothing. However, he constantly caused arguments, stirred up trouble and attempted to sow discord between other testers & me, even AdGuard team and me. He contorted everything! Where is the AdGuard Administrator? Why do you not seriously consider to forbid him commenting on AdGuard GitHub?
zeezeepiggy commented 6 years ago

@camelot-sun and @lancelot-moon , Thanks. Don't worry about my team mates and friends. They understood. If you ever visit Australia, especially Melbourne next time, let me know and we're sure glad to meet up with you, share ideas and tour you.

For the other person who tries to discredit you and disrespect you (saw it in the other post. user name has "chibi"), there is an option in GITHUB called "Block Bullies" https://github.com/blog/862-block-the-bullies . Please try it to that person. If you need help reporting that user, let me know and I will help too.

camelot-sun commented 6 years ago

Thanks both. I think for consistency, they should have the below logic:

Logic 1: for checking limit swith hard coded limit value of 90000. If above 90K then put a warning message.

If count(rules) >= 90000 then warning_message (“Limit Reach”); result=true; else result=false; End

Logic 2: only refresh when below 90K and after each add and delete.

If result = false then If uiaction in (“Add”, “Delete”) then checkrefresh() End Else Null End

This is just a sample logic and algorithm.

Performace should not be an issue if the main code is already tuned already.

I can be wrong since I dont have visibility in the code, but i think hopefully the adguard team will know what i mean.

Then testers need to test the functionality so that VPN will not be brocken.

lancelot-moon commented 6 years ago

Afterwards, I tested beta v2.0.0(136) for several times. In the most situation, ad-blocking would be back if I deleted subscriptions for decreasing rules under the upper limit.

But I met the issue once. Ad-blocking was still invalid even if I deleted subscriptions for decreasing rules under the upper limit. Those ad domains in subscriptions were shown as trackers, not blocked.... Maybe those ad domains are also in Disconnect database. Anyway, ad-blocking was invalid at that time before clicking "Check for Updates" in Subscriptions.

Screenshot: ![image](https://i.imgur.com/SRuXMoD.png)

> I also noted that it seems that Ivanlin changed the priority of Disconnect database and subscriptions. Update information. ||voicefive.com^, it's in AdGuard SDN filter. During my several tests, sb.voicefive.com is sometimes not blocked (shown as tracker, only processed) in DNS Request Log. Now, I am not sure if Ivanlin really fixed the bug. I still suggest Ivanlin to design "Automatically Refresh" for subscriptions after adding or deleting subscriptions every time.
ameshkov commented 4 years ago

Dup of https://github.com/AdguardTeam/AdguardForiOS/issues/616