Skip to content

feat(android): set search-domain on VPN configuration - #8436

Merged
thomaseizinger merged 3 commits into
mainfrom
feat/android-search-domains
Mar 14, 2025
Merged

feat(android): set search-domain on VPN configuration#8436
thomaseizinger merged 3 commits into
mainfrom
feat/android-search-domains

Conversation

@jamilbk

@jamilbk jamilbk commented Mar 14, 2025

Copy link
Copy Markdown
Member

On Android, we can use addSearchDomain to configure the search domain list for our VPN tunnel.

Thankfully, this gets applied to the system resolver without any other hackery involved (unlike for Apple in #8421), and most apps use the system resolver for queries. The one exception to this are some network utilities like AndroDNS and Fing.

Tested to work fine in Termux using github.io as the search domain, which responds to ICMP echoes to any subdomain:

Screenshot 2025-03-13 at 10 19 41 PM

Related #8410

@jamilbk
jamilbk requested a review from thomaseizinger March 14, 2025 03:25
@vercel

vercel Bot commented Mar 14, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
firezone ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 14, 2025 4:39am

@thomaseizinger thomaseizinger changed the title feat(android): Support search_domain on Android feat(android): set search-domain on VPN configuration Mar 14, 2025
})
})
.transpose()?
.unwrap_or_default();

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Does this return a nullable pointer if None? Otherwise I think this might send an empty string by default which won't be what we want on the other side of the FFI right?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes the default for JString is null.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I can add a quick unit test for that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nevermind, scratch that - just tested and I think the Option makes it over the FFI. I wouldn't have thought so, quite surprising.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nevermind, scratch that - just tested and I think the Option makes it over the FFI. I wouldn't have thought so, quite surprising.

The Option doesn't, the function-signature is still Ljava/lang/String;, so we are passing a String. But everything in Java can be nullable so we are effectively passing null. The Default impl for JString creates a JObject::null, which is why this works.

@thomaseizinger
thomaseizinger disabled auto-merge March 14, 2025 04:31
@thomaseizinger
thomaseizinger added this pull request to the merge queue Mar 14, 2025
Merged via the queue into main with commit 7e19668 Mar 14, 2025
@thomaseizinger
thomaseizinger deleted the feat/android-search-domains branch March 14, 2025 05:11
Sign up for free to 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.

2 participants