Fix/missing content length header on empty post req

[Libero-Dev]

This change aims to resolve issue #996 by changing requests that contain nil body to http.NoBody when they have also specified to "SetContentLength" as true.

Validity as to whether to merge this or not depends on multiple factors.

1. The root issue as to why Content-Length was ommitted when requests were created with a nil body ultimately originates from within the go http standard library. The library explicitly states that Content-Length will be omitted when body is nil unless Transfer-Encoding header is explicitly set to "identity". Where exactly does this happen in the http library? I have no idea, despite trying to find it. However I have found similar issues that reference to this: net/http: Content-Length: 0 is not set for PATCH requests with empty body golang/go#40978.

Setting the bodyBuf value to http.NoBody signals to the http library when creating the request that the body is no longer nil and a Content-Length header component can be created for the request.

3. Considering this is a feature, albeit a poorly documented one in the standard http library it does seem to be behaving as intended?

4. In resty setting SetContentLength(true) and r.Body = nil, could be considered as the user intentionally overriding this requirement to force the Content-Length header on whilst passing in a nil body.

Ultimately, either the intention of the http libraries functionality should be obeyed or we should consider this unique resty request setting combination as an intentional force override to achieve a valid Content-Length header with a nil body.

Additional Notes:

I have had to update middleware_test.go:Test_parseRequestBody() for the reason being that now that nil body + SetContentLength(true) is supported in this PR change this now results in the expectedBodyBuf value to change since in this scenario the bodyBuf changes to http.NoBody.

[jeevatkm]

@Libero-Dev I have read the issue #996 and this PR description along with the code changes. Thanks for the details.

Can you please have a look at these lines and apply the fix accordingly instead of adding new repeated code lines to fix?

resty/middleware.go

Lines 208 to 215 in 1e19d6b

	// by default resty won't set content length, you can if you want to :)
	if c.setContentLength || r.setContentLength {
	if r.bodyBuf == nil {
	r.Header.Set(hdrContentLengthKey, "0")
	} else {
	r.Header.Set(hdrContentLengthKey, strconv.Itoa(r.bodyBuf.Len()))
	}
	}

[Libero-Dev]

@jeevatkm there is reliance on the handleRequestBody() function being triggered before proceeding with the rest of the logic. Placing the conditional check where you have recommended will break tests and continue to exhibit missing content-length header for recipients of the request. I have compromised with a cleaner alternative that attempts to keep both requirements.

[jeevatkm]

@Libero-Dev Thanks for the updates.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.70%. Comparing base (47780b1) to head (cc8d952).
Report is 1 commits behind head on v2.

Additional details and impacted files

@@           Coverage Diff           @@
##               v2    #1003   +/-   ##
=======================================
  Coverage   95.69%   95.70%           
=======================================
  Files          14       14           
  Lines        2207     2211    +4     
=======================================
+ Hits         2112     2116    +4     
  Misses         71       71           
  Partials       24       24           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.