From 285e80cd79d6daee7d5f0ac9555d885f368346f0 Mon Sep 17 00:00:00 2001 From: Doug MacEachern Date: Thu, 10 Feb 2022 16:38:06 -0800 Subject: [PATCH] fix: avoid debug trace if http.Request.Body is nil The govmomi soap and rest clients ensure Request.Body is not nil, but custom clients may not. --- vim25/debug/debug_test.go | 74 +++++++++++++++++++++++++++++++++++++++ vim25/soap/debug.go | 4 ++- 2 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 vim25/debug/debug_test.go diff --git a/vim25/debug/debug_test.go b/vim25/debug/debug_test.go new file mode 100644 index 000000000..2c36fead0 --- /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) + } + }) +} diff --git a/vim25/soap/debug.go b/vim25/soap/debug.go index ab23fd63f..bc5b90203 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)