windows/svc: use separate (and more descriptive) service names in tests
Notably, the DisplayName field was set to the same thing in both
sys.TestExample and mrg.TestMyService, which may explain the collision
reported in golang/go#59298.
Moreover, the adjective ”my” conveys no information whatsoever — we
shouldn't use it in tests or examples.
Also skip the tests that install services if GO_BUILDER_NAME is not
set, to reduce the likelihood of 'go test all' in a user's working
directory being mistaken for a malicious or compromised program.
Fixes golang/go#59298.
Change-Id: Ib00bf7400bfaa34e1a1d49125c43b97019b53c82
Reviewed-on: https://go-review.googlesource.com/c/sys/+/481015
Reviewed-by: Carlos Amedee <carlos@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
diff --git a/windows/svc/example/main.go b/windows/svc/example/main.go
index 2ea7330..62f9470 100644
--- a/windows/svc/example/main.go
+++ b/windows/svc/example/main.go
@@ -15,6 +15,7 @@
package main
import (
+ "flag"
"fmt"
"log"
"os"
@@ -33,8 +34,11 @@
os.Exit(2)
}
+var svcName = "exampleservice"
+
func main() {
- const svcName = "myservice"
+ flag.StringVar(&svcName, "name", svcName, "name of the service")
+ flag.Parse()
inService, err := svc.IsWindowsService()
if err != nil {
@@ -55,7 +59,7 @@
runService(svcName, true)
return
case "install":
- err = installService(svcName, "my service")
+ err = installService(svcName, "example service")
case "remove":
err = removeService(svcName)
case "start":
diff --git a/windows/svc/example/service.go b/windows/svc/example/service.go
index c989abf..08d54b5 100644
--- a/windows/svc/example/service.go
+++ b/windows/svc/example/service.go
@@ -19,9 +19,9 @@
var elog debug.Log
-type myservice struct{}
+type exampleService struct{}
-func (m *myservice) Execute(args []string, r <-chan svc.ChangeRequest, changes chan<- svc.Status) (ssec bool, errno uint32) {
+func (m *exampleService) Execute(args []string, r <-chan svc.ChangeRequest, changes chan<- svc.Status) (ssec bool, errno uint32) {
const cmdsAccepted = svc.AcceptStop | svc.AcceptShutdown | svc.AcceptPauseAndContinue
changes <- svc.Status{State: svc.StartPending}
fasttick := time.Tick(500 * time.Millisecond)
@@ -79,7 +79,7 @@
if isDebug {
run = debug.Run
}
- err = run(name, &myservice{})
+ err = run(name, &exampleService{})
if err != nil {
elog.Error(1, fmt.Sprintf("%s service failed: %v", name, err))
return
diff --git a/windows/svc/mgr/mgr_test.go b/windows/svc/mgr/mgr_test.go
index 6f849f3..10d2310 100644
--- a/windows/svc/mgr/mgr_test.go
+++ b/windows/svc/mgr/mgr_test.go
@@ -227,25 +227,26 @@
}
func TestMyService(t *testing.T) {
+ if os.Getenv("GO_BUILDER_NAME") == "" {
+ // Don't install services on arbitrary users' machines.
+ t.Skip("skipping test that modifies system services: GO_BUILDER_NAME not set")
+ }
if testing.Short() {
- t.Skip("skipping test in short mode - it modifies system services")
+ t.Skip("skipping test in short mode that modifies system services")
}
- const name = "mymgrservice"
+ const name = "mgrtestservice"
m, err := mgr.Connect()
if err != nil {
- if errno, ok := err.(syscall.Errno); ok && errno == syscall.ERROR_ACCESS_DENIED {
- t.Skip("Skipping test: we don't have rights to manage services.")
- }
t.Fatalf("SCM connection failed: %s", err)
}
defer m.Disconnect()
c := mgr.Config{
StartType: mgr.StartDisabled,
- DisplayName: "my service",
- Description: "my service is just a test",
+ DisplayName: "x-sys mgr test service",
+ Description: "x-sys mgr test service is just a test",
Dependencies: []string{"LanmanServer", "W32Time"},
}
@@ -288,14 +289,14 @@
if err != nil {
t.Fatalf("ListServices failed: %v", err)
}
- var myserviceIsInstalled bool
+ var serviceIsInstalled bool
for _, sn := range svcnames {
if sn == name {
- myserviceIsInstalled = true
+ serviceIsInstalled = true
break
}
}
- if !myserviceIsInstalled {
+ if !serviceIsInstalled {
t.Errorf("ListServices failed to find %q service", name)
}
diff --git a/windows/svc/svc_test.go b/windows/svc/svc_test.go
index f7833ad..5d794e1 100644
--- a/windows/svc/svc_test.go
+++ b/windows/svc/svc_test.go
@@ -77,11 +77,15 @@
}
func TestExample(t *testing.T) {
- if testing.Short() && os.Getenv("GO_BUILDER_NAME") != "" {
- t.Skip("skipping test in short mode - it modifies system services")
+ if os.Getenv("GO_BUILDER_NAME") == "" {
+ // Don't install services on arbitrary users' machines.
+ t.Skip("skipping test that modifies system services: GO_BUILDER_NAME not set")
+ }
+ if testing.Short() {
+ t.Skip("skipping test in short mode that modifies system services")
}
- const name = "myservice"
+ const name = "svctestservice"
m, err := mgr.Connect()
if err != nil {
@@ -103,7 +107,7 @@
stopAndDeleteIfInstalled(t, m, name)
- s, err := m.CreateService(name, exepath, mgr.Config{DisplayName: "my service"}, "is", "auto-started")
+ s, err := m.CreateService(name, exepath, mgr.Config{DisplayName: "x-sys svc test service"}, "-name", name)
if err != nil {
t.Fatalf("CreateService(%s) failed: %v", name, err)
}
@@ -141,7 +145,7 @@
t.Fatalf("Delete failed: %s", err)
}
- out, err := exec.Command("wevtutil.exe", "qe", "Application", "/q:*[System[Provider[@Name='myservice']]]", "/rd:true", "/c:10").CombinedOutput()
+ out, err := exec.Command("wevtutil.exe", "qe", "Application", "/q:*[System[Provider[@Name='"+name+"']]]", "/rd:true", "/c:10").CombinedOutput()
if err != nil {
t.Fatalf("wevtutil failed: %v\n%v", err, string(out))
}
@@ -149,7 +153,7 @@
// Test context passing (see servicemain in sys_386.s and sys_amd64.s).
want += "-123456"
if !strings.Contains(string(out), want) {
- t.Errorf("%q string does not contain %q", string(out), want)
+ t.Errorf("%q string does not contain %q", out, want)
}
}
