Repository navigation
Conversation
Dead code: - Remove the unreachable adware scanner. UpdateViewModel extended its view model and scanned every installed app each time the update screen opened, without ever using the result. - Remove the unused DohPacketProxy and its okhttp-dnsoverhttps dependency. - Remove unused methods in PreferenceHelper, UpdateModel, Animations, MissingAppDialog, ListsViewModel, AbstractListFragment and RegexUtils. - Remove unused layouts, drawables, mipmaps, menus, strings, arrays, colors and dimens from every locale. Configuration: - Remove the "Hide reboot dialog after apply" setting, which had no effect since the reboot dialog was removed. - Remove ProGuard rules for classes that no longer exist and for Sentry. - Run CI on main and on pull requests (it targeted master and never ran), drop the SonarCloud setup pointing at the upstream organisation and replace the deprecated dependency submission action. - Declare mavenCentral() for the build script classpath, which only resolved through the Sonar plugin repository. Fixes: - Use a notification priority instead of a channel importance constant. - Fix DnsPacketProxyTest failing on the android.util.Log stubs. - Mark the API 27 window attributes with tools:targetApi (lint NewApi). - Reuse UpdateModel.getVersionName() instead of two deprecated copies.
- Stop bundling libc++_shared.so: the OpenSSL prefab package forces the STL setting, but no packaged native binary links against it (APK 14.70 MB -> 13.09 MB). - Remove SourceModel's never observed updateAvailable LiveData, unused DAO methods, VpnServiceControls.isStarted, DnsQueryQueue.size and LogEntry.setType. - Replace assert statements, which never run on Android, with requireNonNull so a missing preference fails with its message. - Reuse createPendingObserver for createObserver in ApplyConfigurationSnackbar. - Use PackageInfoCompat.getLongVersionCode and document the remaining PackageInfo.signatures fallback. - Give static-only utility classes private constructors, add missing serialVersionUIDs and drop a redundant close(). - Run the locale check workflow on main, drop the deleted strings_support.xml from the Transifex config, let Dependabot update Gradle dependencies, and move linker flags out of the web server CFLAGS.
The executable renaming task guessed the build type from the command line: a release build run with any debug task (for example "testDebugUnitTest assembleRelease") packaged the debug executables, and debug builds never packaged the executables at all, only the empty lib*_exec.so stubs, because the app's debug variant reads them from the ndk-build output rather than the legacy copy. Register one renaming task per build type that replaces the stubs in both locations and runs before that build type's own native library consumers, and share it between the tcpdump and webserver modules instead of duplicating it. This also drops the dependency on AGP's internal LibraryJniLibsTask class.
…dapter FocusSkippingPreferenceGroupAdapter now wraps the stock adapter from PreferenceFragmentCompat.onCreateAdapter instead of subclassing the RestrictTo PreferenceGroupAdapter, which removes the last seven lint errors. A row now stays focusable only when its preference is both selectable and enabled, so non-selectable info rows are no longer D-pad stops either.
Replace the deprecated LocalBroadcastManager round trip, and its Serializable intent extra, with a direct call from VpnService to the current VpnModel. LocalBroadcastManager also kept every VpnModel ever built registered for good.
- Move the identical add/edit dialog of the blocked and allowed lists into a shared HostnameListFragment, and merge the redirected list's add and edit dialogs - Build the three update notifications with one helper - Reuse ApplicationLog.isApplicationDebuggable in the preferences - Add hosts to the user lists through one HostListItemDao transaction: the DNS log inserted a new row even when the host was already in a user list, which could leave a host both allowed and blocked
The last update tests queried the live GitHub, Gist and GitLab APIs, so they failed offline or on a rate limit. Check the API URL each hosting builds and the parsing of a recorded API response instead.
- Drop fragment-ktx: it was only added to work around a duplicate class clash between lifecycle 2.3 and 2.5, which current AndroidX versions no longer have - Declare the AndroidX dependencies in the version catalog like the others, so they share one place and get Dependabot updates - Drop the TV home activity label that repeats the application one
The web server used the com.android.ndk.thirdparty OpenSSL prefab package, a 2022 beta of OpenSSL 1.1.1q that never got updates, and the 1.1.1 series has been end of life since September 2023. That package also forced a shared libc++ into the APK that nothing linked against. Build Mongoose with Mbed TLS 3.6.7, the current long-term support release, vendored from the release archive and compiled by ndk-build into the web server executable like the other native sources. It keeps TLS 1.2 and 1.3 and the existing RSA certificate, so the certificate users installed stays valid. The libssl, libcrypto and libc++_shared libraries are no longer packaged (release APK 13.09 MB -> 8.58 MB).
The root mode DNS log runs the bundled tcpdump as root on live traffic. Its sources dated from 2014 (the binary reported tcpdump 4.5.1 and libpcap 1.5.2, although the vendored VERSION files said 4.7.4 and 1.7.4) and missed a decade of security fixes in the packet printers. Replace them with the current tcpdump 4.99.7 and libpcap 1.10.7 releases (signatures checked against the Tcpdump Group release key). config.h, the grammar and the scanner were generated by the releases' own configure and Makefiles with the NDK clang, with the word size dependent values made generic, and the Android.mk source lists follow the release Makefiles. The SMB printer is no longer built, as upstream defaults. All four ABIs build without a single compiler warning. The DNS log parsing is unchanged: on the same capture of A, AAAA and IPv6 queries, the old and new tcpdump give the same host names.
Declare the VPN service as a specialUse foreground service with the vpn subtype and request FOREGROUND_SERVICE_SPECIAL_USE, which Android 14 requires to start it in the foreground. Build the native modules against API 34 too and drop their targetSdk, which AGP ignores for library modules.
F-Droid reads the store listing from BCP 47 language folders (es-MX, zh-TW, he, id), as fixed upstream for issue AdAway#2776, but Transifex kept writing some translations to Android resource style folders (es-rMX, nb-rNO, zh-rTW) or to the legacy Java codes (iw, in), so those updates never reached F-Droid. Move the newest listing into each BCP 47 folder (the 2023 Traditional Chinese description and the only complete Hebrew one), drop the duplicates, map Hebrew and Indonesian to he and id for the store listing resources, and fix the metadata path in CODEOWNERS.
Both home activities built the same dialog, with the same retry action for the errors a sync retry can fix. Build it once in HostErrorDialog.
Both the lists and the DNS log screens built a user list item with the same six setters; add HostListItem.userItem for it.
Every accessor opened the preferences and resolved its key and default resources the same way: share it in prefs, getBoolean and getString. Each accessor keeps the exact same key and default (375 -> 243 lines).
- Rename the native executables in a task class using an injected FileSystemOperations: the doLast closure added earlier referenced the Gradle script, which the configuration cache rejects - Update dnsjava to 3.6.5, which brings its security fixes, and the test-only org.json to 20260814, which fixes its known CVEs - Test every database migration against the exported schemas with MigrationTestHelper, and the user list upsert transaction - Drop an unchecked cast in the DB test LiveData helper
- Run the bundled executables without LD_LIBRARY_PATH, which they no longer need, and quote every path passed to a root shell command through one helper. - Keep the dnsjava Java 18 resolver provider out of the R8 missing class check (dnsjava 3.6 lists it in a service file, Android never loads it). - Share the app bar title and focus skipping adapter of the preference screens in a PrefsFragment base class. - Keep the GitHub repository coordinates in one constant, and the update install state in PreferenceHelper with the other preferences. - Don't export the boot and package replaced receivers, which only receive system broadcasts. - Leave the dependency metadata block out of the APK, as F-Droid asks. - Web server: close the OOM score file on a read error, log the debug mode at info level, use static helpers with (void) prototypes. - Keep the cause when Subnet fails to build an address, drop an unused constant, use a string resource for the TV version label. - Convert the launcher icons to lossless WebP (28.7 KB -> 13.8 KB). - Remove unreferenced files from Resources/ and document the automation broadcast in docs/automation.md. - Fix the RELEASING.md lint report path, build steps and APK name.
Both source adapters had their own copy of the update status text, the approximate delay, the host count and the diff callback; move them to HostsSourceFormat. The host count rounding counted one digit short below 10, so 1000 to 1099 hosts showed as a raw number while 1100 showed as "1k": count the digits correctly and test the compact count.
SourceEditActivity and TvSourceEditActivity had the same source loading, file picker, form behavior, validation, saving and deletion, only on different views. Move them to AbstractSourceEditActivity, which each activity sets up with its own views; the TV one keeps its header buttons and title, the mobile one its action bar menu.
Lists the changes that need a running Android system to be confirmed, with the emulator setup to run them.
- Reject hosts source downloads answered with an HTTP error instead of replacing the source hosts with the parsed error page. - Stop polling a cancelled app update download and let the user retry a failed one instead of showing a frozen progress bar. - Share the hosts file for "Open hosts file" through the FileProvider: a file:// URI throws FileUriExposedException since targetSdk 24. - Tell the user when no app can open a link or a settings screen instead of crashing (common on Android TV, which often has no browser). - Stop the root check callbacks from touching a closed screen. - Ask for the VPN consent once on the mobile home screen instead of on every resume, which looped when it was declined. - Group the digits of the host counters. - Parse the web server certificate with java.security.cert instead of the deprecated javax.security.cert. - Rename the preference screen animators so they no longer override the private ones of the fragment library, and fix an es-MX typo.
Only Wi-Fi and cellular were tracked, so on a cable connected TV box losing Wi-Fi stopped the tunnel while the device was still online. The transport choice now prefers Ethernet like Android does and is unit tested. Also share one method between the three host list pagers, and validate IP addresses without parsing through an exception.
The exported 4.json was overwritten with the version 5 schema in 2020, so the migration test created a version 4 database with the host_entries table instead of the host_entries view the 3 to 4 migration creates, and the 4 to 5 migration (DROP VIEW) failed on it. All migrations (1 to 7) pass on Android 14.
On a fresh install, the AdAway and Pete Lowe default sources held no host at all while showing up to date. The startup update check stores the online modification date of each source before its first download, which then sent it as If-Modified-Since: the server answered 304, the source was not parsed but stamped as retrieved, and later syncs skipped it as up to date. - Only send conditional request headers for a source holding hosts. - Never skip an empty source as up to date, which also repairs the installs already in that state. - Only fail the sync when no enabled source could be used, instead of reporting "none of your enabled hosts sources are reachable" for one failed source among usable ones. - Restart ad blocking after the VPN consent is granted again on the mobile home screen.
On dm-verity system partitions (Android 10 and later without the Magisk systemless hosts module), "mount -o rw,remount" prints that the partition is read-only but exits with 0, so the hosts file copy was attempted anyway and failed with an empty message: libsu only collects stderr when asked to. - Check the hosts file is writable after remounting, and fail with a message naming the read-only partition otherwise. - Collect stderr for the remount and copy commands so their errors are logged. - Record the root mode checks on the rooted Android 14 emulator in the device checklist.
- Apply the Android Gradle plugins through the plugins block and the version catalog instead of the buildscript classpath, and declare the repositories once in settings.gradle (FAIL_ON_PROJECT_REPOS). JitPack is restricted to libsu, the only dependency it serves. - Keep compileSdk, targetSdk, minSdk and the NDK version in the version catalog instead of repeating them in each module. - Shrink the release build with proguard-android-optimize.txt: proguard-android.txt is deprecated and rejected by AGP 9. The release APK goes from 9.09 MB to 8.32 MB; root and VPN modes were checked on the Android 14 emulator. - Enable BuildConfig, used for the version name and debug checks. - Drop the room.incremental option, the default since Room 2.3, and the supports-screens element, which only restated the defaults.
…nload receiver - The daily update worker read the manifest LiveData right after posting it, so it got the previous value (null in a fresh process) and never showed the update notification. checkForUpdate() now returns the fetched manifest, which replaces checkForUpdateNow(). - Remove ApkDownloadReceiver. Registered with RECEIVER_NOT_EXPORTED, it never receives ACTION_DOWNLOAD_COMPLETE from the download manager on Android 13 and later (checked on Android 14), and on older versions it launched the installer a second time next to the update screen. - Stop polling the download progress once the update screen is closed. - Read the version name and the debug flag from BuildConfig instead of the package manager and the application flags. - Call PreferenceHelper for the pending install state directly instead of through UpdateModel wrappers.
The web server switch ran its root shell commands on the main thread, then two background tasks each slept 500 ms before checking the process and the server state. Starting or stopping now runs in one background task that polls for the expected state (up to 2 s) and then updates the switch and the state summary; the icon setting restart goes through it too.
… services - Replace the deprecated ActivityManager.getRunningServices() lookup with a flag set in VpnService.onCreate() and cleared in onDestroy(). The service runs in the application process, so an external kill clears it with the service. - Replace the stale TODO notes in VpnWorker with its documentation, and drop a null check on pcap4j raw data, which is never null. - Make VpnService and VpnWorker final.
- Check the bundled executables with pidof instead of ps piped to grep. - Finish the apply snackbar on the main thread instead of building views on the disk executor after a 100 ms sleep. - Use Context.getColor() instead of Resources.getColor(id, null). - Use the AppCompat drawable attributes on the TV home screen. - Import Handler, Looper, VpnService and Timber instead of using their qualified names inline. - Make ApplyConfigurationSnackbar, BumpBottomBar and ListsViewModel final (javac this-escape warnings).
values-en-rUS held a Transifex "translation" of English into US English: 247 of its 250 strings were identical to the default ones and the other 3 were stale copies, so on devices set to US English it hid every later change to the English text (the theme setting label and two Magisk error messages). English is the source language: the default resources already cover it. Also drop en-US from the per-app language list. The store listing in metadata/en-US is unrelated and stays.
A source whose download fails (unreachable server, HTTP error, lost file access) used to look like any other: the home screen only counted up to date and outdated sources, and the sources list showed "Unknown update status". - Record the result of the last retrieval in a new last_sync_failed column (database version 8, with its migration and exported schema; the migration test covers 1 to 8). - Show "N unreachable sources" below the source counts on the mobile and TV home screens when there are any, and leave failed sources out of the up to date and outdated counts. - Show "Last download failed" for those sources in the sources list. - Compare the label, size and failure state in HostsSource.equals() so the list rows refresh when they change. - Give the three home screen tiles the same height.
…ation - Center the welcome pages vertically instead of leaving the lower half empty, with a larger logo and title and more spacing on the method page. - Skip the battery optimization and OEM battery manager steps in root mode: they keep the VPN service running and do not apply to the hosts file. The step dots follow the chosen method. - Add a back button to the update screen, which has no action bar. - Rename the "Dark theme" setting to "Theme": it picks the light, dark or system theme.
- The number of steps depends on the ad blocking method (two for root, four for VPN), so the dots are only shown once a method is chosen instead of showing four steps up front. - Reword the sync step text to explain what is downloaded and where to change it, and drop the inter-word justification that spread its words apart.
Go through every English string and the help pages for grammar,
consistency and plain wording:
- Use one style throughout: sentence case, "ad blocking" as a noun and
"root-based"/"VPN-based" as adjectives, "app" instead of
"application", "Could not ..." for errors with what to do next.
- Fix wrong or broken text, such as the hostname hint of the add host
dialog ("The hosts source URL"), "wait for network" in the VPN
notification, "Open Source ad blocker" and the blacklist/whitelist
wording left from the old lists.
- Give the home screen counters a single label (Blocked, Allowed,
Redirected) instead of two lines that read as one broken phrase.
- Shorten the welcome method texts so both cards keep one line each.
- Update the menu paths in the troubleshooting page to the current
setting names, and use https for the hosts file link.
- Remove raw-en-rUS: like values-en-rUS, it was a stale US English copy
of the help pages that US English devices showed instead of the
current ones.
Author
|
I don't think submitting everything in same PR was a genius move lol. |
Author
|
i'll separate and resubmit split PRs for each section, but let's discuss things here before doing so, any adjustments you want / don't want integrated. Also are you open for preference revamping? Splitting it into sections, moving help and update check and version number inside? Adding new default block lists such as Cubic |
Owner
|
Hi, I’ll be straight with you, because I think you can really grow on this project and it’s better to say it clearly. On AI, this is where it goes wrong. I accept it in contributions, and I’m not blaming you for using it. But here it is being misused. AI is not a magic button: a prompt like “fix all the bugs in the app” doesn’t give you a contribution, it gives you a pile of changes that looks impressive but wasn’t thought through for any specific problem. An AI doesn’t know the whole app or all of its behavior, it produces plausible code, not verified code. Using it well is the opposite: one precise problem, a narrow request (“change only this”), a result you check, then you repeat for the next problem. You drive it, it doesn’t drive you. Let’s be clear: if the goal is to fire off a prompt and send me whatever comes out, I can do that myself. I don’t need a contributor for that. What I expect from you is what the AI doesn’t do on its own: choosing the right problems, framing the request, checking the result and really testing it. On the size. Your PR touches about 1,570 files, with 370,000 lines added and 120,000 removed, across 36 commits. It mixes bug fixes, web server encryption, the VPN, root mode, Android TV, the database (with a version migration), the build system, translations and a rewrite of the texts. With a change this big, any part of the app can break, and the effects of one change can spread elsewhere. Nobody can seriously verify that in one go. It’s not a question of skill, it’s a question of volume. On testing. The description announces tests and checks. Yet it states itself that Android TV, Ethernet and the update download are not tested yet, while your PR changes exactly those parts. For the VPN, the project’s CONTRIBUTING asks for a full week of real daily use before opening the PR, with the conditions described in the testing section. An emulator and a few minutes of testing are not enough, because VPN bugs often show up after several days: sleep, network changes, reboots. In the end, it gives the impression of work that went fast, let the AI do most of the job, and wasn’t reviewed by the person who produced it. I’m not saying it’s ill-intentioned, but a contribution has to show it was checked before it gets here. On the project rules. CONTRIBUTING says one simple thing: one PR, one topic. Several small PRs are even encouraged. What is discouraged is mixing a VPN fix, a UI redesign and a dependency update in the same PR. Your PR does all of that at once. What I’m asking for, concretely: pick a bug you reproduced yourself, and describe it in your own words; Thanks for your work on the project, and good luck. |
Author
|
Just to be clear, didn't use AI entirely to generate the code. Large part of the code was written by me, AI was mainly used to scan for depreciations and migrations. And everything was hand tested, nothing was written and pushed without testing. And regarding mixes, i already mentioned i can split it across multiple PRs for easier tracking, it's mentioned in my earlier comments. And after all, it's your fork and your decisions, so it's understandable no worries |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is a maintenance pass over the whole app: bug fixes, security updates of the bundled native code, Android 14 support, dead code removal, refactoring and a rewrite of the English text. Each commit is self-contained and builds on its own, so it can be reviewed commit by commit.
Bug fixes
If-Modified-Since. The server answered 304, the source was stamped as retrieved without being parsed, and later syncs skipped it: the AdAway and Pete Lowe sources held 0 hosts while showing "Up-to-date". Existing installs in that state are repaired on the next sync.file://URI,FileUriExposedException), and opening links or settings on devices without a browser (common on Android TV).ACTION_DOWNLOAD_COMPLETEreceiver, registered as not exported, never ran on Android 13+ and launched the installer twice on older versions.mountexits with 0 even when it cannot remount.values-en-rUSandraw-en-rUSwere stale copies of the English text and help pages, so US English devices never saw English text changes. Removed (themetadata/en-USstore listing is unrelated and stays).Security and platform
specialUseforeground service.Features and UI
Code and build
LocalBroadcastManager,getRunningServices,javax.security.cert,PackageInfoversion lookups (nowBuildConfig), the restrictedPreferenceGroupAdaptersubclass.settings.gradle, configuration cache compatible,proguard-android-optimize.txt, native executables packaged for every build type (debug builds had none).mainand pull requests (it targetedmasterand never ran).Testing
4.jsonschema had held the version 5 schema since 2020; it is restored.docs/device-checks.mdlists these with the steps.