Skip to content

binding:fix empty value error #2169

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
appleboy merged 5 commits into from
Dec 3, 2025
+53 −5
Merged

binding:fix empty value error #2169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8456664
fix empty value error
guonaihong Dec 6, 2019
6341a25
modify code
guonaihong Dec 11, 2019
2891214
add comment
guonaihong Jan 16, 2020
af5561c
test(binding): use 'any' alias and require.NoError in form mapping tests
guonaihong Nov 30, 2025
e0ee47d
Merge branch 'master' into fix-binding-empty-value
appleboy Dec 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions binding/form_mapping.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,11 @@ func setByForm(value reflect.Value, field reflect.StructField, form map[string][
}

func setWithProperType(val string, value reflect.Value, field reflect.StructField) error {
// If it is a string type, no spaces are removed, and the user data is not modified here
if value.Kind() != reflect.String {
Copy link
Copy Markdown
Contributor

@vkd vkd Jan 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is not explained by the PR description.
Please update PR description with explanation what cases have you fixed and why.

Copy link
Copy Markdown
Contributor Author

@guonaihong guonaihong Jan 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is a string type, no spaces are removed, and the user data is not modified here.

Copy link
Copy Markdown
Contributor

@vkd vkd Jan 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please document your changes in the PR's description. Just add information about what is changed and why.

val = strings.TrimSpace(val)
Copy link
Copy Markdown
Member

@thinkerou thinkerou Feb 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If val is not string type, can call func TrimSpace(s string) string?

Copy link
Copy Markdown
Contributor Author

@guonaihong guonaihong Feb 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type of val is string, as explained below.
If the client command is the following code

curl -H 'intkey: v1" -H "strkey: v2" url
We define the following structure to parse the http header

type needHeader struct {
    IntKey int `header:"intkey"
    StrKey string `header:"strkey"
}

Such a data structure.
Before setting into the structure, regardless of the int or string required by the structure, it is of type string before being parsed.

}

switch value.Kind() {
case reflect.Int:
return setIntField(val, 0, value)
Expand Down Expand Up @@ -404,6 +409,11 @@ func setTimeField(val string, structField reflect.StructField, value reflect.Val
timeFormat = time.RFC3339
}

if val == "" {
value.Set(reflect.ValueOf(time.Time{}))
return nil
}

switch tf := strings.ToLower(timeFormat); tf {
case "unix", "unixmilli", "unixmicro", "unixnano":
tv, err := strconv.ParseInt(val, 10, 64)
Expand All @@ -427,11 +437,6 @@ func setTimeField(val string, structField reflect.StructField, value reflect.Val
return nil
}

if val == "" {
value.Set(reflect.ValueOf(time.Time{}))
return nil
}

l := time.Local
if isUTC, _ := strconv.ParseBool(structField.Tag.Get("time_utc")); isUTC {
l = time.UTC
Expand Down Expand Up @@ -475,6 +480,10 @@ func setSlice(vals []string, value reflect.Value, field reflect.StructField) err
}

func setTimeDuration(val string, value reflect.Value) error {
if val == "" {
val = "0"
}

d, err := time.ParseDuration(val)
if err != nil {
return err
Expand Down
39 changes: 39 additions & 0 deletions binding/form_mapping_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,35 @@ func TestMappingTime(t *testing.T) {
require.Error(t, err)
}

type bindTestData struct {
need any
got any
in map[string][]string
}

func TestMappingTimeUnixNano(t *testing.T) {
type needFixUnixNanoEmpty struct {
CreateTime time.Time `form:"createTime" time_format:"unixNano"`
}

// ok
tests := []bindTestData{
{need: &needFixUnixNanoEmpty{}, got: &needFixUnixNanoEmpty{}, in: formSource{"createTime": []string{" "}}},
Copy link
Copy Markdown
Contributor

@vkd vkd Jan 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The non-empty string does not look to me as a default empty value. For me it looks like a wrong value.

Copy link
Copy Markdown
Contributor Author

@guonaihong guonaihong Jan 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The strconv.ParseInt function in Go will report an error when it encounters data beginning with a space. The function of removing spaces is added so that the function does not report an error.

package main

import (
	"fmt"
	"strconv"
)

func main() {
	fmt.Println(strconv.ParseInt("  5", 10, 0))
	fmt.Println(strconv.ParseInt("5", 10, 0))
}

/*
0 strconv.ParseInt: parsing "  5": invalid syntax
5 <nil>

*/

Copy link
Copy Markdown
Contributor

@vkd vkd Jan 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and from my point of view the binding shouldn't do this.

{need: &needFixUnixNanoEmpty{}, got: &needFixUnixNanoEmpty{}, in: formSource{"createTime": []string{}}},
}

for _, v := range tests {
err := mapForm(v.got, v.in)
require.NoError(t, err)
assert.Equal(t, v.need, v.got)
}
}

func TestMappingTimeDuration(t *testing.T) {
type needFixDurationEmpty struct {
Duration time.Duration `form:"duration"`
}

var s struct {
D time.Duration
}
Expand All @@ -236,6 +264,17 @@ func TestMappingTimeDuration(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, 5*time.Second, s.D)

// ok
tests := []bindTestData{
{need: &needFixDurationEmpty{}, got: &needFixDurationEmpty{}, in: formSource{"duration": []string{" "}}},
{need: &needFixDurationEmpty{}, got: &needFixDurationEmpty{}, in: formSource{"duration": []string{}}},
}

for _, v := range tests {
err := mapForm(v.got, v.in)
require.NoError(t, err)
assert.Equal(t, v.need, v.got)
}
// error
err = mappingByPtr(&s, formSource{"D": {"wrong"}}, "form")
require.Error(t, err)
Expand Down