fix: Don't try to use system resolv.conf when other servers are configured #2460

Open
silversys wants to merge 3 commits from silversys/continuwuity:silversys/fix-dns-resolution-errors into main
Member

Previously, only HTTP clients targeting matrix servers (and using the
resolution spec) would use the DNS servers configured in the c10y config.
This happened because reqwest requires any DNS resolvers passed in to
implement reqwest::dns::Resolve, which only the matrix resolver did,
and no other HTTP client had a proper DNS resolver (which used the
configured DNS servers) passed into it.

To fix this we make a custom resolution struct based on reqwest's
reqwest::dns::HickoryDnsResolver, which implements Request with no
other modifications to the DNS response. HickoryDnsResolver itself isn't
exported, which is why we re-created it instead of using it directly.

no im not an LLM i just wrote this to remember my own changes later on

Fixes: #2441

Pull request checklist:

  • This pull request targets the main branch, and the branch is named something other than
    main.
  • I have written an appropriate pull request title and my description is clear.
  • I understand I am responsible for the contents of this pull request.
  • I have followed the contributing guidelines:
Previously, only HTTP clients targeting matrix servers (and using the resolution spec) would use the DNS servers configured in the c10y config. This happened because reqwest requires any DNS resolvers passed in to implement reqwest::dns::Resolve, which only the matrix resolver did, and no other HTTP client had a proper DNS resolver (which used the configured DNS servers) passed into it. To fix this we make a custom resolution struct based on reqwest's reqwest::dns::HickoryDnsResolver, which implements Request with no other modifications to the DNS response. HickoryDnsResolver itself isn't exported, which is why we re-created it instead of using it directly. no im not an LLM i just wrote this to remember my own changes later on Fixes: #2441 **Pull request checklist:** <!-- You need to complete these before your PR can be considered. If you aren't sure about some, feel free to ask for clarification in #dev:continuwuity.org. --> - [x] This pull request targets the `main` branch, and the branch is named something other than `main`. - [x] I have written an appropriate pull request title and my description is clear. - [x] I understand I am responsible for the contents of this pull request. - I have followed the [contributing guidelines][c1]: - [x] My contribution follows the [code style][c2], if applicable. - [x] I ran [pre-commit checks][c1pc] before opening/drafting this pull request. - [ ] I have [tested my contribution][c1t] (or proof-read it for documentation-only changes) myself, if applicable. This includes ensuring code compiles. - [x] My commit messages follow the [commit message format][c1cm] and are descriptive. <!-- Notes on these requirements: - While not required, we encourage you to sign your commits with GPG or SSH to attest the authenticity of your changes. - While we allow LLM-assisted contributions, we do not appreciate contributions that are low quality, which is typical of machine-generated contributions that have not had a lot of love and care from a human. Please do not open a PR if all you have done is asked ChatGPT to tidy up the codebase with a +-100,000 diff. - In the case of code style violations, reviewers may leave review comments/change requests indicating what the ideal change would look like. For example, a reviewer may suggest you lower a log level, or use `match` instead of `if/else` etc. - In the case of code style violations, pre-commit check failures, minor things like typos/spelling errors, and in some cases commit format violations, reviewers may modify your branch directly, typically by making changes and adding a commit. Particularly in the latter case, a reviewer may rebase your commits to squash "spammy" ones (like "fix", "fix", "actually fix"), and reword commit messages that don't satisfy the format. - Pull requests MUST pass the `Checks` CI workflows to be capable of being merged. This can only be bypassed in exceptional circumstances. If your CI flakes, let us know in matrix:r/dev:continuwuity.org. - Pull requests have to be based on the latest `main` commit before being merged. If the main branch changes while you're making your changes, you should make sure you rebase on main before opening a PR. Your branch will be rebased on main before it is merged if it has fallen behind. - We typically only do fast-forward merges, so your entire commit log will be included. Once in main, it's difficult to get out cleanly, so put on your best dress, smile for the cameras! --> [c1]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md [c2]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/docs/development/code_style.mdx [c1pc]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#pre-commit-checks [c1t]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#running-tests-locally [c1cm]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#commit-messages
fix: Don't try to use system resolv.conf when other servers are configured
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Auto Labeler / Apply labels based on changed files (pull_request_target) Successful in 3s
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 6s
Checks / Prek / Check changed files (pull_request) Successful in 9s
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
4d69ceeceb
Previously, only HTTP clients targeting matrix servers (and using the
resolution spec) would use the DNS servers configured in the c10y config.
This happened because reqwest requires any DNS resolvers passed in to
implement reqwest::dns::Resolve, which only the matrix resolver did,
and no other HTTP client had a proper DNS resolver (which used the
configured DNS servers) passed into it.

To fix this we make a custom resolution struct based on reqwest's
reqwest::dns::HickoryDnsResolver, which implements Request with no
other modifications to the DNS response. HickoryDnsResolver itself isn't
exported, which is why we re-created it instead of using it directly.

no im not an LLM i just wrote this to remember my own changes later on
silversys force-pushed silversys/fix-dns-resolution-errors from 4d69ceeceb
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Auto Labeler / Apply labels based on changed files (pull_request_target) Successful in 3s
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 6s
Checks / Prek / Check changed files (pull_request) Successful in 9s
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
to 809585cc96
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 6s
Checks / Prek / Check changed files (pull_request) Successful in 8s
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
2026-11-04 23:48:38 +00:00
Compare
silversys force-pushed silversys/fix-dns-resolution-errors from 809585cc96
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 6s
Checks / Prek / Check changed files (pull_request) Successful in 8s
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
to 588cdc14d0
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Changelog / Check changelog is added (pull_request_target) Failing after 12s
Checks / Prek / Check changed files (pull_request) Successful in 9s
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
2026-11-04 23:49:33 +00:00
Compare
chore: Changelog
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 10s
Checks / Prek / Check changed files (pull_request) Successful in 5s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 21m0s
Checks / Prek / Clippy and Cargo Tests (pull_request) Failing after 40m37s
7832f1a91a
silversys changed title from fix: Don't try to use system resolv.conf when other servers are configured to WIP: fix: Don't try to use system resolv.conf when other servers are configured 2026-11-04 23:50:16 +00:00
Author
Member

WIP until tested

WIP until tested
@ -314,3 +339,3 @@
.map_err(|e| err!(error!("Failed to configure DNS resolver from system: {e}")))?;
let mut conf = hickory_resolver::config::ResolverConfig::default();
let mut conf = ResolverConfig::from_name_servers(vec![]);
Author
Member

Note to reviewers here: I replaced ::default() with ::from_name_servers(...) as the Default implementation is going to be removed:
image

Note to reviewers here: I replaced `::default()` with `::from_name_servers(...)` as the `Default` implementation is going to be removed: ![image](/attachments/74d335e3-1001-49b6-8181-d7f16f696998)
eleboucher requested changes 2026-11-05 07:21:29 +00:00
Dismissed
eleboucher left a comment

we need also to add it in oidc/mod.rs:165 as we don't pass any dns stuff

we need also to add it in oidc/mod.rs:165 as we don't pass any dns stuff
@ -983,7 +983,7 @@ impl crate::Context<'_> {
let resolver: &MatrixResolver = if no_cache {
Contributor

Passing matrix_http_client fixes .well-known fetching only. Without .dns_resolver(...), MatrixResolverBuilder creates a system resolver for SRV lookups. Pass the configured resolver here too.

Passing matrix_http_client fixes .well-known fetching only. Without .dns_resolver(...), MatrixResolverBuilder creates a system resolver for SRV lookups. Pass the configured resolver here too.
silversys marked this conversation as resolved
@ -130,3 +151,2 @@
appservice: base(config)?
.dns_resolver(matrix_dns_resolver)
appservice: base(config, matrix_dns_resolver)?
Contributor

this should be reqwest_dns_resolver as per the MSC

this should be reqwest_dns_resolver as per the MSC
silversys marked this conversation as resolved
Because openidconnect vendors reqwest (??), we have to re-implement
the entire Resolve trait.

No I don't like it, but I can't think of a better solution.
fixup! chore: Changelog
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Successful in 10s
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
21be870bd3
silversys force-pushed silversys/fix-dns-resolution-errors from 21be870bd3
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Successful in 10s
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
to 3445d54a19
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Successful in 8s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m14s
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
2026-11-05 12:29:58 +00:00
Compare
silversys force-pushed silversys/fix-dns-resolution-errors from 3445d54a19
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 7s
Checks / Prek / Check changed files (pull_request) Successful in 8s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m14s
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
to 5e82a4eab2
All checks were successful
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 8s
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 13s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 7m49s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 21m19s
2026-11-05 12:38:03 +00:00
Compare
eleboucher approved these changes 2026-11-05 12:51:51 +00:00
silversys force-pushed silversys/fix-dns-resolution-errors from 5e82a4eab2
All checks were successful
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 8s
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 13s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 7m49s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 21m19s
to c4e4376bdf
All checks were successful
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 14s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m15s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m6s
2026-11-05 13:03:24 +00:00
Compare
silversys changed title from WIP: fix: Don't try to use system resolv.conf when other servers are configured to fix: Don't try to use system resolv.conf when other servers are configured 2026-11-05 15:25:24 +00:00
silversys force-pushed silversys/fix-dns-resolution-errors from c4e4376bdf
All checks were successful
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 14s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m15s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m6s
to 1ffb3da4d1
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 6s
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m13s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m16s
2026-11-05 15:44:00 +00:00
Compare
Aranjedeath left a comment

I don't use alternative DNS servers, but the standard path still works for me. no new explosions.

I don't use alternative DNS servers, but the standard path still works for me. no new explosions.
stratself approved these changes 2026-11-05 19:15:34 +00:00
stratself left a comment

WOMM!!

No requests were sent to the /etc/resolv.conf nameservers. I don't have appservices yet but this is great

WOMM!! _No_ requests were sent to the `/etc/resolv.conf` nameservers. I don't have appservices yet but this is great
silversys force-pushed silversys/fix-dns-resolution-errors from 1ffb3da4d1
All checks were successful
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 6s
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 1m13s
Checks / Prek / Check changed files (pull_request) Successful in 6s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 8m16s
to f626232448
All checks were successful
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 8s
Checks / Prek / Check changed files (pull_request) Successful in 10s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m23s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 9m14s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 21m2s
2026-11-05 22:48:42 +00:00
Compare
@ -0,0 +1 @@
Ensure HTTP clients actually follow the `dns_servers` config option. Contributed by @silversys
Owner

.bugfix.md

`.bugfix.md`
@ -0,0 +38,4 @@
Box::pin(async move {
let lookup = resolver.lookup_ip(name.as_str()).await?;
let addrs: Addrs = Box::new(SocketAddrs {
iter: lookup.iter().collect::<Vec<_>>().into_iter(),
Owner

This collect is creating an unnecessary allocation, it's already an iterator?

This `collect` is creating an unnecessary allocation, it's already an iterator?
@ -21,3 +26,3 @@
pub dns_resolver: Arc<TokioResolver>,
pub dns: reqwest::Client,
/// Tokio system resolver wrapped
Owner

Isn't this wrapping the resolver which can prefer the configured resolvers?

Isn't this wrapping the resolver which can prefer the configured resolvers?
@ -24,0 +31,4 @@
/// Matrix server resolver according to server resolution spec
pub matrix_resolver: Arc<MatrixResolver>,
/// HTTP client following matrix server resolution spec for requests
Owner

HTTP client used for fetching files during server resolution

HTTP client used for fetching files during server resolution
@ -231,3 +253,1 @@
.pool_max_idle_per_host(config.request_idle_per_host.into())
.user_agent(conduwuit::user_agent())
.redirect(redirect::Policy::limited(6))
.hickory_dns(true)
Owner

this gets overridden by the resolver, right?

this gets overridden by the resolver, right?
All checks were successful
Documentation / Build and Deploy Documentation (pull_request) Has been skipped
Checks / Changelog / Check changelog is added (pull_request_target) Successful in 8s
Required
Details
Checks / Prek / Check changed files (pull_request) Successful in 10s
Required
Details
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m23s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 9m14s
Required
Details
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 21m2s
Required
Details
This pull request has changes requested by an official reviewer.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u 'silversys/fix-dns-resolution-errors:silversys-silversys/fix-dns-resolution-errors'
git switch 'silversys-silversys/fix-dns-resolution-errors'
Sign in to join this conversation.
No milestone
No assignees
5 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
continuwuation/continuwuity!2460
No description provided.