docker / compose

Define and run multi-container applications with Docker
https://docs.docker.com/compose/
Apache License 2.0
34.02k stars 5.23k forks source link

[BUG] dashes not supported in environment variable names in .env files #12302

Open vvuk opened 3 hours ago

vvuk commented 3 hours ago

Description

This was seemingly fixed in #9570 last year, and looking at the code it doesn't look like the code regressed, but I can't seem to include environment variables with a - in them via env_file in a compose file.

Steps To Reproduce

foo.env contains (hello ASP.NET config env vars):

services__storage-service__http__0=http://storage:2227

docker-compose.yaml contains:

services:
  storage:
    image: ..
    env_file: foo.env

trying to do up errors out with:

failed to read ...foo.env: line 1: unexpected character "-" in variable name "services__storage-service__http__0=http://storage:2227"

(I'm also a little confused why it prints the entire line as "variable name" but I think that's just a printing issue?)

I can define variables with a - in them fine directly in environment: entries in the compose file.

Compose Version

Docker Compose version v2.29.7, Ubuntu 22.04.

Docker Environment

lient: Docker Engine - Community
 Version:    27.3.1
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc.)
    Version:  v0.17.1
    Path:     /usr/libexec/docker/cli-plugins/docker-buildx
  compose: Docker Compose (Docker Inc.)
    Version:  v2.29.7
    Path:     /usr/libexec/docker/cli-plugins/docker-compose

Anything else?

No response

vvuk commented 2 hours ago

Actually, wait. #9570 claims it was fixed in https://github.com/compose-spec/compose-go/pull/336 but the source change (which is the same that remains today):

https://github.com/compose-spec/compose-go/blob/58f8cade1fa80ca0d59947c1ebd965c4c215ab71/dotenv/parser.go#L122-L132:

        case '_', '.', '-', '[', ']':
        default:
            // variable name should match [A-Za-z0-9_.-]
            if unicode.IsLetter(rune) || unicode.IsNumber(rune) {
                continue
            }

            return "", "", inherited, fmt.Errorf(
                `line %d: unexpected character %q in variable name %q`,
                p.line, string(rune), strings.Split(src, "\n")[0])
        }

the patch added '-' to the case statement, but it would have fallen through default anyway -- and the following lines are still checking IsNumber or IsLetter exactly like before. So I'm not sure if this actually did anything to allow -?