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

Environment variables with dashes stopped working #1036

Closed
guimeira opened this issue Nov 2, 2023 · 2 comments · Fixed by #1041
Closed

Environment variables with dashes stopped working #1036

guimeira opened this issue Nov 2, 2023 · 2 comments · Fixed by #1041

Comments

@guimeira
Copy link

guimeira commented Nov 2, 2023

Hello!
I just came across a problem and I'm not even sure if it's a bug or the intended behavior.
In the latest version of smallrye-config, environment variables with dashes in them are not picked up by the library.
Here's a small reproducer:

public class Main {
  public static void main(String[] args) {
    Config config = ConfigProvider.getConfig();
    String message = config.getValue("my-message", String.class);
    System.out.println(message);
  }
}

If I set an environment variable my-message=Hello world (bash won't allow this, but IntelliJ and Docker do) and run the code above using version 3.3.4, it prints Hello world, but if I run it using version 3.4.1 it throws an exception.
I'm not sure if this was ever supposed to work since the documentation explains the proper way of defining config environment variables, but in previous versions of the library this worked, so I thought I should open this issue.
I think this is caused by this bit of the equals method of EnvConfigSource.EnvProperty. I wonder if it should instead be similar to the one right above it, something like:

...
} else if (o == '-') {
  if (n != '.' && n != '-' && n != '_') {
    return false
  }
} ...
@radcortez
Copy link
Member

Yes, this was never supposed to work. Even if other environments allow for such names, I think it is best to limit the names to POSIX-compatible names: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html

Is it a big issue for you to adjust?

@guimeira
Copy link
Author

guimeira commented Nov 6, 2023

Sorry, I forgot to respond last week!
Sticking to posix-compliant variables is definitely a good idea and I'll be doing that from now on. But thank you for fixing this! Really appreciate it!

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 a pull request may close this issue.

2 participants