Merge e6f5267f407b969659135fb8ab964b1e3d7b6a4c into 5f4f9643258dc2a65e684b63f12c8d543c936c67

binding/all.go

@@ -28,26 +28,22 @@ func (allBinding) BindMany(req *http.Request, uriParams map[string][]string, obj
 		return err
 	}
 
-	// from binding.Query
-	values := req.URL.Query()
-	if err := mapForm(obj, values); err != nil {
-		return err
-	}
-
 	// from context.Bind (for body/post-form/anything else)
 	contentType := req.Header.Get("Content-Type")
-	// trim contentType parameters, e.g. "application/json; charset=utf-8" -> "application/json"
-	contentTypeLastIdx := strings.IndexAny(contentType, " ;")
+	contentTypeLastIdx := strings.IndexAny(contentType, " ;") // trim "application/json; charset=utf-8" -> "application/json"
 	if contentTypeLastIdx != -1 {
 		contentType = contentType[:contentTypeLastIdx]
 	}
+	b := Default(req.Method, contentType)
 
-	// if no Content-Type, assume request has no body. This avoids binding query params again in binding.Default
-	if contentType == "" {
-		return validate(obj)
+	// Since req.ParseForm() reads body and query, check whether the selected binding includes query parsing before parsing query values explicitly.
+	if !bindingIncludesQueryParsing(b) {
+		// from binding.Query
+		if err := mapForm(obj, req.URL.Query()); err != nil {
+			return err
+		}
 	}
 
-	b := Default(req.Method, contentType)
-	// final validation done by whatever binding is selected here
+	// final validation done by whatever binding was selected by Default
 	return b.Bind(req, obj)
 }

binding/binding.go

@@ -131,6 +131,16 @@ func Default(method, contentType string) Binding {
 	}
 }
 
+// bindingIncludesQueryParsing reports whether a binding parses URL query values as part of Bind.
+func bindingIncludesQueryParsing(binding Binding) bool {
+	switch binding {
+	case Form, FormPost, FormMultipart:
+		return true
+	default:
+		return false
+	}
+}
+
 func validate(obj any) error {
 	if Validator == nil {
 		return nil

binding/binding_nomsgpack.go

@@ -123,6 +123,16 @@ func Default(method, contentType string) Binding {
 	}
 }
 
+// bindingIncludesQueryParsing reports whether a binding parses URL query values as part of Bind.
+func bindingIncludesQueryParsing(binding Binding) bool {
+	switch binding {
+	case Form, FormPost, FormMultipart:
+		return true
+	default:
+		return false
+	}
+}
+
 func validate(obj any) error {
 	if Validator == nil {
 		return nil

binding/binding_test.go

@@ -154,6 +154,10 @@ type FooStructForMapPtrType struct {
 	PtrBar *map[string]any `form:"ptr_bar"`
 }
 
+type FooStructForAllFormPrecedence struct {
+	Count int `form:"count" binding:"required"`
+}
+
 func TestBindingDefault(t *testing.T) {
 	assert.Equal(t, Form, Default(http.MethodGet, ""))
 	assert.Equal(t, Form, Default(http.MethodGet, MIMEJSON))
@@ -1498,6 +1502,13 @@ func TestBindingAllQuery(t *testing.T) {
 	req = requestWithBody(http.MethodGet, "/?bool_foo=fasl", "")
 	err = b.BindMany(req, nil, &obj2)
 	require.Error(t, err)
+
+	// fail case 2
+	obj3 := FooStructForBoolType{}
+	req = requestWithBody(http.MethodPost, "/?bool_foo=fasl", "{}")
+	req.Header.Set("Content-Type", MIMEJSON)
+	err = b.BindMany(req, nil, &obj3)
+	require.Error(t, err)
 }
 
 func TestBindingAllBody(t *testing.T) {
@@ -1518,6 +1529,19 @@ func TestBindingAllBody(t *testing.T) {
 	require.Error(t, err)
 }
 
+// TestBindingAllFormBodyOverridesInvalidQuery. Since req.ParseForm() reads body and query, invalid queries are replaced
+// by valid values in body
+func TestBindingAllFormBodyOverridesInvalidQuery(t *testing.T) {
+	b := All
+
+	obj := FooStructForAllFormPrecedence{}
+	req := requestWithBody(http.MethodPost, "/?count=invalid", "count=7")
+	req.Header.Set("Content-Type", MIMEPOSTForm)
+	err := b.BindMany(req, nil, &obj)
+	require.NoError(t, err)
+	assert.Equal(t, 7, obj.Count)
+}
+
 func TestBindingAllHeaderUriQueryBody(t *testing.T) {
 	b := All
 

context.go

@@ -809,9 +809,9 @@ func (c *Context) BindUri(obj any) error {
 //
 // Note:
 //   - Caller must tag struct fields appropriately for the desired binding (eg `header:"xxx"` vs `uri:"xxx"`)
-//   - Caller must ensure no duplication between field names (else use separate binding engines instead)
+//   - Caller must ensure no duplication between field names. Duplication may result in undefined behavior. (use separate binding engines instead)
 //   - Caller must provide Content-Type header to select the correct body binding (eg "application/json" for JSON binding).
-//     A request without Content-Type will result in no body binding
+//     A request without Content-Type will fallback to gin's default behavior for body binding (see ShouldBind)
 //   - Binding validation tags are verified after all request parts have been bound
 func (c *Context) BindAll(obj any) error {
 	err := c.ShouldBindAll(obj)
@@ -947,9 +947,9 @@ func (c *Context) ShouldBindUri(obj any) error {
 //
 // Note:
 //   - Caller must tag struct fields appropriately for the desired binding (eg `header:"xxx"` vs `uri:"xxx"`)
-//   - Caller must ensure no duplication between field names (else use separate binding engines instead)
+//   - Caller must ensure no duplication between field names. Duplicate fields may result in undefined behavior. (use separate binding engines instead)
 //   - Caller must provide Content-Type header to select the correct body binding (eg "application/json" for JSON binding).
-//     A request without Content-Type will result in no body binding
+//     A request without Content-Type will fallback to gin's default behavior for body binding (see ShouldBind)
 //   - Binding validation tags are verified after all request parts have been bound
 func (c *Context) ShouldBindAll(obj any) error {
 	uriParams := make(map[string][]string, len(c.Params))