Use UnmarshalText in URI binding

[kszafran]

Description

I have a custom type that implements a few interfaces, including encoding.TextUnmarshaler and encoding/json.Unmarshaler. I want to use binding.Uri to bind path params that include values of that type, e.g. /entities/:customType. However, it seems that binding.Uri does not support encoding.TextUnmarshaler, and so I can't bind directly to my custom type, although I can do it with binding.JSON through the UnmarshalJSON method (which implements encoding/json.Unmarshaler).

How to reproduce

package main

import (
	"github.com/gin-gonic/gin/binding"
	"testing"
)

type ByteID []byte

// implements encoding.TextUnmarshaler
func (id *ByteID) UnmarshalText(data []byte) error {
	*id = []byte("called")
	return nil
}

type bytePath struct {
	ID ByteID `uri:"id"`
}

func TestURIBindingByteID(t *testing.T) {
	m := map[string][]string{
		"id": {"asdf"},
	}

	var path bytePath
	err := binding.Uri.BindUri(m, &path)
	if err != nil {
		t.Fatal(err)
	}
	if string(path.ID) != "called" {
		t.Fatal("UnmarshalText not called")
	}
}

type TextID string

// implements encoding.TextUnmarshaler
func (id *TextID) UnmarshalText(data []byte) error {
	*id = "called"
	return nil
}

type textPath struct {
	ID TextID `uri:"id"`
}

func TestURIBindingTextID(t *testing.T) {
	m := map[string][]string{
		"id": {"asdf"},
	}

	var path textPath
	err := binding.Uri.BindUri(m, &path)
	if err != nil {
		t.Fatal(err)
	}
	if string(path.ID) != "called" {
		t.Fatal("UnmarshalText not called")
	}
}

These tests return:

=== RUN   TestURIBindingByteID
    uri_test.go:28: strconv.ParseUint: parsing "asdf": invalid syntax
--- FAIL: TestURIBindingByteID (0.00s)

=== RUN   TestURIBindingTextID
    uri_test.go:58: UnmarshalText not called
--- FAIL: TestURIBindingTextID (0.00s)

Expectations

I expect to be able to use my custom type in both path params (binding.Uri) and the request body (binding.JSON). Only the latter works currently.

Actual result

If the type is string-based, it's value is set directly without calling the UnmarshalText method, which might transform the value or validate it. If the type is byte-based, binding fails with an error.

For more context: we used to have an ID type type ID []byte, where the IDs would look like: 5e26b012eee9e281c2046558. The UmarshalText and UnmarshalJSON functions would verify that the length is 24 characters and decode the hex digits into the byte array (we later transitioned to type ID string for reasons unrelated to this issue). We're successfully using the ID type in request bodies and bind them with binding.JSON. I'd expect binding.Uri to work similarly.

Environment

• go version: 1.15.2
• gin version (or commit ref): 1.6.2
• operating system: macOS

[kszafran]

@thinkerou @appleboy Let me know if you think this is a legitimate use case. I've been looking around https://github.com/gin-gonic/gin/blob/master/binding/form_mapping.go, so if you agree that binding.Uri (and by extension binding.Query, binding.Form, binding.FormPost, and binding.FormMultipart, which share a lot of the implementation) should support encoding.TextUnmarshaler I can try to contribute a pull request.

[kszafran]

@thinkerou @appleboy Any opinions on this? I believe this would be useful. I'd even say it's expected behavior, I was very surprised to find out UnmarshalText was not used by URI binding... I can contribute a PR, but I don't want to waste time if you decide against this feature.

[0xjac]

I am also having issues with binding custom types (specifically binding a string to an enum which underneath is a uint).

It would be really useful to have a way to bind those customs types. However I would argue not to support encoding.TextUnmarshaler because time.Time implements it but without any configuration on the time format. This may create issues with the parsing of time.Time values in different formats. There might be similar issues with other types too.

I would suggest providing something similar to echo's BindUnmarshaler.

This would still allow to call UnmarshalText in the binding function, but prevent unexpected behaviors with types satisfying the encoding.TextUnmarshaler interface:

type Custom struct{}

// UnmarshalText satisfies encoding.TextUnmarshaler
func (c *Custom) UnmarshalText(text []byte) error {
   // custom unmarshal logic
}

// UnmarshalParam satisfies gin.BindUnmarshaler
func (c *Custom) UnmarshalParam(param string) error {
   return c.UnmarshalText([]byte(param))
}

[kszafran]

I think we could support both. You'd use BindUnmarshaler if it's supported by the type and TextUnmarshaler otherwise. This way you'll get unmarshaling for free if your type implements the standard TextUnmarshaler interface and you can customize the behavior with BindUnmarshaler if needed.

I might contribute a PR at some point, but for now I have just one type that needs this and I hacked my way around it, so I'm not in a rush to implement this.

[0xjac]

That sounds good to me if BindUnmarshaler unmarshaller is called first to give the option of a custom binding. Good idea!

I've also hacked my way around in my case using strings instead of iota but it's quite ugly. If you can make a PR it would be great, providing maintainers are OK with this!

[asv]

any status?

[kszafran]

any status?

There's this PR: https://github.com/gin-gonic/gin/pull/3045, but it's up to the maintainers to either approve it and merge or not.

[ssoroka]

if you're waiting on this I'd probably fork the project and merge PR #3045 yourself. I've been using it for 6 months now without issues.

[eloyekunle]

This is still an issue. When will it be fixed in gin?

[mohit-singh-pepper]

I am also using a fork for this as last resort. However the issue with fork is we cannot use any library directly dependent on this repo.

[victorperezc]

Hi, this is still an issue. I can't unmarshal custom types coming from query parameters and have to resort to the fork. I can help move forward the PR https://github.com/gin-gonic/gin/pull/3045