joke2k / django-environ

Django-environ allows you to utilize 12factor inspired environment variables to configure your Django application.
https://django-environ.rtfd.org
MIT License
3.01k stars 318 forks source link

Hash sign inside double quotes is interpreted as the start of a comment #499

Closed alvra closed 1 year ago

alvra commented 1 year ago

Ran into this problem when updating to 0.11.2 for a project with an .env file like this:

SECRET_KEY="abc#def"

The SECRET_KEY used to be abc#def but now is read as "abc.

Apparently the # is interpreted as the start of a comment now, except in singly-quoted strings. Looking at the tests for this new feature only the single-quoted case is tested, but the documentation contains examples with double quotes. It seems they are not treated the same in this case; is this intended behaviour?

sergeyklay commented 1 year ago

We are currently investigating the issue related to the handling of hash signs (#) within double-quoted strings in .env files. This behavior seems to have been introduced in PR #475.

We understand that this might be causing unexpected behavior in your projects, especially if you've recently updated to version 0.11.2. Our team is actively looking into whether this is a regression or an intended feature that was not clearly communicated.

The reproducible sample is bellow:

.env file contents:

SECRET_KEY_499_1=abc#def
SECRET_KEY_499_2='abc#def'
SECRET_KEY_499_3='abc\#def'
SECRET_KEY_499_4="abc\#def"
SECRET_KEY_499_5=abc\#def

Test 2750dd54e0e4b1b591a31409cfb1dc16a82b958d (#475)

SECRET_KEY_499_1=abc
SECRET_KEY_499_2=abc#def
SECRET_KEY_499_3=abc\#def
SECRET_KEY_499_4="abc\
SECRET_KEY_499_5=abc\

Test 8874288ac11bc70fa491439d52a4c307f9388aea (v0.10.0)

SECRET_KEY_499_1=abc#def
SECRET_KEY_499_2=abc#def
SECRET_KEY_499_3=abc\#def
SECRET_KEY_499_4=abc#def
SECRET_KEY_499_5=abc\#def

We appreciate your patience as we work to identify the root cause and plan the next steps. If you have any additional information or test cases that could help us in this investigation, please feel free to share.

sergeyklay commented 1 year ago

Hi @alvra,

I'm currently working on resolving the issue you've raised regarding the handling of hash signs within double-quoted strings in .env files. I've submitted a pull request #500 that aims to provide a compromise by making inline comment handling optional and disabled by default.

I'd be interested to hear your thoughts on this solution. Any feedback would be greatly appreciated.

alvra commented 1 year ago

Disabling comments by default makes sense to me from a backwards compatibility perspective because previously (1.10.0) it seems simply everything was accepted and interpreted in one way or another. It certainly solves the issue on my side.

To move forwards with the new comments feature, which is an improvement IMO, I would suggest to accept both single and double quotes in all situations equally (or choose either, but this is probably too breaking) and document/test this as such. If this is fixed, to be strictly backwards compatible it should be opt-in, but in the long term I would prefer it to be the default.

However, I would consider placing new features such as this behind a flag that is more strict about parsing values and disallows certain cases that are clearly confusing and make it harder to extend the syntax in the future. For example, all these were originally accepted while these are arguably either invalid or the result is unexpected.

KEY1="abc"def                        → "abc"def
KEY2="abc" # looks like a comment    → "abc" # looks like a comment
KEY3="abc"def"                       → abc"def

Thank you for picking this up so quickly.

sergeyklay commented 1 year ago

This was fixed by https://github.com/joke2k/django-environ/pull/500. We'll try to release new version as soon as possible. Thank you for the report, and for helping us make django-environ better.

pulse-mind commented 6 months ago

Hi,

When do you plan to release a version ?