Skip to content

Fix filecontent mapper panicking on a non-[]byte target - #625

Merged
alecthomas merged 1 commit into
alecthomas:masterfrom
chuenchen309:fix/filecontent-nonbyte-panic
Jul 19, 2026
Merged

Fix filecontent mapper panicking on a non-[]byte target#625
alecthomas merged 1 commit into
alecthomas:masterfrom
chuenchen309:fix/filecontent-nonbyte-panic

Conversation

@chuenchen309

Copy link
Copy Markdown
Contributor

Problem

type:"filecontent" on a field that isn't []byte panics with an opaque reflect error at parse time, instead of the mapper's own "must be applied to []byte" validation error:

var cli struct { F string `type:"filecontent"` }
kong.Parse(&cli, ...)   // panics: reflect: call of reflect.Value.Elem on string Value

var cli2 struct { F []string `type:"filecontent"` }
kong.Parse(&cli2, ...)  // panics: reflect.Value.SetBytes of non-byte slice

Cause

The guard in fileContentMapper (mapper.go) has two defects on one line:

if target.Kind() != reflect.Slice && target.Elem().Kind() != reflect.Uint8 {
  • && should be ||: a slice with a non-byte element makes the first operand false, short-circuits, skips the error, and falls through to SetBytes on a non-byte slice.
  • target.Elem() should be target.Type().Elem(): reflect.Value.Elem() panics on a scalar Kind like string.

The sibling mappers (path, existingfile, existingdir) validate their target kind correctly and return a clean error; filecontent was the only one that panicked.

Fix

if target.Kind() != reflect.Slice || target.Type().Elem().Kind() != reflect.Uint8 {

Now both misuse cases return the intended --f: "filecontent" must be applied to []byte not <type> error, and the []byte case is unchanged. Added TestFileContentMapperRejectsNonByteSlice (string + []string). go test ./... passes; golangci-lint run ./... is clean; gofmt clean.

This only hardens the guard — it does not add the string support requested in #482; that remains a separate feature decision.


Disclosure: found, written and verified by an AI coding agent (Claude Code) running on this account — it wrote the repro, ran it, and produced this description from running the code, not reading it. I review every change and am accountable for it. Happy to adjust anything.

The "filecontent" mapper's own guard is meant to return
`"filecontent" must be applied to []byte not <type>` for a wrong target,
but the condition used `&&` (so a slice with a non-byte element skipped
the error and fell through to SetBytes) and `target.Elem()` instead of
`target.Type().Elem()` (which panics on a scalar Kind). A plain `string`
field therefore panicked with an opaque reflect error instead of the
intended message. Use `||` and `target.Type().Elem()`, matching the
sibling path/existingfile/existingdir guards.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrew Chen <48723787+chuenchen309@users.noreply.github.com>
@alecthomas
alecthomas merged commit b401ebf into alecthomas:master Jul 19, 2026
5 checks passed
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.

2 participants