Skip to content

Handle denied system DNS lookup in manager - #771

Closed
Shallow-dusty wants to merge 3 commits into
JingMatrix:masterfrom
Shallow-dusty:fix/dns-security-exception-crash
Closed

Handle denied system DNS lookup in manager#771
Shallow-dusty wants to merge 3 commits into
JingMatrix:masterfrom
Shallow-dusty:fix/dns-security-exception-crash

Conversation

@Shallow-dusty

@Shallow-dustyShallow-dusty commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Fixes#636

When DoH is disabled, or when the DoH resolver fails and falls back, the manager uses OkHttp's system DNS implementation through CloudflareDNS.lookup(). The crash log in #636 shows Android 11 can throw an unchecked SecurityException from that DNS lookup path:

  • FATAL EXCEPTION: OkHttp Dispatcher
  • java.lang.SecurityException: Permission denied (missing INTERNET permission?)
  • at org.lsposed.manager.util.CloudflareDNS.lookup(...)

The manifest already declares android.permission.INTERNET, so this is not a manifest-permission fix. Instead, handle the denied lookup defensively around the final system resolver call by converting SecurityException into UnknownHostException, which is the checked failure type OkHttp expects from DNS implementations.

This keeps the DoH timeout, session-scoped disable flag, fallback log, and UnknownHostException fallback behavior added by #765 unchanged. Normal system-DNS UnknownHostException failures also continue to propagate unchanged.

Validation

  • git diff --check
  • ./gradlew.bat --no-daemon :app:compileDebugJavaWithJavac
  • ./gradlew.bat --no-daemon :app:assembleDebug
  • Android 11 / API 30 emulator (Vector_API30), doh=false: manager UI launches and remains alive; no fatal crash in logcat.
  • Android 11 / API 30 local-only fault injection against current master: base reproduces FATAL EXCEPTION: OkHttp Dispatcher; this branch converts the same denial to a hostname-bearing UnknownHostException with no fatal crash.
  • Android 11 / API 30 local-only DoH-failure + system-denial injection: the Fix module repository loading #765 fallback log remains, and the system denial is converted without a fatal crash.

Reproduction boundary

I could not reproduce the exact EPERM / SecurityException on a clean API 30 emulator. Attempts included Private DNS off, opportunistic, invalid hostname mode, Data Saver / app UID restrict-background blacklist, and a local-only no-INTERNET manifest build. These did not reproduce the reporter's environment-specific android_getaddrinfo failed: EPERM path.

To verify the fix still covers the reported failure mode, I used a local-only fault-injection comparison that was not included in this PR:

  • On current origin/master after Fix module repository loading #765 (2a633ef0), injecting the same SecurityException("Permission denied (missing INTERNET permission?)") before its final system DNS call with doh=false reproduced FATAL EXCEPTION: OkHttp Dispatcher and Process: org.lsposed.manager.
  • On this branch, injecting the same exception inside the system-resolver try block kept the app alive and converted request failures to hostname-bearing messages such as java.net.UnknownHostException: api.github.com: Permission denied (missing INTERNET permission?), with no AndroidRuntime fatal crash.
  • With doh=true, a second local-only injection forced the Fix module repository loading #765 DoH failure path before the same system denial. The existing DoH resolver unreachable, falling back to system DNS for this session log still appeared, and the system denial was converted with no fatal crash.

So the clean emulator cannot recreate the original device policy state, but the branch directly covers the exception class/message at the same method boundary shown in the issue stack. OnePlus 9 Pro / LE2120 was online but runs Android 14, so it was not used to claim coverage of this Android 11-specific boundary.

@Shallow-dusty
Shallow-dusty marked this pull request as ready for review June 24, 2026 02:18
CopilotAI review requested due to automatic review settings June 24, 2026 02:18

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Defensively prevents Vector manager crashes on Android 11 when using OkHttp’s system DNS (DoH disabled) by converting unexpected SecurityException failures at the Dns.lookup() boundary into the checked UnknownHostException OkHttp expects.

Changes:

  • Wrap CloudflareDNS.lookup() resolution path in a try/catch.
  • Convert SecurityException from DNS lookup into UnknownHostException with the original exception set as the cause.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadapp/src/main/java/org/lsposed/manager/util/CloudflareDNS.java Outdated
Preserve the session-scoped DoH fallback from JingMatrix#765, and convert SecurityException only around the final system resolver call.
@JingMatrix

Copy link
Copy Markdown
Owner

Thanks for taking a look at this. That said, I don't see anything here that points to the actual cause of #636, and the reporter hasn't been asked whether this helps — so there's no way to tell if it changes anything for them. I'd rather not merge a fix we can't confirm, so I'll close this for now. If the reporter tries it and it helps, happy to revisit.

@Shallow-dusty

Copy link
Copy Markdown
ContributorAuthor

The CI build linked in #636 was produced by the Core workflow for this PR’s current head (6cd9716). @rahaaatul confirmed that the manager no longer crashes on their Android 11 device, which provides device confirmation for the scoped crash-prevention fix.

The Repository tab still does not load, so I’ve asked for targeted logs and network details in #636. I’ll treat that as a separate failure path rather than expanding this PR.

Would you be open to reconsidering #771 based on the confirmed crash fix?

@JingMatrix

Copy link
Copy Markdown
Owner

#795 fixed the cause — the manager had no network at all on Android 11 and below — so converting the SecurityException is no longer needed; thanks for the testing in #636.

Sign up for freeto 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.

[Android 11] Disabling DoH cause Vector manager to crash

3 participants

@Shallow-dusty@JingMatrix