Panic overriding a []string field via env var inside a subsection map

Open Beginner friendly
#9 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
88/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
go
Domain
backend

Research direction

Start in gcfgenv.go, in setGcfgWithEnvMap's sec.Kind() == reflect.Map branch, and inspect the existing-key and new-subsection loops. Add regression coverage for []string fields in both an existing file-defined subsection and an env-only subsection, then run the package tests; done means both cases set the value without a reflection panic.

Written by the indexing model from the issue text.

Description

Goal

Fix setGcfgWithEnvMap's subsection-map handling in gcfgenv.go so an
environment variable targeting a []string field inside a map[string]*struct
gcfg subsection sets/appends the value instead of panicking.

Two call sites call reflect.Value.Elem() on a value whose Kind() is already
Slice (only Ptr/Interface support Elem(), so this always panics once
that branch is reached):

  • the "existing map key" branch:
    f.Set(reflect.AppendSlice(f.Elem(), newRef)) should be
    f.Set(reflect.AppendSlice(f, newRef)) -- f (subsec.Field(j)) is already
    the slice itself.
  • the "new subsection created from the env var" branch:
    f.Elem().Field(j).Set(reflect.AppendSlice(f.Elem().Field(j).Elem(), newRef))
    needs the same fix, dropping the inner .Elem().

Done when a []string field inside a subsection map (for example
Flag []string gcfg:"Flag"`` in a map[string]*Section) can be set via its
derived env var, for both an existing file-defined subsection and a
brand-new env-only one, with a regression test for each case.

Why

A []string field on a plain (non-map) struct section already works: that
code path never calls the extra .Elem(). Subsection maps ([Section "name"],
keyed, multiple instances) hit the bug because their equivalent code paths
already dereference a pointer once (iter.Value().Elem() / sec.MapIndex(key)
gives a *T), then incorrectly call .Elem() again on the inner slice field.
Any repeatable option added to a subsection map hits a guaranteed startup
panic the moment it's set via an env var, with no workaround except avoiding
env vars for that field entirely.

References

  • gcfgenv.go, setGcfgWithEnvMap, the sec.Kind() == reflect.Map branch
    (both the existing-key loop and the new-subsection loop)

  • Minimal repro (panics on main, v0.2.1):

    package main
    
    import (
    	"os"
    	"strings"
    
    	"github.com/rstudio/gcfgenv"
    )
    
    type Section struct {
    	Flag []string `gcfg:"Flag"`
    }
    
    type Config struct {
    	Section map[string]*Section
    }
    
    func main() {
    	os.Setenv("APP_SECTION_HELLO_FLAG", "beta")
    	cfg := &Config{}
    	_ = gcfgenv.ReadWithEnvInto(strings.NewReader(""), "APP", cfg)
    	// panics: reflect: call of reflect.Value.Elem on slice Value
    }
    
Dominant language
Go
Stars
1
Forks
0
Avg merge
1h 42m
Merged PRs (30d)
1

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from rstudio/gcfgenv

All issues in rstudio/gcfgenv

Similar issues

More Go issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.