Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add app.configureHostResolver #30576

Merged
merged 7 commits into from
Aug 31, 2021
Merged

feat: add app.configureHostResolver #30576

merged 7 commits into from
Aug 31, 2021

Conversation

nornagon
Copy link
Member

@nornagon nornagon commented Aug 17, 2021 •
edited

Description of Change

This adds a new API, app.configureHostResolver, and also configures the host
resolver slightly differently by default:

  1. The built-in resolver is used in preference to getaddrinfo when the
    AsyncDns feature flag is enabled, which it is by default on macOS.
  2. DNS-over-HTTPS is enabled by default, which will only have an effect when
    the system provider is one of the known-to-Chromium DoH providers.
    Additionally, this respects the DnsOverHttps feature flag, along with the
    Fallback param, e.g. --enable-features=DnsOverHttps:Fallback/false to
    disable fallback to insecure DNS.
  3. DoH servers can be specified on the command line using the Templates
    feature param, e.g. --enable-features=DnsOverHttps:Templates/https%3A%2F%2Fdoh.opendns.com%2Fdns-query

The command-line functionality is left undocumented as (1) it matches Chrome
and (2) it's unclear how stable that feature is in Chrome.

Ref #21064.

Checklist

Release Notes

Notes: Added app.configureHostResolver API for configuring DNS-over-HTTPS.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Aug 17, 2021
@nornagon nornagon added the semver/minor backwards-compatible functionality label Aug 17, 2021
@nornagon nornagon mentioned this pull request Aug 17, 2021
Closed
3 tasks
@jkleinsc
Copy link
Contributor

API LGTM

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Aug 24, 2021
itsananderson
itsananderson approved these changes Aug 26, 2021
View reviewed changes
Copy link
Contributor

@itsananderson itsananderson left a comment

Choose a reason for hiding this comment

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

Feedback is minor

API LGTM

docs/api/app.md Show resolved Hide resolved
shell/browser/api/electron_api_app.cc Show resolved Hide resolved
shell/browser/api/electron_api_app.cc Show resolved Hide resolved
ckerr
ckerr approved these changes Aug 26, 2021
View reviewed changes
docs/api/app.md Show resolved Hide resolved
docs/api/app.md Outdated Show resolved Hide resolved
docs/api/app.md Outdated Show resolved Hide resolved
shell/browser/net/system_network_context_manager.cc Show resolved Hide resolved
@codebytere
Copy link
Member

API LGTM

@nornagon
Copy link
Member Author

CI failures are unrelated. Merging.

nornagon
nornagon commented Aug 31, 2021
View reviewed changes
docs/api/app.md Outdated Show resolved Hide resolved
@nornagon nornagon merged commit dd7aeda into main Aug 31, 2021
@nornagon nornagon deleted the configure-host-resolver branch August 31, 2021 18:55
@release-clerk
Copy link

release-clerk bot commented Aug 31, 2021

Release Notes Persisted

Added app.configureHostResolver API for configuring DNS-over-HTTPS.

@commit-lint commit-lint bot mentioned this pull request Aug 31, 2021
Open
@trop trop bot mentioned this pull request Aug 31, 2021
Merged
trop bot pushed a commit that referenced this pull request Aug 31, 2021
@nornagon @electron-bot
feat: add app.configureHostResolver (#30576)
0026d49
@trop
Copy link
Contributor

trop bot commented Aug 31, 2021

I have automatically backported this PR to "15-x-y", please check out #30775

@jkleinsc jkleinsc mentioned this pull request Sep 1, 2021
Merged
@nornagon nornagon mentioned this pull request Sep 1, 2021
Merged
jkleinsc pushed a commit that referenced this pull request Sep 2, 2021
@trop @nornagon
feat: add app.configureHostResolver (#30775)
4fb3b58 
* feat: add app.configureHostResolver (#30576)

* fix tests

Co-authored-by: Jeremy Rose <jeremya@chromium.org>
Co-authored-by: Jeremy Rose <nornagon@nornagon.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ckerr ckerr ckerr approved these changes

@itsananderson itsananderson itsananderson approved these changes

@jkleinsc jkleinsc jkleinsc left review comments

@codebytere codebytere codebytere approved these changes

Assignees
No one assigned
Labels
api-review/requested 🗳 semver/minor backwards-compatible functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@nornagon @jkleinsc @codebytere @ckerr @itsananderson