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

Jump to bottom

net: DNS doesn't work on Windows if environment is empty? #20473

Closed
shhsu opened this issue May 23, 2017 · 7 comments
Closed

net: DNS doesn't work on Windows if environment is empty? #20473

shhsu opened this issue May 23, 2017 · 7 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Unplanned

Comments

@shhsu
Copy link

shhsu commented May 23, 2017 •
edited

Please answer these questions before submitting your issue. Thanks!

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

1.8.1

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

set GOARCH=amd64
set GOBIN=
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=D:\gopath181
set GORACE=
set GOROOT=D:\go181
set GOTOOLDIR=D:\go181\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\shhsu\AppData\Local\Temp\go-build529806931=/tmp/go-build -gno-record-gcc-switches
set CXX=g++
set CGO_ENABLED=1
set PKG_CONFIG=pkg-config
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2

What did you do?

I created a go program that makes http requests. I want to be able to call that program from another go program. When calling the callee, the caller wants to be able to set an environment variable to modify its behavior

Example callee:

func main() {
	thing := os.Getenv("SOMETHING")
	fmt.Printf("something: %s\n", thing)

	var resp *http.Response
	var err error
	if resp, err = http.Get("http://www.google.com"); err != nil {
		fmt.Fprintf(os.Stderr, "Error reaching www.google.com, error: %s\n", err)
	}

	if resp == nil {
		fmt.Printf("resp is nil\n")
	} else {
		fmt.Printf("resp Status code %d\n", resp.StatusCode)
	}
	fmt.Print("Done\n")
}

Example caller:

func main() {
	programCmd := exec.Command(filepath.Join("..", "http"))
	pwd, _ := os.Getwd()
	if len(os.Args) < 2 {
		programCmd.Env = []string{fmt.Sprintf("%s=%s", "SOMETHING", pwd)}
	}
	programCmd.Stderr = os.Stderr
	programCmd.Stdout = os.Stdout
	if err := programCmd.Run(); err != nil {
		fmt.Printf("%s\n", err)
	}
	fmt.Printf("Done")
}

What did you expect to see?

http requests are successfully made

What did you see instead?

If I call caller noEnv everything is fine, the http request was successfully made with 200 status code
If I call caller, the caller would pass the environmental variable SOMETHING={pwd}, causing the following error on the callee

Error reaching www.google.com, error: Get http://www.google.com: dial tcp: lookup www.google.com: getaddrinfow: A non-recoverable error occurred during a database lookup.

Example project on VSCode
@shhsu
Copy link
Author

shhsu commented May 23, 2017

@sajayantony
This is a minor blocker for a feature for Azure Docker Registry

@bradfitz
Copy link
Contributor

You're replacing the entire environment of the child process with exactly 1 environment variable pair, removing all the standard Windows environment variables. Windows things are probably not happy if they lack their entire standard environment.

You should use:

   programCmd.Env = append(programCmd.Env, fmt.Sprintf("%s=%s", "SOMETHING", pwd))

I bet that fixes it.

I don't think we need to guarantee that Go programs work flawlessly in an empty environment.

@alexbrainman, thoughts?

@bradfitz bradfitz changed the title windows/http: setting environment variable for os/exec/Cmd causes http package to malfunction net: DNS doesn't work on Windows if environment is empty? May 23, 2017
@bradfitz bradfitz added this to the Unplanned milestone May 23, 2017
@bradfitz bradfitz added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels May 23, 2017
@shhsu
Copy link
Author

shhsu commented May 23, 2017

@bradfitz
Thanks for the quick response. Actually that append(...) was the first thing I tried. However, that was not the issue. Appending programCmd.Env does not fix the issue,

@bradfitz
Copy link
Contributor

Please post a complete but minimal self-contained repro.

@shhsu
Copy link
Author

shhsu commented May 23, 2017

http.zip
(Ah... Just figured out how to post attachment...)

@alexbrainman
Copy link
Member

Actually that append(...) was the first thing I tried. However, that was not the issue. Appending programCmd.Env does not fix the issue,

programCmd.Env is set to nil at the start, so appending 1 string to it, makes your environment have 1 environment variable - and that (as @bradfitz suggested) is not acceptable under Windows - some Windows APIs use these environment variables that you delete. Try this instead:

d:\a\src\issues\issue20473\caller>type program.go
package main

import (
        "fmt"
        "os"
        "os/exec"
        "path/filepath"
)

func main() {
        programCmd := exec.Command(filepath.Join("..", "http"))
        pwd, _ := os.Getwd()
        programCmd.Env = append(os.Environ(), fmt.Sprintf("%s=%s", "SOMETHING",pwd)) // []string{fmt.Sprintf("%s=%s", "SOMETHING", pwd)}
        programCmd.Stderr = os.Stderr
        programCmd.Stdout = os.Stdout
        if err := programCmd.Run(); err != nil {
                fmt.Printf("%s\n", err)
        }
        fmt.Printf("Done")
}

d:\a\src\issues\issue20473\caller>type ..\program.go
package main

import (
        "fmt"
        "net/http"
        "os"
)

func main() {
        thing := os.Getenv("SOMETHING")
        fmt.Printf("something: %s\n", thing)

        var resp *http.Response
        var err error
        if resp, err = http.Get("http://www.google.com"); err != nil {
                fmt.Fprintf(os.Stderr, "Error reaching www.google.com, error: %s\n", err)
        }

        if resp == nil {
                fmt.Printf("resp is nil\n")
        } else {
                fmt.Printf("resp Status code %d\n", resp.StatusCode)
        }
        fmt.Print("Done\n")
}

d:\a\src\issues\issue20473\caller>go build -o %GOPATH%\src\issues\issue20473\http.exe issues/issue20473 && go build -o a.exe && a
something: d:\a\src\issues\issue20473\caller
resp Status code 200
Done
Done
d:\a\src\issues\issue20473\caller>

Alex

@bradfitz
Copy link
Contributor

Oh, whoops, thanks @alexbrainman. That's what I meant to write above but I was spacing out.

Closing this since it looks like program error and/or something not fixable on Windows anyway.

Comment if you disagree.

@golang golang locked and limited conversation to collaborators May 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@bradfitz @shhsu @gopherbot @alexbrainman