pberkel / caddy-storage-redis

Apache License 2.0
40 stars 5 forks source link

Panic in Caddyfile unmarshal #13

Open francislavoie opened 3 weeks ago

francislavoie commented 3 weeks ago
{"level":"info","ts":1726153008.6148074,"msg":"using config from file","file":"/etc/caddy/Caddyfile"}
panic: runtime error: index out of range [0] with length 0

goroutine 1 [running]:
github.com/pberkel/caddy-storage-redis.(*RedisStorage).UnmarshalCaddyfile(0xc00039d8c0, 0xc0004eeba0)
    github.com/pberkel/caddy-storage-redis@v1.3.0/caddyfile.go:85 +0x11b9
github.com/caddyserver/caddy/v2/caddyconfig/caddyfile.UnmarshalModule(0xc0004eeb70, {0xc000054828, 0x13})
    github.com/caddyserver/caddy/v2@v2.8.4/caddyconfig/caddyfile/adapter.go:137 +0x1b4
github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile.parseOptStorage(0xc0004eeb70, {0xc0004ee990?, 0xc000263660?})
    github.com/caddyserver/caddy/v2@v2.8.4/caddyconfig/httpcaddyfile/options.go:179 +0xb7
github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile.ServerType.evaluateGlobalOptionsBlock({}, {0xc000970480, 0x2, 0x2}, 0xc0004ee990)
    github.com/caddyserver/caddy/v2@v2.8.4/caddyconfig/httpcaddyfile/httptype.go:372 +0x1c2
github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile.ServerType.Setup({}, {0xc000960e80, 0x2, 0xc0004eea20?}, 0xc0004ee990)
    github.com/caddyserver/caddy/v2@v2.8.4/caddyconfig/httpcaddyfile/httptype.go:83 +0x2a7
github.com/caddyserver/caddy/v2/caddyconfig/caddyfile.Adapter.Adapt({{0x2047e20?, 0x2e46020?}}, {0xc000411680, 0x20c, 0x20d}, 0xc00039d7a0?)
    github.com/caddyserver/caddy/v2@v2.8.4/caddyconfig/caddyfile/adapter.go:50 +0x12d
github.com/caddyserver/caddy/v2/cmd.loadConfigWithLogger(0x188ee40?, {0x7fff83d28e4a, 0x14}, {0x7fff83d28e69, 0x9})
    github.com/caddyserver/caddy/v2@v2.8.4/cmd/main.go:214 +0x75b
github.com/caddyserver/caddy/v2/cmd.LoadConfig({0x7fff83d28e4a, 0x14}, {0x7fff83d28e69, 0x9})
    github.com/caddyserver/caddy/v2@v2.8.4/cmd/main.go:107 +0x45
github.com/caddyserver/caddy/v2/cmd.cmdRun({0x0?})
    github.com/caddyserver/caddy/v2@v2.8.4/cmd/commandfuncs.go:214 +0x58c
github.com/caddyserver/caddy/v2/cmd.init.1.func2.WrapCommandFuncForCobra.1(0xc000958308, {0x1b15ae1?, 0x4?, 0x1b15ab1?})
    github.com/caddyserver/caddy/v2@v2.8.4/cmd/cobra.go:137 +0x2f
github.com/spf13/cobra.(*Command).execute(0xc000958308, {0xc000240dc0, 0x4, 0x4})
    github.com/spf13/cobra@v1.8.0/command.go:983 +0xaca
github.com/spf13/cobra.(*Command).ExecuteC(0x2dc15c0)
    github.com/spf13/cobra@v1.8.0/command.go:1115 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
    github.com/spf13/cobra@v1.8.0/command.go:1039
github.com/caddyserver/caddy/v2/cmd.Main()
    github.com/caddyserver/caddy/v2@v2.8.4/cmd/main.go:75 +0x1d8
main.main()
    caddy/main.go:12 +0xf

My config is just:

{
    storage redis {
        address redis:6379
        db 1
    }
}

The problem is here:

https://github.com/pberkel/caddy-storage-redis/blob/739937ec7e63f88437782b2f312cd25e31e9ba90/caddyfile.go#L85

Why are we doing configVal[0]? d.Val() wouldn't be returning a slice, it returns a single token value. Pretty sure it should just be configVal with no [0].

francislavoie commented 3 weeks ago

Nope I'm dumb, the config value for db just wasn't set (missing env var, my fault) so the slice was empty. Whoops.

Either way, it shouldn't panic on bad config. It should be adjusted to properly report an error saying there's no input.

pberkel commented 2 weeks ago

Thanks for the report @francislavoie, the caddyfile unmarshalling code attempts to handle properties with single or multiple values (in a nested block):

            var configVal []string

            if d.NextArg() {
                // configuration item with single parameter
                configVal = append(configVal, d.Val())
            } else {
                // configuration item with nested parameter list
                for nesting := d.Nesting(); d.NextBlock(nesting); {
                    configVal = append(configVal, d.Val())
                }
            }

but subsequent code does not check if any values were supplied. I will look at updating the above block to check for conditions where parameter values are not supplied and handle the error gracefully.