Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

character '-' dash no longer valid environment variable #310

Closed
asyarif93 opened this issue Sep 29, 2022 · 5 comments · Fixed by #336
Closed

character '-' dash no longer valid environment variable #310

asyarif93 opened this issue Sep 29, 2022 · 5 comments · Fixed by #336
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@asyarif93
Copy link

got this error unexpected character "-" in variable name near "AIRFLOW_CONN_COMPOSER-WORKER-SAMPLE..."

afaik, specs for env is IEEE Std 1003.1-2001 - any printable character but = and NUL is valid

i'm not sure if the problems come from this lib or docker/compose

please close this issue if it's not relevant in this repository.

thank you

@shusso
Copy link

shusso commented Nov 29, 2022

Not sure if this is related or not but digging up from
https://stackoverflow.com/questions/2821043/allowed-characters-in-linux-environment-variable-names
https://pubs.opengroup.org/onlinepubs/000095399/basedefs/xbd_chap08.html
we can see

Environment variable names used by the utilities in the Shell and Utilities volume of IEEE Std 1003.1-2001 consist solely of uppercase letters, digits, and the '_' (underscore) from the characters defined so I assume it's then up to each shell implementation. E.g. in Bash

↳  bash --version
GNU bash, version 5.2.2(1)-release (aarch64-apple-darwin22.1.0)
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
↳  export MY-VAR="foobar"
-bash: export: `MY-VAR=foobar': not a valid identifier
↳  export MY_VAR="foobar"

@asyarif93
Copy link
Author

then i read the article wrong, i think we can close this. thanks for the effort

@milas
Copy link
Member

milas commented Dec 6, 2022

You're both right!

Here's an expanded snippet from IEEE Std 1003.1, 2004 Edition :: 8.1 (emphasis mine):

These strings have the form name=value; names shall not contain the character '='. For values to be portable across systems conforming to IEEE Std 1003.1-2001, the value shall be composed of characters from the portable character set (except NUL and as indicated below). There is no meaning associated with the order of strings in the environment. If more than one string in a process' environment has the same name, the consequences are undefined.

Environment variable names used by the utilities in the Shell and Utilities volume of IEEE Std 1003.1-2001 consist solely of uppercase letters, digits, and the '_' (underscore) from the characters defined in Portable Character Set and do not begin with a digit. Other characters may be permitted by an implementation; applications shall tolerate the presence of such names

Currently, Compose MOSTLY behaves like a Shell - uppercase letters, digits, & _. We do also allow . as a valid character due to its prevalence in many Java frameworks for environment variable overrides.

I don't think we want to open up to all non-NUL but I could see allowing - if a similar argument to . can be made, i.e. there's widespread use of - from popular frameworks/languages.


TL;DR Stick to alphanumeric + _ for compatibility purposes with Shell & other apps

@ndeloof
Copy link
Collaborator

ndeloof commented Dec 9, 2022

Agree, let's reopen this one, until there's some technical reason that prevent us to accept - within environment variables

@ndeloof ndeloof reopened this Dec 9, 2022
@xzyfer
Copy link

xzyfer commented Jan 3, 2023

TL;DR Stick to alphanumeric + _ for compatibility purposes with Shell & other apps

Looks like this rule of thumb has already been broken in #271. The justification being that . was support in v1 which appears to be true for - as the current "work-around" found online is to use v1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants