Skip to content

Conversation

@KianYang-Lee
Copy link

Add a new StringToMailAddressHookFunc to convert strings to mail.Address.

Closes #108

Copy link
Member

@sagikazarmark sagikazarmark left a comment

Choose a reason for hiding this comment

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

Could you please rebase your PR and add some tests?

Thanks!

@KianYang-Lee KianYang-Lee force-pushed the feat/add-string-to-mail-address-hook-func branch from 12f7f91 to d279398 Compare July 16, 2025 02:52
@KianYang-Lee
Copy link
Author

Could you please rebase your PR and add some tests?

Thanks!

Done

Copy link
Member

@sagikazarmark sagikazarmark left a comment

Choose a reason for hiding this comment

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

Apologies for a second round, but I had another finding. PTAL, thanks!

// Convert it by parsing
addr, err := mail.ParseAddress(data.(string))
if err != nil {
return mail.Address{}, fmt.Errorf("failed parsing mail address %v: %w", data, err)
Copy link
Member

Choose a reason for hiding this comment

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

As of 2.4.0 decode hooks must not return errors that have the decoded value in the error message.

Please take a look at other decode hooks (particularly net and time hooks) for examples.

Also, I briefly looked into the mail package and it looks like in some cases their error messages also contain the decoded value. That needs to be dealt with as well.

Make sure to add a test here as well verifying that the decode hook doesn't leak values: https://github.com/go-viper/mapstructure/blob/main/decode_hooks_test.go#L2055

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.

Feature Request: hook to decode email address

2 participants