go-playground-form/PR.md

Fix panic when a custom type function returns nil for a pointer field

Problem

A DecodeCustomTypeFunc registered against a pointer type is a natural way to say "there is no value, leave this pointer nil" by returning nil, nil for an empty input (the time.Time-style workaround for "unset" is not available for a custom pointer type). Doing that panics:

type CustomTime *time.Time

type TestError struct {
    CT CustomTime
}

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))

var test TestError
decoder.Decode(&test, url.Values{"CT": {""}})

panic: reflect: call of reflect.Value.Set on zero Value

Root cause

decoder.go:196-204, inside the custom-type-function branch of setFieldByType:

val, err := cf(arr[idx:])
if err != nil {
    d.setError(namespace, err)
    return
}

v.Set(reflect.ValueOf(val))

When cf returns a nil interface{}, reflect.ValueOf(val) is the invalid zero reflect.Value, and Set panics on it.

This exact bug was fixed once in #68, but that fix went through the generic case reflect.Ptr: recursion path instead of guarding the nil case, which discarded the custom function's own return value for a pointer type whenever it returned something other than nil. That regressed the case where the custom function itself returns a fully populated pointer (&User{...}), so #68 was reverted in #69, and the original nil-pointer panic has been back on master since.

Fix

Guard against a nil val before calling Set, right where the value is produced, instead of special-casing reflect.Ptr after the fact. A nil return leaves the field at its existing zero value, everything else is set exactly as before:

if val == nil {
    return
}

v.Set(reflect.ValueOf(val))

Verification

TestDecoder_CustomTypeFuncPointerNilValue in decoder_test.go covers both sides: a custom function for *User that returns nil, nil for an empty value (must not panic, field stays nil), and the same function returning a populated *User (must still decode correctly - this is the case #68 broke).

Reverting only the setFieldByType guard (test left in place) panics the test binary itself, not just a failed assertion:

--- FAIL: TestDecoder_CustomTypeFuncPointerNilValue
panic: reflect: call of reflect.Value.Set on zero Value [recovered]
	panic: reflect: call of reflect.Value.Set on zero Value
...
	.../decoder.go:202 +0x35f
	.../decoder.go:177 +0x1f0
	.../form_decoder.go:174 +0x225

go build ./..., go vet ./... and gofmt -l decoder.go decoder_test.go are clean. go test ./... -count=1 and go test ./... -count=1 -race: 48 tests, all passing, including the new one.

Flags for the account owner

• .github/CONTRIBUTING.md only asks for tests covering the change; no CLA/DCO step.
• No behaviour change for any case that already worked: a nil-returning custom function on a pointer field previously always panicked, so there is no existing caller relying on that panic.