diff --git a/VERSION b/VERSION index 74f07e53c7161b..e050468bb03737 100644 --- a/VERSION +++ b/VERSION @@ -1,2 +1,2 @@ -go1.21.4 -time 2023-11-01T20:46:39Z +go1.21.5 +time 2023-11-29T21:21:46Z diff --git a/src/cmd/compile/internal/ssa/loopbce.go b/src/cmd/compile/internal/ssa/loopbce.go index b7dfaa33e3bf46..7f432e61ba5078 100644 --- a/src/cmd/compile/internal/ssa/loopbce.go +++ b/src/cmd/compile/internal/ssa/loopbce.go @@ -127,6 +127,13 @@ func findIndVar(f *Func) []indVar { less = false } + if ind.Block != b { + // TODO: Could be extended to include disjointed loop headers. + // I don't think this is causing missed optimizations in real world code often. + // See https://go.dev/issue/63955 + continue + } + // Expect the increment to be a nonzero constant. if !inc.isGenericIntConst() { continue diff --git a/src/cmd/go/internal/modcmd/download.go b/src/cmd/go/internal/modcmd/download.go index e49cd9fe0c1c5c..373accef067961 100644 --- a/src/cmd/go/internal/modcmd/download.go +++ b/src/cmd/go/internal/modcmd/download.go @@ -10,6 +10,7 @@ import ( "errors" "os" "runtime" + "sync" "cmd/go/internal/base" "cmd/go/internal/cfg" @@ -17,6 +18,7 @@ import ( "cmd/go/internal/modfetch" "cmd/go/internal/modfetch/codehost" "cmd/go/internal/modload" + "cmd/go/internal/toolchain" "golang.org/x/mod/module" ) @@ -153,7 +155,10 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) { // 'go mod graph', and similar commands. _, err := modload.LoadModGraph(ctx, "") if err != nil { - base.Fatal(err) + // TODO(#64008): call base.Fatalf instead of toolchain.SwitchOrFatal + // here, since we can only reach this point with an outdated toolchain + // if the go.mod file is inconsistent. + toolchain.SwitchOrFatal(ctx, err) } for _, m := range modFile.Require { @@ -194,8 +199,26 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) { // from the resulting TooNewError), all before we try the actual full download // of each module. // - // For now, we just let it fail: the user can explicitly set GOTOOLCHAIN - // and retry if they want to. + // For now, we go ahead and try all the downloads and collect the errors, and + // if any download failed due to a TooNewError, we switch toolchains and try + // again. Any downloads that already succeeded will still be in cache. + // That won't give optimal concurrency (we'll do two batches of concurrent + // downloads instead of all in one batch), and it might add a little overhead + // to look up the downloads from the first batch in the module cache when + // we see them again in the second batch. On the other hand, it's way simpler + // to implement, and not really any more expensive if the user is requesting + // no explicit arguments (their go.mod file should already list an appropriate + // toolchain version) or only one module (as is used by the Go Module Proxy). + + if infosErr != nil { + var sw toolchain.Switcher + sw.Error(infosErr) + if sw.NeedSwitch() { + sw.Switch(ctx) + } + // Otherwise, wait to report infosErr after we have downloaded + // when we can. + } if !haveExplicitArgs && modload.WorkFilePath() == "" { // 'go mod download' is sometimes run without arguments to pre-populate the @@ -205,7 +228,7 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) { // (golang.org/issue/45332). We do still fix inconsistencies in go.mod // though. // - // TODO(#45551): In the future, report an error if go.mod or go.sum need to + // TODO(#64008): In the future, report an error if go.mod or go.sum need to // be updated after loading the build list. This may require setting // the mode to "mod" or "readonly" depending on haveExplicitArgs. if err := modload.WriteGoMod(ctx, modload.WriteOpts{}); err != nil { @@ -213,6 +236,7 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) { } } + var downloadErrs sync.Map for _, info := range infos { if info.Replace != nil { info = info.Replace @@ -239,7 +263,11 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) { } sem <- token{} go func() { - DownloadModule(ctx, m) + err := DownloadModule(ctx, m) + if err != nil { + downloadErrs.Store(m, err) + m.Error = err.Error() + } <-sem }() } @@ -249,6 +277,39 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) { sem <- token{} } + // If there were explicit arguments + // (like 'go mod download golang.org/x/tools@latest'), + // check whether we need to upgrade the toolchain in order to download them. + // + // (If invoked without arguments, we expect the module graph to already + // be tidy and the go.mod file to declare a 'go' version that satisfies + // transitive requirements. If that invariant holds, then we should have + // already upgraded when we loaded the module graph, and should not need + // an additional check here. See https://go.dev/issue/45551.) + // + // We also allow upgrades if in a workspace because in workspace mode + // with no arguments we download the module pattern "all", + // which may include dependencies that are normally pruned out + // of the individual modules in the workspace. + if haveExplicitArgs || modload.WorkFilePath() != "" { + var sw toolchain.Switcher + // Add errors to the Switcher in deterministic order so that they will be + // logged deterministically. + for _, m := range mods { + if erri, ok := downloadErrs.Load(m); ok { + sw.Error(erri.(error)) + } + } + // Only call sw.Switch if it will actually switch. + // Otherwise, we may want to write the errors as JSON + // (instead of using base.Error as sw.Switch would), + // and we may also have other errors to report from the + // initial infos returned by ListModules. + if sw.NeedSwitch() { + sw.Switch(ctx) + } + } + if *downloadJSON { for _, m := range mods { b, err := json.MarshalIndent(m, "", "\t") @@ -302,34 +363,27 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) { // DownloadModule runs 'go mod download' for m.Path@m.Version, // leaving the results (including any error) in m itself. -func DownloadModule(ctx context.Context, m *ModuleJSON) { +func DownloadModule(ctx context.Context, m *ModuleJSON) error { var err error _, file, err := modfetch.InfoFile(ctx, m.Path, m.Version) if err != nil { - m.Error = err.Error() - return + return err } m.Info = file m.GoMod, err = modfetch.GoModFile(ctx, m.Path, m.Version) if err != nil { - m.Error = err.Error() - return + return err } m.GoModSum, err = modfetch.GoModSum(ctx, m.Path, m.Version) if err != nil { - m.Error = err.Error() - return + return err } mod := module.Version{Path: m.Path, Version: m.Version} m.Zip, err = modfetch.DownloadZip(ctx, mod) if err != nil { - m.Error = err.Error() - return + return err } m.Sum = modfetch.Sum(ctx, mod) m.Dir, err = modfetch.Download(ctx, mod) - if err != nil { - m.Error = err.Error() - return - } + return err } diff --git a/src/cmd/go/internal/vcs/vcs.go b/src/cmd/go/internal/vcs/vcs.go index c65dd0f624a1b6..dbf16d1de7f375 100644 --- a/src/cmd/go/internal/vcs/vcs.go +++ b/src/cmd/go/internal/vcs/vcs.go @@ -1204,18 +1204,31 @@ func repoRootFromVCSPaths(importPath string, security web.SecurityMode, vcsPaths var ok bool repoURL, ok = interceptVCSTest(repo, vcs, security) if !ok { - scheme := vcs.Scheme[0] // default to first scheme - if vcs.PingCmd != "" { - // If we know how to test schemes, scan to find one. + scheme, err := func() (string, error) { for _, s := range vcs.Scheme { if security == web.SecureOnly && !vcs.isSecureScheme(s) { continue } - if vcs.Ping(s, repo) == nil { - scheme = s - break + + // If we know how to ping URL schemes for this VCS, + // check that this repo works. + // Otherwise, default to the first scheme + // that meets the requested security level. + if vcs.PingCmd == "" { + return s, nil + } + if err := vcs.Ping(s, repo); err == nil { + return s, nil } } + securityFrag := "" + if security == web.SecureOnly { + securityFrag = "secure " + } + return "", fmt.Errorf("no %sprotocol found for repository", securityFrag) + }() + if err != nil { + return nil, err } repoURL = scheme + "://" + repo } diff --git a/src/cmd/go/testdata/script/gotoolchain_modcmds.txt b/src/cmd/go/testdata/script/gotoolchain_modcmds.txt index 83b75f0abbdea1..1edd6d85a5e66c 100644 --- a/src/cmd/go/testdata/script/gotoolchain_modcmds.txt +++ b/src/cmd/go/testdata/script/gotoolchain_modcmds.txt @@ -3,25 +3,18 @@ env TESTGO_VERSION_SWITCH=switch # If the main module's go.mod file lists a version lower than the version # required by its dependencies, the commands that fetch and diagnose the module -# graph (such as 'go mod download' and 'go mod graph') should fail explicitly: +# graph (such as 'go mod graph' and 'go mod verify') should fail explicitly: # they can't interpret the graph themselves, and they aren't allowed to update # the go.mod file to record a specific, stable toolchain version that can. -! go mod download rsc.io/future@v1.0.0 -stderr '^go: rsc.io/future@v1.0.0 requires go >= 1.999 \(running go 1.21.0\)' - -! go mod download rsc.io/future -stderr '^go: rsc.io/future@v1.0.0 requires go >= 1.999 \(running go 1.21.0\)' - -! go mod download -stderr '^go: rsc.io/future@v1.0.0: module rsc.io/future@v1.0.0 requires go >= 1.999 \(running go 1.21.0\)' - ! go mod verify stderr '^go: rsc.io/future@v1.0.0: module rsc.io/future@v1.0.0 requires go >= 1.999 \(running go 1.21.0\)' ! go mod graph stderr '^go: rsc.io/future@v1.0.0: module rsc.io/future@v1.0.0 requires go >= 1.999 \(running go 1.21.0\)' +# TODO(#64008): 'go mod download' without arguments should fail too. + # 'go get' should update the main module's go.mod file to a version compatible with the # go version required for rsc.io/future, not fail. @@ -33,8 +26,6 @@ stderr '^go: added toolchain go1.999testmod$' # Now, the various 'go mod' subcommands should succeed. -go mod download rsc.io/future@v1.0.0 -go mod download rsc.io/future go mod download go mod verify diff --git a/src/cmd/go/testdata/script/mod_download_exec_toolchain.txt b/src/cmd/go/testdata/script/mod_download_exec_toolchain.txt new file mode 100644 index 00000000000000..6cf863b28a28f6 --- /dev/null +++ b/src/cmd/go/testdata/script/mod_download_exec_toolchain.txt @@ -0,0 +1,107 @@ +env TESTGO_VERSION=go1.21 +env TESTGO_VERSION_SWITCH=switch + +# First, test 'go mod download' outside of a module. +# +# There is no go.mod file into which we can record the selected toolchain, +# so unfortunately these version switches won't be as reproducible as other +# go commands, but that's still preferable to failing entirely or downloading +# a module zip that we don't understand. + +# GOTOOLCHAIN=auto should run the newer toolchain +env GOTOOLCHAIN=auto +go mod download rsc.io/needgo121@latest rsc.io/needgo122@latest rsc.io/needgo123@latest rsc.io/needall@latest +stderr '^go: rsc.io/needall@v0.0.1 requires go >= 1.23; switching to go1.23.9$' +! stderr '\(running' + +# GOTOOLCHAIN=min+auto should run the newer toolchain +env GOTOOLCHAIN=go1.21+auto +go mod download rsc.io/needgo121@latest rsc.io/needgo122@latest rsc.io/needgo123@latest rsc.io/needall@latest +stderr '^go: rsc.io/needall@v0.0.1 requires go >= 1.23; switching to go1.23.9$' +! stderr '\(running' + +# GOTOOLCHAIN=go1.21 should NOT run the newer toolchain +env GOTOOLCHAIN=go1.21 +! go mod download rsc.io/needgo121@latest rsc.io/needgo122@latest rsc.io/needgo123@latest rsc.io/needall@latest +! stderr switching +stderr 'rsc.io/needgo122@v0.0.1 requires go >= 1.22' +stderr 'rsc.io/needgo123@v0.0.1 requires go >= 1.23' +stderr 'rsc.io/needall@v0.0.1 requires go >= 1.23' +stderr 'requires go >= 1.23' +! stderr 'requires go >= 1.21' # that's us! + + +# JSON output should be emitted exactly once, +# and non-JSON output should go to stderr instead of stdout. +env GOTOOLCHAIN=auto +go mod download -json rsc.io/needgo121@latest rsc.io/needgo122@latest rsc.io/needgo123@latest rsc.io/needall@latest +stderr '^go: rsc.io/needall@v0.0.1 requires go >= 1.23; switching to go1.23.9$' +! stderr '\(running' +stdout -count=1 '"Path": "rsc.io/needgo121",' +stdout -count=1 '"Path": "rsc.io/needgo122",' +stdout -count=1 '"Path": "rsc.io/needgo123",' +stdout -count=1 '"Path": "rsc.io/needall",' + +# GOTOOLCHAIN=go1.21 should write the errors in the JSON Error fields, not to stderr. +env GOTOOLCHAIN=go1.21 +! go mod download -json rsc.io/needgo121@latest rsc.io/needgo122@latest rsc.io/needgo123@latest rsc.io/needall@latest +! stderr switching +stdout -count=1 '"Error": "rsc.io/needgo122@v0.0.1 requires go .*= 1.22 \(running go 1.21; GOTOOLCHAIN=go1.21\)"' +stdout -count=1 '"Error": "rsc.io/needgo123@v0.0.1 requires go .*= 1.23 \(running go 1.21; GOTOOLCHAIN=go1.21\)"' +stdout -count=1 '"Error": "rsc.io/needall@v0.0.1 requires go .*= 1.23 \(running go 1.21; GOTOOLCHAIN=go1.21\)"' +! stdout '"Error": "rsc.io/needgo121' # We can handle this one. +! stderr . + + +# Within a module, 'go mod download' of explicit versions should upgrade if +# needed to perform the download, but should not change the main module's +# toolchain version (because the downloaded modules are still not required by +# the main module). + +cd example +cp go.mod go.mod.orig + +env GOTOOLCHAIN=auto +go mod download rsc.io/needgo121@latest rsc.io/needgo122@latest rsc.io/needgo123@latest rsc.io/needall@latest +stderr '^go: rsc.io/needall@v0.0.1 requires go >= 1.23; switching to go1.23.9$' +! stderr '\(running' +cmp go.mod go.mod.orig + + +# However, 'go mod download' without arguments should fix up the +# 'go' and 'toolchain' lines to be consistent with the existing +# requirements in the module graph. + +go mod edit -require=rsc.io/needall@v0.0.1 +cp go.mod go.mod.121 + +# If an upgrade is needed, GOTOOLCHAIN=go1.21 should cause +# the command to fail without changing go.mod. + +env GOTOOLCHAIN=go1.21 +! go mod download +stderr 'rsc.io/needall@v0.0.1 requires go >= 1.23' +! stderr switching +cmp go.mod go.mod.121 + +# If an upgrade is needed, GOTOOLCHAIN=auto should perform +# the upgrade and record the resulting toolchain version. + +env GOTOOLCHAIN=auto +go mod download +stderr '^go: module rsc.io/needall@v0.0.1 requires go >= 1.23; switching to go1.23.9$' +cmp go.mod go.mod.final + + +-- example/go.mod -- +module example + +go 1.21 +-- example/go.mod.final -- +module example + +go 1.23 + +toolchain go1.23.9 + +require rsc.io/needall v0.0.1 diff --git a/src/cmd/go/testdata/script/mod_get_future.txt b/src/cmd/go/testdata/script/mod_get_future.txt index 72c0b978046009..3f1c777d967953 100644 --- a/src/cmd/go/testdata/script/mod_get_future.txt +++ b/src/cmd/go/testdata/script/mod_get_future.txt @@ -1,6 +1,7 @@ env TESTGO_VERSION=go1.21 +env GOTOOLCHAIN=local ! go mod download rsc.io/future@v1.0.0 -stderr '^go: rsc.io/future@v1.0.0 requires go >= 1.999 \(running go 1.21\)$' +stderr '^go: rsc.io/future@v1.0.0 requires go >= 1.999 \(running go 1.21; GOTOOLCHAIN=local\)$' -- go.mod -- module m diff --git a/src/cmd/go/testdata/script/mod_insecure_issue63845.txt b/src/cmd/go/testdata/script/mod_insecure_issue63845.txt new file mode 100644 index 00000000000000..c051c05f53931b --- /dev/null +++ b/src/cmd/go/testdata/script/mod_insecure_issue63845.txt @@ -0,0 +1,28 @@ +# Regression test for https://go.dev/issue/63845: +# If 'git ls-remote' fails for all secure protocols, +# we should fail instead of falling back to an arbitrary protocol. +# +# Note that this test does not use the local vcweb test server +# (vcs-test.golang.org), because the hook for redirecting to that +# server bypasses the "ping to determine protocol" logic +# in cmd/go/internal/vcs. + +[!net:golang.org] skip +[!git] skip +[short] skip 'tries to access a nonexistent external Git repo' + +env GOPRIVATE=golang.org +env CURLOPT_TIMEOUT_MS=100 +env GIT_SSH_COMMAND=false + +! go get -x golang.org/nonexist.git@latest +stderr '^git ls-remote https://golang.org/nonexist$' +stderr '^git ls-remote git\+ssh://golang.org/nonexist' +stderr '^git ls-remote ssh://golang.org/nonexist$' +! stderr 'git://' +stderr '^go: golang.org/nonexist.git@latest: no secure protocol found for repository$' + +-- go.mod -- +module example + +go 1.19 diff --git a/src/crypto/rand/rand.go b/src/crypto/rand/rand.go index 62738e2cb1a7df..d0dcc7cc71fc0c 100644 --- a/src/crypto/rand/rand.go +++ b/src/crypto/rand/rand.go @@ -15,7 +15,7 @@ import "io" // available, /dev/urandom otherwise. // On OpenBSD and macOS, Reader uses getentropy(2). // On other Unix-like systems, Reader reads from /dev/urandom. -// On Windows systems, Reader uses the RtlGenRandom API. +// On Windows systems, Reader uses the ProcessPrng API. // On JS/Wasm, Reader uses the Web Crypto API. // On WASIP1/Wasm, Reader uses random_get from wasi_snapshot_preview1. var Reader io.Reader diff --git a/src/crypto/rand/rand_windows.go b/src/crypto/rand/rand_windows.go index 6c0655c72b692a..7380f1f0f1e6e6 100644 --- a/src/crypto/rand/rand_windows.go +++ b/src/crypto/rand/rand_windows.go @@ -15,11 +15,8 @@ func init() { Reader = &rngReader{} } type rngReader struct{} -func (r *rngReader) Read(b []byte) (n int, err error) { - // RtlGenRandom only returns 1<<32-1 bytes at a time. We only read at - // most 1<<31-1 bytes at a time so that this works the same on 32-bit - // and 64-bit systems. - if err := batched(windows.RtlGenRandom, 1<<31-1)(b); err != nil { +func (r *rngReader) Read(b []byte) (int, error) { + if err := windows.ProcessPrng(b); err != nil { return 0, err } return len(b), nil diff --git a/src/internal/syscall/windows/syscall_windows.go b/src/internal/syscall/windows/syscall_windows.go index 892b878d165dd5..e9390b07cd40a1 100644 --- a/src/internal/syscall/windows/syscall_windows.go +++ b/src/internal/syscall/windows/syscall_windows.go @@ -373,7 +373,7 @@ func ErrorLoadingGetTempPath2() error { //sys DestroyEnvironmentBlock(block *uint16) (err error) = userenv.DestroyEnvironmentBlock //sys CreateEvent(eventAttrs *SecurityAttributes, manualReset uint32, initialState uint32, name *uint16) (handle syscall.Handle, err error) = kernel32.CreateEventW -//sys RtlGenRandom(buf []byte) (err error) = advapi32.SystemFunction036 +//sys ProcessPrng(buf []byte) (err error) = bcryptprimitives.ProcessPrng //sys RtlLookupFunctionEntry(pc uintptr, baseAddress *uintptr, table *byte) (ret uintptr) = kernel32.RtlLookupFunctionEntry //sys RtlVirtualUnwind(handlerType uint32, baseAddress uintptr, pc uintptr, entry uintptr, ctxt uintptr, data *uintptr, frame *uintptr, ctxptrs *byte) (ret uintptr) = kernel32.RtlVirtualUnwind diff --git a/src/internal/syscall/windows/zsyscall_windows.go b/src/internal/syscall/windows/zsyscall_windows.go index a5c246b773d398..26ec290e026c1a 100644 --- a/src/internal/syscall/windows/zsyscall_windows.go +++ b/src/internal/syscall/windows/zsyscall_windows.go @@ -37,13 +37,14 @@ func errnoErr(e syscall.Errno) error { } var ( - modadvapi32 = syscall.NewLazyDLL(sysdll.Add("advapi32.dll")) - modiphlpapi = syscall.NewLazyDLL(sysdll.Add("iphlpapi.dll")) - modkernel32 = syscall.NewLazyDLL(sysdll.Add("kernel32.dll")) - modnetapi32 = syscall.NewLazyDLL(sysdll.Add("netapi32.dll")) - modpsapi = syscall.NewLazyDLL(sysdll.Add("psapi.dll")) - moduserenv = syscall.NewLazyDLL(sysdll.Add("userenv.dll")) - modws2_32 = syscall.NewLazyDLL(sysdll.Add("ws2_32.dll")) + modadvapi32 = syscall.NewLazyDLL(sysdll.Add("advapi32.dll")) + modbcryptprimitives = syscall.NewLazyDLL(sysdll.Add("bcryptprimitives.dll")) + modiphlpapi = syscall.NewLazyDLL(sysdll.Add("iphlpapi.dll")) + modkernel32 = syscall.NewLazyDLL(sysdll.Add("kernel32.dll")) + modnetapi32 = syscall.NewLazyDLL(sysdll.Add("netapi32.dll")) + modpsapi = syscall.NewLazyDLL(sysdll.Add("psapi.dll")) + moduserenv = syscall.NewLazyDLL(sysdll.Add("userenv.dll")) + modws2_32 = syscall.NewLazyDLL(sysdll.Add("ws2_32.dll")) procAdjustTokenPrivileges = modadvapi32.NewProc("AdjustTokenPrivileges") procDuplicateTokenEx = modadvapi32.NewProc("DuplicateTokenEx") @@ -52,7 +53,7 @@ var ( procOpenThreadToken = modadvapi32.NewProc("OpenThreadToken") procRevertToSelf = modadvapi32.NewProc("RevertToSelf") procSetTokenInformation = modadvapi32.NewProc("SetTokenInformation") - procSystemFunction036 = modadvapi32.NewProc("SystemFunction036") + procProcessPrng = modbcryptprimitives.NewProc("ProcessPrng") procGetAdaptersAddresses = modiphlpapi.NewProc("GetAdaptersAddresses") procCreateEventW = modkernel32.NewProc("CreateEventW") procGetACP = modkernel32.NewProc("GetACP") @@ -148,12 +149,12 @@ func SetTokenInformation(tokenHandle syscall.Token, tokenInformationClass uint32 return } -func RtlGenRandom(buf []byte) (err error) { +func ProcessPrng(buf []byte) (err error) { var _p0 *byte if len(buf) > 0 { _p0 = &buf[0] } - r1, _, e1 := syscall.Syscall(procSystemFunction036.Addr(), 2, uintptr(unsafe.Pointer(_p0)), uintptr(len(buf)), 0) + r1, _, e1 := syscall.Syscall(procProcessPrng.Addr(), 2, uintptr(unsafe.Pointer(_p0)), uintptr(len(buf)), 0) if r1 == 0 { err = errnoErr(e1) } diff --git a/src/net/http/internal/chunked.go b/src/net/http/internal/chunked.go index 5a174415dc4014..aad8e5aa09ebcc 100644 --- a/src/net/http/internal/chunked.go +++ b/src/net/http/internal/chunked.go @@ -39,7 +39,8 @@ type chunkedReader struct { n uint64 // unread bytes in chunk err error buf [2]byte - checkEnd bool // whether need to check for \r\n chunk footer + checkEnd bool // whether need to check for \r\n chunk footer + excess int64 // "excessive" chunk overhead, for malicious sender detection } func (cr *chunkedReader) beginChunk() { @@ -49,10 +50,36 @@ func (cr *chunkedReader) beginChunk() { if cr.err != nil { return } + cr.excess += int64(len(line)) + 2 // header, plus \r\n after the chunk data + line = trimTrailingWhitespace(line) + line, cr.err = removeChunkExtension(line) + if cr.err != nil { + return + } cr.n, cr.err = parseHexUint(line) if cr.err != nil { return } + // A sender who sends one byte per chunk will send 5 bytes of overhead + // for every byte of data. ("1\r\nX\r\n" to send "X".) + // We want to allow this, since streaming a byte at a time can be legitimate. + // + // A sender can use chunk extensions to add arbitrary amounts of additional + // data per byte read. ("1;very long extension\r\nX\r\n" to send "X".) + // We don't want to disallow extensions (although we discard them), + // but we also don't want to allow a sender to reduce the signal/noise ratio + // arbitrarily. + // + // We track the amount of excess overhead read, + // and produce an error if it grows too large. + // + // Currently, we say that we're willing to accept 16 bytes of overhead per chunk, + // plus twice the amount of real data in the chunk. + cr.excess -= 16 + (2 * int64(cr.n)) + cr.excess = max(cr.excess, 0) + if cr.excess > 16*1024 { + cr.err = errors.New("chunked encoding contains too much non-data") + } if cr.n == 0 { cr.err = io.EOF } @@ -140,11 +167,6 @@ func readChunkLine(b *bufio.Reader) ([]byte, error) { if len(p) >= maxLineLength { return nil, ErrLineTooLong } - p = trimTrailingWhitespace(p) - p, err = removeChunkExtension(p) - if err != nil { - return nil, err - } return p, nil } diff --git a/src/net/http/internal/chunked_test.go b/src/net/http/internal/chunked_test.go index 5e29a786dd61e4..b99090c1f8ad73 100644 --- a/src/net/http/internal/chunked_test.go +++ b/src/net/http/internal/chunked_test.go @@ -239,3 +239,62 @@ func TestChunkEndReadError(t *testing.T) { t.Errorf("expected %v, got %v", readErr, err) } } + +func TestChunkReaderTooMuchOverhead(t *testing.T) { + // If the sender is sending 100x as many chunk header bytes as chunk data, + // we should reject the stream at some point. + chunk := []byte("1;") + for i := 0; i < 100; i++ { + chunk = append(chunk, 'a') // chunk extension + } + chunk = append(chunk, "\r\nX\r\n"...) + const bodylen = 1 << 20 + r := NewChunkedReader(&funcReader{f: func(i int) ([]byte, error) { + if i < bodylen { + return chunk, nil + } + return []byte("0\r\n"), nil + }}) + _, err := io.ReadAll(r) + if err == nil { + t.Fatalf("successfully read body with excessive overhead; want error") + } +} + +func TestChunkReaderByteAtATime(t *testing.T) { + // Sending one byte per chunk should not trip the excess-overhead detection. + const bodylen = 1 << 20 + r := NewChunkedReader(&funcReader{f: func(i int) ([]byte, error) { + if i < bodylen { + return []byte("1\r\nX\r\n"), nil + } + return []byte("0\r\n"), nil + }}) + got, err := io.ReadAll(r) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if len(got) != bodylen { + t.Errorf("read %v bytes, want %v", len(got), bodylen) + } +} + +type funcReader struct { + f func(iteration int) ([]byte, error) + i int + b []byte + err error +} + +func (r *funcReader) Read(p []byte) (n int, err error) { + if len(r.b) == 0 && r.err == nil { + r.b, r.err = r.f(r.i) + r.i++ + } + n = copy(p, r.b) + r.b = r.b[n:] + if len(r.b) > 0 { + return n, nil + } + return n, r.err +} diff --git a/src/path/filepath/path_test.go b/src/path/filepath/path_test.go index a6466183b4b964..bd1904cd4d6ef8 100644 --- a/src/path/filepath/path_test.go +++ b/src/path/filepath/path_test.go @@ -109,6 +109,8 @@ var wincleantests = []PathTest{ {`//abc`, `\\abc`}, {`///abc`, `\\\abc`}, {`//abc//`, `\\abc\\`}, + {`\\?\C:\`, `\\?\C:\`}, + {`\\?\C:\a`, `\\?\C:\a`}, // Don't allow cleaning to move an element with a colon to the start of the path. {`a/../c:`, `.\c:`}, @@ -1610,10 +1612,13 @@ var volumenametests = []VolumeNameTest{ {`//.`, `\\.`}, {`//./`, `\\.\`}, {`//./NUL`, `\\.\NUL`}, - {`//?/`, `\\?`}, + {`//?`, `\\?`}, + {`//?/`, `\\?\`}, + {`//?/NUL`, `\\?\NUL`}, + {`/??`, `\??`}, + {`/??/`, `\??\`}, + {`/??/NUL`, `\??\NUL`}, {`//./a/b`, `\\.\a`}, - {`//?/`, `\\?`}, - {`//?/`, `\\?`}, {`//./C:`, `\\.\C:`}, {`//./C:/`, `\\.\C:`}, {`//./C:/a/b/c`, `\\.\C:`}, @@ -1622,8 +1627,8 @@ var volumenametests = []VolumeNameTest{ {`//./UNC/host\`, `\\.\UNC\host\`}, {`//./UNC`, `\\.\UNC`}, {`//./UNC/`, `\\.\UNC\`}, - {`\\?\x`, `\\?`}, - {`\??\x`, `\??`}, + {`\\?\x`, `\\?\x`}, + {`\??\x`, `\??\x`}, } func TestVolumeName(t *testing.T) { diff --git a/src/path/filepath/path_windows.go b/src/path/filepath/path_windows.go index c490424f20c968..eacab0e5ced678 100644 --- a/src/path/filepath/path_windows.go +++ b/src/path/filepath/path_windows.go @@ -102,12 +102,14 @@ func volumeNameLen(path string) int { // \\.\unc\a\b\..\c into \\.\unc\a\c. return uncLen(path, len(`\\.\UNC\`)) - case pathHasPrefixFold(path, `\\.`): - // Path starts with \\., and is a Local Device path. + case pathHasPrefixFold(path, `\\.`) || + pathHasPrefixFold(path, `\\?`) || pathHasPrefixFold(path, `\??`): + // Path starts with \\.\, and is a Local Device path; or + // path starts with \\?\ or \??\ and is a Root Local Device path. // - // We currently treat the next component after the \\.\ prefix - // as part of the volume name, although there doesn't seem to be - // a principled reason to do this. + // We treat the next component after the \\.\ prefix as + // part of the volume name, which means Clean(`\\?\c:\`) + // won't remove the trailing \. (See #64028.) if len(path) == 3 { return 3 // exactly \\. } @@ -117,14 +119,6 @@ func volumeNameLen(path string) int { } return len(path) - len(rest) - 1 - case pathHasPrefixFold(path, `\\?`) || pathHasPrefixFold(path, `\??`): - // Path starts with \\?\ or \??\, and is a Root Local Device path. - // - // While Windows usually treats / and \ as equivalent, - // /??/ does not seem to be recognized as a Root Local Device path. - // We treat it as one anyway here to be safe. - return 3 - case len(path) >= 2 && isSlash(path[1]): // Path starts with \\, and is a UNC path. return uncLen(path, 2) diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go index 44479cc2be262c..b2026ad0dc72d7 100644 --- a/src/runtime/malloc.go +++ b/src/runtime/malloc.go @@ -853,6 +853,10 @@ retry: // // The heap lock must not be held over this operation, since it will briefly acquire // the heap lock. +// +// Must be called on the system stack because it acquires the heap lock. +// +//go:systemstack func (h *mheap) enableMetadataHugePages() { // Enable huge pages for page structure. h.pages.enableChunkHugePages() diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index de5ae0ae00c4f1..a12dbfe9df292c 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -1186,7 +1186,9 @@ func gcMarkTermination() { // Enable huge pages on some metadata if we cross a heap threshold. if gcController.heapGoal() > minHeapForMetadataHugePages { - mheap_.enableMetadataHugePages() + systemstack(func() { + mheap_.enableMetadataHugePages() + }) } semrelease(&worldsema) diff --git a/src/runtime/mpagealloc.go b/src/runtime/mpagealloc.go index 3e789ab85cc019..2861fa93ebf0d5 100644 --- a/src/runtime/mpagealloc.go +++ b/src/runtime/mpagealloc.go @@ -437,6 +437,10 @@ func (p *pageAlloc) grow(base, size uintptr) { // // The heap lock must not be held over this operation, since it will briefly acquire // the heap lock. +// +// Must be called on the system stack because it acquires the heap lock. +// +//go:systemstack func (p *pageAlloc) enableChunkHugePages() { // Grab the heap lock to turn on huge pages for new chunks and clone the current // heap address space ranges. diff --git a/src/runtime/os_windows.go b/src/runtime/os_windows.go index f5c2429a05d146..735a905b6161ac 100644 --- a/src/runtime/os_windows.go +++ b/src/runtime/os_windows.go @@ -127,15 +127,8 @@ var ( _AddVectoredContinueHandler, _ stdFunction - // Use RtlGenRandom to generate cryptographically random data. - // This approach has been recommended by Microsoft (see issue - // 15589 for details). - // The RtlGenRandom is not listed in advapi32.dll, instead - // RtlGenRandom function can be found by searching for SystemFunction036. - // Also some versions of Mingw cannot link to SystemFunction036 - // when building executable as Cgo. So load SystemFunction036 - // manually during runtime startup. - _RtlGenRandom stdFunction + // Use ProcessPrng to generate cryptographically random data. + _ProcessPrng stdFunction // Load ntdll.dll manually during startup, otherwise Mingw // links wrong printf function to cgo executable (see issue @@ -152,12 +145,12 @@ var ( ) var ( - advapi32dll = [...]uint16{'a', 'd', 'v', 'a', 'p', 'i', '3', '2', '.', 'd', 'l', 'l', 0} - kernel32dll = [...]uint16{'k', 'e', 'r', 'n', 'e', 'l', '3', '2', '.', 'd', 'l', 'l', 0} - ntdlldll = [...]uint16{'n', 't', 'd', 'l', 'l', '.', 'd', 'l', 'l', 0} - powrprofdll = [...]uint16{'p', 'o', 'w', 'r', 'p', 'r', 'o', 'f', '.', 'd', 'l', 'l', 0} - winmmdll = [...]uint16{'w', 'i', 'n', 'm', 'm', '.', 'd', 'l', 'l', 0} - ws2_32dll = [...]uint16{'w', 's', '2', '_', '3', '2', '.', 'd', 'l', 'l', 0} + bcryptprimitivesdll = [...]uint16{'b', 'c', 'r', 'y', 'p', 't', 'p', 'r', 'i', 'm', 'i', 't', 'i', 'v', 'e', 's', '.', 'd', 'l', 'l', 0} + kernel32dll = [...]uint16{'k', 'e', 'r', 'n', 'e', 'l', '3', '2', '.', 'd', 'l', 'l', 0} + ntdlldll = [...]uint16{'n', 't', 'd', 'l', 'l', '.', 'd', 'l', 'l', 0} + powrprofdll = [...]uint16{'p', 'o', 'w', 'r', 'p', 'r', 'o', 'f', '.', 'd', 'l', 'l', 0} + winmmdll = [...]uint16{'w', 'i', 'n', 'm', 'm', '.', 'd', 'l', 'l', 0} + ws2_32dll = [...]uint16{'w', 's', '2', '_', '3', '2', '.', 'd', 'l', 'l', 0} ) // Function to be called by windows CreateThread @@ -256,11 +249,11 @@ func loadOptionalSyscalls() { } _AddVectoredContinueHandler = windowsFindfunc(k32, []byte("AddVectoredContinueHandler\000")) - a32 := windowsLoadSystemLib(advapi32dll[:]) - if a32 == 0 { - throw("advapi32.dll not found") + bcryptPrimitives := windowsLoadSystemLib(bcryptprimitivesdll[:]) + if bcryptPrimitives == 0 { + throw("bcryptprimitives.dll not found") } - _RtlGenRandom = windowsFindfunc(a32, []byte("SystemFunction036\000")) + _ProcessPrng = windowsFindfunc(bcryptPrimitives, []byte("ProcessPrng\000")) n32 := windowsLoadSystemLib(ntdlldll[:]) if n32 == 0 { @@ -617,7 +610,7 @@ func initWine(k32 uintptr) { //go:nosplit func getRandomData(r []byte) { n := 0 - if stdcall2(_RtlGenRandom, uintptr(unsafe.Pointer(&r[0])), uintptr(len(r)))&0xff != 0 { + if stdcall2(_ProcessPrng, uintptr(unsafe.Pointer(&r[0])), uintptr(len(r)))&0xff != 0 { n = len(r) } extendRandom(r, n) diff --git a/test/fixedbugs/issue63955.go b/test/fixedbugs/issue63955.go new file mode 100644 index 00000000000000..258e874220f0ef --- /dev/null +++ b/test/fixedbugs/issue63955.go @@ -0,0 +1,22 @@ +// compile + +// Copyright 2023 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package j + +func f(try func() int, shouldInc func() bool, N func(int) int) { + var n int +loop: // we want to have 3 preds here, the function entry and both gotos + if v := try(); v == 42 || v == 1337 { // the two || are to trick findIndVar + if n < 30 { // this aims to be the matched block + if shouldInc() { + n++ + goto loop + } + n = N(n) // try to prevent some block joining + goto loop + } + } +}