Add Conditions to Ensure Bind Succeeds with Transfer-Encoding: chunked

[178inaba]

In Go, when the length of the Body is unknown, ContentLength is set to -1. As demonstrated in #2716, this applies to cases where Transfer-Encoding: chunked is used.

	// ContentLength records the length of the associated content.
	// The value -1 indicates that the length is unknown.
	// Values >= 0 indicate that the given number of bytes may
	// be read from Body.
	//
	// For client requests, a value of 0 with a non-nil Body is
	// also treated as unknown.
	ContentLength [int64](https://pkg.go.dev/builtin#int64)

https://pkg.go.dev/net/http#Request

Previously, only 0 was excluded during Bind, but starting from #2710, -1 was also excluded, making it impossible to Bind requests with Transfer-Encoding: chunked. I have fixed this issue.

Fixes #2716.

[aldas]

I think going back to == 0 check is better.

and lets add your test for chunked and modify that failing test to test error case and add another empty body case

		{
			name:             "nok, JSON POST bind to http.NoBody",
			givenURL:         "/api/real_node/endpoint?node=xxx",
			givenMethod:      http.MethodPost,
			givenContentType: MIMEApplicationJSON,
			givenContent:     http.NoBody,
			expect:           &Node{ID: 0, Node: ""},
			expectError:      "code=400, message=EOF, internal=EOF",
		},
		{
			name:             "ok, JSON POST with empty body",
			givenURL:         "/api/real_node/endpoint?node=xxx",
			givenMethod:      http.MethodPost,
			givenContentType: MIMEApplicationJSON,
			givenContent:     strings.NewReader(``),
			expect:           &Node{ID: 0, Node: ""},
		},
		{
			name:             "ok, JSON POST bind to struct with: path + query + chunked body",
			givenURL:         "/api/real_node/endpoint?node=xxx",
			givenMethod:      http.MethodPost,
			givenContentType: MIMEApplicationJSON,
			givenContent:     httputil.NewChunkedReader(strings.NewReader("18\r\n" + `{"id": 1, "node": "zzz"}` + "\r\n0\r\n")),
			whenChunkedBody:  true,
			expect:           &Node{ID: 1, Node: "zzz"},
		},

I think allowing chunked post with -1 length to work is better that allowing nil body with unknown size to pass without error

[178inaba]

@aldas
Thank you for the review!
I reverted the ContentLength check to only 0 and added tests.
Should we add a condition for ContentLength == -1 && chunked?

[aldas]

Should we add a condition for ContentLength == -1 && chunked?

No at the moment. I do not think we gain much from it. #2710 as a little bit theoretical problem to fix as in real world making chunked not to bind is worse than allowing POST with len -1 and nil body to error out (which I do not even how to reproduce with CURL etc).

edit: you can reproduce http.NoBody with curl -v -X GET http://localhost:8082 -H 'Content-Type: application/json' -H "Transfer-Encoding: chunked". Due "chunked" condition req.ContentLength == -1 is true, but this seems more error on client side than something that Echo has to overcome.

I am going to merge this and do patch release v4.13.1.

Thanks.

[aldas]

release v4.13.1 is available