nickstenning / honcho

Honcho: a python clone of Foreman. For managing Procfile-based applications.
http://pypi.python.org/pypi/honcho
MIT License
1.59k stars 145 forks source link

Allow more types of punctuation in .env values #143

Closed dsem closed 9 years ago

dsem commented 9 years ago

Prior to using shlex to parse the .env file in 0.6.0, the regex parser seemed to allow any printable character in an environment variable value. The implementation of the shlex parser only allows a-zA-Z0-9_/.+-(): characters (with special rules for quotes). This patch brings the behavior closer to the original regex parser, allowing values to include any non-whitespace printable character except = and \ (with special rules for quotes).

In my case, allowing @ in environment variables was crucial for me to store my postgreSQL URI in an environment variable (e.g. DATABASE_URL=postgres://user:pass@domain.com:port/database)

nickstenning commented 9 years ago

Thanks for this. Sorry about what is clearly a regression on our part. Thank you in particular for updating the tests.

That said, I think we can do better than this. I don't think we should be updating wordchars at all, or asserting that there are three tokens, but instead we should be doing something like:

tokens = list(shlex.shlex(line, posix=True))
name, op = tokens[:2]
value = ''.join(tokens[2:])
...

In particular, you might want to add a unicode item to the tests, as this will fail with the current implementation but not with the above.

COMMUNIST_SNOWMAN=☭☃
dsem commented 9 years ago

I was thinking the same thing and had codded those exact lines, but decided to go with my first version because I was worried there were reasons the original patch was written the way it was.

Any character should now be allowed as environment variable values. However, values that include whitespaces or backslashes need to be quoted, as shlex doesn't store the whitespace as a token and it parses \\ as \ (unless inside quotes). There's also still the same special handling of quotes that shlex does.

nickstenning commented 9 years ago

LGTM. Thank you!