gorilla/schema

[question] Error converting into []byte or []uint8

nikbanerjee-unity opened this issue · 2 comments

Describe the problem you're having

Schema fails to convert into []uint8 despite it being listed as supported. Anyone aware of why this might be the case? Or if Im misunderstanding the scenario.

I have a server that accepts a multipart/form-data form data, of which one of the fields is a []byte, but when trying to decode into it I get the error:

schema: error converting value for index 0 of "<value>"

Versions

Go version: go version go1.18.1 darwin/arm64

Gorilla version: github.com/gorilla/schema v1.2.0

"Show me the code!"

Minimal test to show this failing:

	req, err := http.NewRequest(http.MethodGet, "todo", nil)
	req.ParseForm()
	req.Form.Set("test1", "anyvalue")

	type demo struct {
		Test1 []byte // I've tried with pointer to byte slice and uint8 slice
	}

	var d demo
	err = formDecoder.Decode(&d, req.Form)
	fmt.Printf("%v", err)

Go playground example: https://go.dev/play/p/e_62qC5zkik

Hi @nikbanerjee-unity,

This can be solved by registering a custom converter:

	formDecoder.RegisterConverter([]byte{}, func(s string) reflect.Value {
		return reflect.ValueOf([]byte(s))
	})

What happens behind the curtains is that the conversion fails here: https://github.com/gorilla/schema/blob/master/converter.go#L127 because anyvalue is treated as a whole value, and obviously this cannot be converted to an uint8 which is an alias for byte. To solve this, the decoding need to loop over the string character by character and convert each one to byte and then build a slice. Not sure if it's worth having this built-in, since it can be solved by a 2 liners function call.

Hi @zak905! Thanks for coming back to me with a solution, I will give see about giving it a try for our setup.

I would argue though, that given this is claimed to be supported and is a pretty fundamental type, that it should be built-in. Don't really see a reason worth keeping it as a separately registered converter. I also recall (and may be wrong here) finding very little in the ways of docs about registering custom conversions making this discovery harder. That, or at least it wasn't clear that would be the way to handle it.

So depending on how complex the addition of this as a built-in is; I might recommend it be added - but don't want to push the decision either way if there are other factors to consider.