Repository: vmware/govmomi
Pull request #2750 (2022-02-11 02:10:10)

fix: avoid debug trace if http.Request.Body is nil
## Description

The govmomi soap and rest clients ensure Request.Body is not nil, but custom clients may not.

## Type of change

Please mark options that are relevant:

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to
  not work as expected)
- [ ] This change requires a documentation update
- [ ] Build related change

## Checklist:

- [x] My code follows the CONTRIBUTION [guidelines](./../CONTRIBUTING.md) of
  this project
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged


Patch:
diff --git a/vim25/soap/debug.go b/vim25/soap/debug.go
index ab23fd63..bc5b9020 100644
--- a/vim25/soap/debug.go
+++ b/vim25/soap/debug.go
@@ -87,7 +87,9 @@ func (d *debugRoundTrip) debugRequest(req *http.Request) string {
 	ext := d.ext(req.Header)
 	// Capture body
 	wc = d.newFile("req." + ext)
-	req.Body = Trace(req.Body, wc, ext)
+	if req.Body != nil {
+		req.Body = Trace(req.Body, wc, ext)
+	}
 
 	// Delay closing until marked done
 	d.cs = append(d.cs, wc)


Test patch:
diff --git a/vim25/debug/debug_test.go b/vim25/debug/debug_test.go
new file mode 100644
index 00000000..2c36fead
--- /dev/null
+++ b/vim25/debug/debug_test.go
@@ -0,0 +1,74 @@
+/*
+   Copyright (c) 2022 VMware, Inc. All Rights Reserved.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package debug_test
+
+import (
+	"context"
+	"net/http"
+	"sync"
+	"testing"
+
+	"github.com/vmware/govmomi/find"
+	"github.com/vmware/govmomi/simulator"
+	"github.com/vmware/govmomi/vapi/rest"
+	"github.com/vmware/govmomi/vim25"
+	"github.com/vmware/govmomi/vim25/debug"
+
+	_ "github.com/vmware/govmomi/vapi/simulator"
+)
+
+func TestSetProvider(t *testing.T) {
+	p := debug.FileProvider{
+		Path: t.TempDir(),
+	}
+	debug.SetProvider(&p)
+
+	simulator.Test(func(ctx context.Context, c *vim25.Client) {
+		var wg sync.WaitGroup
+		rc := rest.NewClient(c)
+
+		// hit the debug package with some concurrency (see PR #2469)
+		for i := 0; i < 5; i++ {
+			wg.Add(1)
+			go func() {
+				defer wg.Done()
+				finder := find.NewFinder(c)
+
+				_, err := finder.VirtualMachineList(ctx, "*")
+				if err != nil {
+					t.Error(err)
+				}
+			}()
+		}
+
+		wg.Wait()
+
+		// send an http request with a nil Body to ensure debug trace doesn't panic in this case
+		u := rc.URL().String() + "/com/vmware/cis/session"
+
+		req, err := http.NewRequest(http.MethodPost, u, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		req.SetBasicAuth("user", "pass")
+		var id string
+		if err = rc.Do(ctx, req, &id); err != nil {
+			t.Fatal(err)
+		}
+	})
+}