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.

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

net: “Name Resolution” documentation does not describe default behavior on Windows #57663

Open
raggi opened this issue Jan 6, 2023 · 10 comments
Labels
Documentation help wanted NeedsFix The path to resolution is known, but the work has not been done. OS-Windows
Milestone

Comments

@raggi
Copy link
Contributor

raggi commented Jan 6, 2023

What version of Go are you using (go version)?

$ go version
go version go1.19.3 linux/amd64

Does this issue reproduce with the latest release?

Yes:

On Windows, in Go 1.18.x and earlier, the resolver always used C

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/raggi/.cache/go-build"
GOENV="/home/raggi/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/raggi/go/pkg/mod"
GOOS="linux"
GOPATH="/home/raggi/go"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19.3"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build370676385=/tmp/go-build -gno-record-gcc-switches"

(GONOPROXY and related redacted for privacy)

What did you do?

I was reading the net/ docs to try to understand some details around DoT/DoH, and came across this statement:

On Windows, in Go 1.18.x and earlier, the resolver always used C library functions, such as GetAddrInfo and DnsQuery.

This statement is somewhat confusing, as it is not clear how it pertains to my current Go version 1.19.3.

What did you expect to see?

I would prefer to see a statement that describes the behavior of my Go version.

What did you see instead?

I saw a statement that describes past versions only.

@bcmills
Copy link
Contributor

bcmills commented Jan 6, 2023

That line was changed to the past tense in CL 409234 (attn @bradfitz @ianlancetaylor).

It does seem to have dropped the description of the default behavior on Windows. 😅

@bcmills bcmills changed the title affected/package: net net: “Name Resolution” documentation does not describe default behavior on Windows Jan 6, 2023
@bcmills bcmills added Documentation help wanted OS-Windows NeedsFix The path to resolution is known, but the work has not been done. labels Jan 6, 2023
@bcmills bcmills added this to the Backlog milestone Jan 6, 2023
@bcmills
Copy link
Contributor

bcmills commented Jan 6, 2023

If I understand #33097 correctly, on Windows the resolver still defaults to those C library functions, but can be overridden to a pure-Go implementation by setting GODEBUG=netdns=go or Resolver.PreferGo.

@ianlancetaylor
Copy link
Contributor

The behavior on 1.19 and above is the behavior described in the lengthy comment for which that sentence is the last paragraph.

Any suggestions on how to make that clearer?

@mateusz834
Copy link
Member

I think that we should mention than windows (by default) regardless of the CGO_ENABLED value uses GetAddrInfo syscall directly. Note: #53490

@ianlancetaylor
Copy link
Contributor

Note that that is true on other systems at all, such as Darwin.

@RyanMuzzey
Copy link

I'd like to tackle this if it's a good first issue. How should the doc read for net.go to account for default behavior in Windows/Darwin/etc.?

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Jan 9, 2023

I think there are two points here.

The first is that the final sentence in the comment suggests that we have failed to document the behavior of Go 1.19 and later on Windows. We should perhaps somehow indicate that the rest of the comment applies.

The second is a point that the comment currently doesn't mention at all. When cgo is not supported (because CGO_ENABLED is set to 0, which happens by default if there is no C compiler when the Go distribution is built), then on most systems only GODEBUG=netdns=go is available. However, on Windows and Darwin GODEBUG=netdns=cgo works even if cgo is not supported.

Any changes here need to be both precise and terse. We do not want several sentences discussing these unusual situations.

Thanks.

@bcmills
Copy link
Contributor

bcmills commented Jan 9, 2023

I think part of the problem is that the second paragraph begins with “On Unix systems,” and the third paragraph seems to only clarify what came before it. So the third paragraph reads as though it applies only to Unix.

But, it seems to me that that paragraph still doesn't apply to Windows. The build constraint in netgo.go excludes darwin and windows, the logic in net.initConfVal sets netCgo by default on windows and plan9, and (*net.conf).hostLookupOrder also has special cases for android, windows, and plan9.


From what I understand from reading the code, the behavior seems to be:

  • On all platforms, there are two resolver implementations: one in pure Go, and one using the platform's native API (typically a C library).
  • On Unix systems other than macOS, the behavior is as described in the long paragraph today. (“By default the pure Go resolver is used,” but “the cgo-based resolver is used instead under a variety of conditions”.)
  • On macOS, Windows, and Plan 9, the platform's resolver is used by default. (The pure Go resolver is used only if it is explicitly enabled.)
    • On these platforms, the platform's resolver does not actually require the use of cgo or CGO_ENABLED, although it may call into a C library.
    • It is not clear to me why the code to choose the resolver uses a different approach on macOS (setting forceCgoLookupHost) than the other two (setting confVal.netCgo).

@RyanMuzzey
Copy link

@bcmills Thanks! I have some text changes I'd like to get feedback on, but I think I have all the points covered.

Between the first and second paragraphs, I'm adding:
All platforms have the options of using either a pure go, or a platform native resolver (typically a C library). .

The third paragraph "On Unix..." is unchanged as well as the long description following it.

I added this paragraph after the description:
For macOS, Windows, and Plan 9, the platform's resolver is used by default. On these platforms, the pure go resolver will be used only if configured as the example below shows. The platform's resolver does not require the use of cgo, or CGO_ENABLED, although they may still call into a c library. Platform specific details follow.
and moved the platform specific details up to follow this new paragraph.

I can submit a PR for the changes, but just wanted to get some feedback on the text before I commit what I have. Thanks again!

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/477155 mentions this issue: net: This change updates the name resolution documentation to be more concise on how the code behaves for the different platforms. Fixes #57663

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation help wanted NeedsFix The path to resolution is known, but the work has not been done. OS-Windows
Projects
None yet
Development

No branches or pull requests

6 participants