CASMHMS-6294: Another checkpoint

cmd/smd/smd-api.go

@@ -293,6 +293,7 @@ func compGetLockFltrToCompLockV2Filter(cglf CompGetLockFltr) (clf sm.CompLockV2F
 // be a resource leak.  Additionally, if the body is not read at all, the
 // network connection will be closed and will not be reused even though the
 // http server will properly drain and close the request body.
+// TODO: This should be moved into hms-base
 
 func DrainAndCloseRequestBody(req *http.Request) {
 	if req != nil && req.Body != nil {

internal/hbtdapi/hbtdapi_test.go

@@ -1,6 +1,6 @@
 // MIT License
 //
-// (C) Copyright [2020-2021] Hewlett Packard Enterprise Development LP
+// (C) Copyright [2020-2021,2024] Hewlett Packard Enterprise Development LP
 //
 // Permission is hereby granted, free of charge, to any person obtaining a
 // copy of this software and associated documentation files (the "Software"),
@@ -28,13 +28,15 @@ package hbtdapi
 
 import (
 	"bytes"
-	"github.com/hashicorp/go-retryablehttp"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"net/url"
 	"os"
 	"testing"
-	"github.com/Cray-HPE/hms-base/v2"
+
+	base "github.com/Cray-HPE/hms-base/v2"
+	"github.com/hashicorp/go-retryablehttp"
 )
 
 var client *retryablehttp.Client
@@ -200,8 +202,24 @@ const testPayloadHBTDAPI_goodNoHB = `
 	}]
 }`
 
+// While it is generally not a requirement to close request bodies in server
+// handlers, it is good practice.  If a body is only partially read, there can
+// be a resource leak.  Additionally, if the body is not read at all, the
+// network connection will be closed and will not be reused even though the
+// http server will properly drain and close the request body.
+// TODO: This should be moved into hms-base
+
+func DrainAndCloseRequestBody(req *http.Request) {
+	if req != nil && req.Body != nil {
+			_, _ = io.Copy(io.Discard, req.Body) // ok even if already drained
+			req.Body.Close()                     // ok even if already closed
+	}
+}
+
 func NewRTFuncSLSAPI() RTFunc {
 	return func(req *http.Request) *http.Response {
+		defer DrainAndCloseRequestBody(req)
+
 		bad := true
 		if (len(req.Header) > 0) {
 			vals,ok := req.Header[base.USERAGENT]

internal/slsapi/slsapi_test.go

@@ -1,6 +1,6 @@
 // MIT License
 //
-// (C) Copyright [2019-2021] Hewlett Packard Enterprise Development LP
+// (C) Copyright [2019-2021,2024] Hewlett Packard Enterprise Development LP
 //
 // Permission is hereby granted, free of charge, to any person obtaining a
 // copy of this software and associated documentation files (the "Software"),
@@ -28,13 +28,15 @@ package slsapi
 
 import (
 	"bytes"
-	"github.com/hashicorp/go-retryablehttp"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"net/url"
 	"os"
 	"testing"
-	"github.com/Cray-HPE/hms-base/v2"
+
+	base "github.com/Cray-HPE/hms-base/v2"
+	"github.com/hashicorp/go-retryablehttp"
 )
 
 var client *retryablehttp.Client
@@ -281,8 +283,24 @@ const testPayloadSLSAPI_comp_noData = `
 	"Xname": "x0c0s3b0n0"
 }`
 
+// While it is generally not a requirement to close request bodies in server
+// handlers, it is good practice.  If a body is only partially read, there can
+// be a resource leak.  Additionally, if the body is not read at all, the
+// network connection will be closed and will not be reused even though the
+// http server will properly drain and close the request body.
+// TODO: This should be moved into hms-base
+
+func DrainAndCloseRequestBody(req *http.Request) {
+	if req != nil && req.Body != nil {
+			_, _ = io.Copy(io.Discard, req.Body) // ok even if already drained
+			req.Body.Close()                     // ok even if already closed
+	}
+}
+
 func NewRTFuncSLSAPI() RTFunc {
 	return func(req *http.Request) *http.Response {
+		defer DrainAndCloseRequestBody(req)
+
 		bad := true
 		if (len(req.Header) > 0) {
 			vals,ok := req.Header[base.USERAGENT]

pkg/redfish/rfcomponents_test.go

@@ -28,12 +28,13 @@ package rf
 import (
 	"bytes"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"testing"
 
-	"github.com/Cray-HPE/hms-xname/xnametypes"
 	"github.com/Cray-HPE/hms-certs/pkg/hms_certs"
+	"github.com/Cray-HPE/hms-xname/xnametypes"
 )
 
 // RoundTrip method override
@@ -1094,8 +1095,24 @@ func VerifyGetRootInfo(e *RedfishEP, v RedfishEPVerifyInfo) error {
 //                         Proliant - Mock Client
 //////////////////////////////////////////////////////////////////////////////
 
+// While it is generally not a requirement to close request bodies in server
+// handlers, it is good practice.  If a body is only partially read, there can
+// be a resource leak.  Additionally, if the body is not read at all, the
+// network connection will be closed and will not be reused even though the
+// http server will properly drain and close the request body.
+// TODO: This should be moved into hms-base
+
+func DrainAndCloseRequestBody(req *http.Request) {
+	if req != nil && req.Body != nil {
+			_, _ = io.Copy(io.Discard, req.Body) // ok even if already drained
+			req.Body.Close()                     // ok even if already closed
+	}
+}
+
 func NewRTFuncPRLT1() RTFunc {
 	return func(req *http.Request) *http.Response {
+    defer DrainAndCloseRequestBody(req)
+
 		// Test request parameters
 		switch req.URL.String() {
 		case "https://" + testFQDN + testPathPRLT_redfish_v1:

pkg/service-reservations/reservations_test.go

@@ -1,17 +1,17 @@
 // MIT License
-// 
-// (C) Copyright [2022-2023] Hewlett Packard Enterprise Development LP
-// 
+//
+// (C) Copyright [2022-2024] Hewlett Packard Enterprise Development LP
+//
 // Permission is hereby granted, free of charge, to any person obtaining a
 // copy of this software and associated documentation files (the "Software"),
 // to deal in the Software without restriction, including without limitation
 // the rights to use, copy, modify, merge, publish, distribute, sublicense,
 // and/or sell copies of the Software, and to permit persons to whom the
 // Software is furnished to do so, subject to the following conditions:
-// 
+//
 // The above copyright notice and this permission notice shall be included
 // in all copies or substantial portions of the Software.
-// 
+//
 // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 // IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 // FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
@@ -25,6 +25,7 @@ package service_reservations
 import (
 	"encoding/json"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -52,6 +53,19 @@ var logger = logrus.New()
 
 var resMap map[string]*ReservationCreateSuccessResponse
 
+// While it is generally not a requirement to close request bodies in server
+// handlers, it is good practice.  If a body is only partially read, there can
+// be a resource leak.  Additionally, if the body is not read at all, the
+// network connection will be closed and will not be reused even though the
+// http server will properly drain and close the request body.
+// TODO: This should be moved into hms-base
+
+func DrainAndCloseRequestBody(req *http.Request) {
+	if req != nil && req.Body != nil {
+			_, _ = io.Copy(io.Discard, req.Body) // ok even if already drained
+			req.Body.Close()                     // ok even if already closed
+	}
+}
 
 /////////////////////////////////////////////////////////////////////////////
 // Funcs to simulate HSM APIs for reservations.
@@ -63,6 +77,7 @@ func smReservationHandler(w http.ResponseWriter, r *http.Request) {
 	var rsp ReservationCreateResponse
 
 	body,_ := ioutil.ReadAll(r.Body)
+	defer DrainAndCloseRequestBody(r)
 	err := json.Unmarshal(body,&jdata)
 	if (err != nil) {
 		logger.Errorf("%s: Error unmarshalling req data: %v",fname,err)
@@ -105,6 +120,7 @@ func smReservationRenewHandler(w http.ResponseWriter, r *http.Request) {
 	fname := "smReservationRenewHandler()"
 
 	body, _ := ioutil.ReadAll(r.Body)
+	defer DrainAndCloseRequestBody(r)
 	err := json.Unmarshal(body, &inData)
 	if err != nil {
 		logger.Errorf("%s: Error unmarshalling req data: %v", fname, err)
@@ -158,6 +174,7 @@ func smReservationReleaseHandler(w http.ResponseWriter, r *http.Request) {
 	fname := "smReservationReleaseHandler()"
 
 	body,_ := ioutil.ReadAll(r.Body)
+	defer DrainAndCloseRequestBody(r)
 	err := json.Unmarshal(body,&relList)
 	if (err != nil) {
 		logger.Errorf("%s: Error unmarshalling req data: %v",fname,err)