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

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

Closed
alvra opened this issue Sep 8, 2023 · 5 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@alvra
Copy link

alvra commented Sep 8, 2023

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
Copy link
Collaborator

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 2750dd5 (#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 8874288 (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
Copy link
Collaborator

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
Copy link
Author

alvra commented Sep 9, 2023

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 added a commit that referenced this issue Sep 10, 2023
Disabled inline comments handling (that was introduced in #475)
by default due to potential side effects. While the feature itself is
useful, the project's philosophy dictates that it should not be enabled
by default for all users.

This also fix the issue described in the #499.
sergeyklay added a commit that referenced this issue Sep 22, 2023
Disabled inline comments handling (that was introduced in #475)
by default due to potential side effects. While the feature itself is
useful, the project's philosophy dictates that it should not be enabled
by default for all users.

This also fix the issue described in the #499.
@sergeyklay
Copy link
Collaborator

This was fixed by #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
Copy link

Hi,

When do you plan to release a version ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants