Skip to content

Conversation

adityarsuryavamshi
Copy link
Contributor

Fixes Or Enhances

When a pointer type has a custom decoder and the decoded value is nil, we get a panic.

Example

func main() {
	type CustomTime *time.Time
	type TestError struct {
		CT CustomTime
	}
	var test TestError
	decoder = form.NewDecoder()
	decoder.RegisterCustomTypeFunc(func(s []string) (interface{}, error) {
		if s[0] == "" {
			return nil, nil
		}
		parsed, err := time.Parse(time.RFC3339, s[0])
		if err != nil {
			return nil, err
		}
		return CustomTime(&parsed), nil
	}, CustomTime(nil))

	values := url.Values{
		"CT": []string{""},
	}
	decoder.Decode(&test, values)
}
panic: reflect: call of reflect.Value.Set on zero Value

This PR specifically handles the case for the case when the type is a pointer.

Make sure that you've checked the boxes below before you submit PR:

  • Tests exist or have been written that cover this particular change.

@go-playground/admins

@coveralls
Copy link

Coverage Status

coverage: 99.815% (+0.001%) from 99.814%
when pulling 3618b1a on adityarsuryavamshi:fix-pointer-decoder-panic
into 8785d3c on go-playground:master.

Copy link
Contributor

@deankarn deankarn left a comment

Choose a reason for hiding this comment

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

Awesome PR TY!

@deankarn deankarn merged commit 212b1c4 into go-playground:master Mar 19, 2025
9 checks passed
@adityarsuryavamshi
Copy link
Contributor Author

@deankarn Thank you so much for the quick response. This PR was raised to fix a specific issue I had with the pointer types. There might be other scenarios (specifically with type declarations of slices and interfaces) where the same issue might occur, I have neither ran across them nor tested for them.

@acynothia
Copy link

@deankarn It's a breaking change. see case below.

type User struct {
   Name string
}

decoder.RegisterCustomTypeFunc(
  func(vals []string) (any, error) {
    return &User{Name: vals[0]}
  }, &User{},
)

@deankarn
Copy link
Contributor

@acynothia can you provide an example I can run with input 🙏

in the meantime @adityarsuryavamshi I will likely have to rollback the change.

@deankarn
Copy link
Contributor

Changes have been reverted/retracted in https://github.com/go-playground/form/releases/tag/v4.2.3 until I can get an example from @acynothia that I can test this change with to try and find a way to have both work successfully.

@acynothia
Copy link

please have a glance at this case, it working well in previous version. https://go.dev/play/p/h6N0oYCZuNP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants