go/src/os/exec/exec_test.go, branch json-isValidNumber

os/exec: avoid leaking an exec.Cmd in TestWaitInterrupt

2023-01-10T17:59:34Z

In CL 436655 I added a GODEBUG setting to this test process to verify that Wait is eventually called for every exec.Cmd before it becomes unreachable. However, the cmdHang test helpers in TestWaitInterrupt/Exit-hang and TestWaitInterrupt/SIGKILL-hang intentially leak a subprocess in order to simulate a leaky third-party program, as Go users might encounter in practical use. To avoid tripping over the leak check, we call Wait on the leaked subprocess in a background goroutine. Since we expect the process running cmdHang to exit before its subprocess does, the call to Wait should have no effect beyond suppressing the leak check. Fixes #57596. Updates #52580. Updates #50436. Change-Id: Ia4b88ea47fc6b605c27ca6d9d7669c874867a900 Reviewed-on: https://go-review.googlesource.com/c/go/+/460998 Run-TryBot: Bryan Mills Auto-Submit: Bryan Mills Reviewed-by: Ian Lance Taylor TryBot-Result: Gopher Robot

os/exec: fix TestWaitInterrupt/WaitDelay error message

2022-11-11T21:43:46Z

As the comments say. Here we expect err to be nil instead of ctx.Err() Change-Id: I4cd02d62ac0a13c9577a567de36742f13d140d36 GitHub-Last-Rev: 6bedfbc9d2511140d088dc9ee5f40015725f68db GitHub-Pull-Request: golang/go#56698 Reviewed-on: https://go-review.googlesource.com/c/go/+/449737 Reviewed-by: Bryan Mills Run-TryBot: Ian Lance Taylor Reviewed-by: Ian Lance Taylor TryBot-Result: Gopher Robot Auto-Submit: Ian Lance Taylor

os/exec: allow NUL in environment variables on Plan 9

2022-11-04T01:23:16Z

Plan 9 uses NUL as os.PathListSeparator, so it's almost always going to appear in the environment variable list. Exempt GOOS=plan9 from the check for NUL in environment variables. For #56284. Fixes #56544. Change-Id: I23df233cdf20c0a9a606fd9253e15a9b5482575a Reviewed-on: https://go-review.googlesource.com/c/go/+/447715 Reviewed-by: David du Colombier <0intro@gmail.com> Run-TryBot: Matthew Dempsky TryBot-Result: Gopher Robot Reviewed-by: Damien Neil

os/exec: allow open descriptors to be closed during TestPipeLookPathLeak

2022-11-03T14:38:29Z

In https://build.golang.org/log/d2eb315305bf3d513c490e7f85d56e9a016aacd2, we observe a failure in TestPipeLookPathLeak due to an additional descriptor (7) that was open at the start of the test being closed while the test executes. I haven't dug much into the failure, but it seems plausible to me that the descriptor may have been opened by libc for some reason, and may have been closed due to some sort of idle timeout or the completion of a background initialization routine. Since the test is looking for a leak, and closing an existing descriptor does not indicate a leak, let's not fail the test if an existing descriptor is unexpectedly closed. Updates #5071. Change-Id: I03973ddff6592c454cfcc790d6e56accd051dd52 Reviewed-on: https://go-review.googlesource.com/c/go/+/447235 Reviewed-by: Ian Lance Taylor Run-TryBot: Bryan Mills TryBot-Result: Gopher Robot

syscall, os/exec: reject environment variables containing NULs

2022-11-01T16:40:37Z

Check for and reject environment variables containing NULs. The conventions for passing environment variables to subprocesses cause most or all systems to interpret a NUL as a separator. The syscall package rejects environment variables containing a NUL on most systems, but erroniously did not do so on Windows. This causes an environment variable such as "FOO=a\x00BAR=b" to be interpreted as "FOO=a", "BAR=b". Check for and reject NULs in environment variables passed to syscall.StartProcess on Windows. Add a redundant check to os/exec as extra insurance. Fixes #56284 Fixes CVE-2022-41716 Change-Id: I2950e2b0cb14ebd26e5629be1521858f66a7d4ae Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1609434 Run-TryBot: Damien Neil Reviewed-by: Tatiana Bradley Reviewed-by: Roland Shoemaker TryBot-Result: Security TryBots Reviewed-on: https://go-review.googlesource.com/c/go/+/446916 Reviewed-by: Tatiana Bradley TryBot-Result: Gopher Robot Run-TryBot: Matthew Dempsky Reviewed-by: Heschi Kreinick

os/exec: add the Cancel and WaitDelay fields

2022-10-25T03:34:36Z

Fixes #50436. Change-Id: I9dff8caa317a04b7b2b605f810b8f12ef8ca485d Reviewed-on: https://go-review.googlesource.com/c/go/+/401835 TryBot-Result: Gopher Robot Run-TryBot: Bryan Mills Auto-Submit: Bryan Mills Reviewed-by: Ian Lance Taylor

os/exec: set traceback to "system" in TestContextCancel

2022-10-13T20:12:51Z

This will dump more goroutines if the test happens to fail. For #50138. Change-Id: Ifae30b5ba8bddcdaa9250dd90be8d8ba7d5604d2 Reviewed-on: https://go-review.googlesource.com/c/go/+/442476 Reviewed-by: Ian Lance Taylor TryBot-Result: Gopher Robot Run-TryBot: Bryan Mills Auto-Submit: Bryan Mills

os/exec: remove protection against a duplicate Close on StdinPipe

2022-10-11T14:31:32Z

As of CL 438347, multiple concurrents calls to Close should be safe. This removes some indirection and may also make some programs that use type-assertions marginally more efficient. For example, if a program calls (*exec.Cmd).StdinPipe to obtain a pipe and then sets that as the Stdout of another command, that program will now allow the second command to inherit the file descriptor directly instead of copying everything through a goroutine. This will also cause calls to Close after the first to return an error wrapping os.ErrClosed instead of nil. However, it seems unlikely that programs will depend on that error behavior: if a program is calling Write in a loop followed by Close, then if a racing Close occurs it is likely that the Write would have already reported an error. (The only programs likely to notice a change are those that call Close — without Write! — after a call to Wait.) Updates #56043. Updates #9307. Updates #6270. Change-Id: Iec734b23acefcc7e7ad0c8bc720085bc45988efb Reviewed-on: https://go-review.googlesource.com/c/go/+/439195 Reviewed-by: Ian Lance Taylor Auto-Submit: Bryan Mills Run-TryBot: Bryan Mills TryBot-Result: Gopher Robot

os/exec: parallelize more tests

2022-10-06T19:09:18Z

This cuts the wall duration for 'go test os/exec' and 'go test -race os/exec' roughly in half on my machine, which is an even more significant speedup with a high '-count'. For better or for worse, it may also increase the repro rate of #34988. Tests that use Setenv or Chdir or check for FDs opened during the test still cannot be parallelized, but they are only a few of those. Change-Id: I8d284d8bff05787853f825ef144aeb7a4126847f Reviewed-on: https://go-review.googlesource.com/c/go/+/439196 TryBot-Result: Gopher Robot Reviewed-by: Ian Lance Taylor Run-TryBot: Bryan Mills Auto-Submit: Bryan Mills

os/exec: delete TestExtraFilesFDShuffle

2022-10-06T18:18:50Z

This test has been disabled for over nine years (since CL 12869049). Although it still compiles, it seems likely to have rotted since then, and if it was going to detect a real bug it also seems like that bug would have been encountered and reported by users since then (and would presumably have its own regression tests). To eliminate overhead from mainining it (or skipping over it while maintaining other tests), let's just delete it. Fixes #5780. Change-Id: I2a85cba20cba98a1dc6fc82336ae5e22d2242e99 Reviewed-on: https://go-review.googlesource.com/c/go/+/439197 Reviewed-by: Ian Lance Taylor Auto-Submit: Bryan Mills Run-TryBot: Bryan Mills TryBot-Result: Gopher Robot