Fix variable expansion order without override

bbc2 · bbc2 · commit 26ff5b74c676 · 2020-12-05T14:46:06.000+01:00
This fixes an issue when a variable is resolved differently in two
bindings.

For instance, take the following env file:

```
PORT=8000
URL=http://localhost:${PORT}
```

With `PORT` set to `1234` in the environment, the environment resulting
from `dotenv_load(override=False)` would be:

```
PORT=1234
URL=http://localhost:8000
```

This was inconsistent and is fixed by this commit.  The environment
would now be:

```
PORT=1234
URL=http://localhost:1234
```

with override, and

```
PORT=8000
URL=http://localhost:8000
```

without override.

The behavior of `load_dotenv` is unchanged and always assumes
`override=True`.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,7 +7,7 @@ project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
 
-_There are no unreleased changes at this time._
+- Fix resolution order in variable expansion with `override=False` (#? by [@bbc2]).
 
 ## [0.15.0] - 2020-10-28
 
diff --git a/README.md b/README.md
@@ -41,13 +41,22 @@ export SECRET_KEY=YOURSECRETKEYGOESHERE
 
 Python-dotenv can interpolate variables using POSIX variable expansion.
 
-The value of a variable is the first of the values defined in the following list:
+With `load_dotenv(override=True)` or `dotenv_values()`, the value of a variable is the
+first of the values defined in the following list:
 
 - Value of that variable in the `.env` file.
 - Value of that variable in the environment.
 - Default value, if provided.
 - Empty string.
 
+With `load_dotenv(override=False)`, the value of a variable is the first of the values
+defined in the following list:
+
+- Value of that variable in the environment.
+- Value of that variable in the `.env` file.
+- Default value, if provided.
+- Empty string.
+
 Ensure that variables are surrounded with `{}` like `${HOME}` as bare
 variables such as `$HOME` are not expanded.
 
diff --git a/src/dotenv/main.py b/src/dotenv/main.py
@@ -43,13 +43,14 @@ def with_warn_for_invalid_lines(mappings):
 
 class DotEnv():
 
-    def __init__(self, dotenv_path, verbose=False, encoding=None, interpolate=True):
-        # type: (Union[Text, _PathLike, _StringIO], bool, Union[None, Text], bool) -> None
+    def __init__(self, dotenv_path, verbose=False, encoding=None, interpolate=True, override=True):
+        # type: (Union[Text, _PathLike, _StringIO], bool, Union[None, Text], bool, bool) -> None
         self.dotenv_path = dotenv_path  # type: Union[Text,_PathLike, _StringIO]
         self._dict = None  # type: Optional[Dict[Text, Optional[Text]]]
         self.verbose = verbose  # type: bool
         self.encoding = encoding  # type: Union[None, Text]
         self.interpolate = interpolate  # type: bool
+        self.override = override  # type: bool
 
     @contextmanager
     def _get_stream(self):
@@ -73,7 +74,7 @@ def dict(self):
         raw_values = self.parse()
 
         if self.interpolate:
-            self._dict = OrderedDict(resolve_variables(raw_values))
+            self._dict = OrderedDict(resolve_variables(raw_values, override=self.override))
         else:
             self._dict = OrderedDict(raw_values)
 
@@ -86,13 +87,13 @@ def parse(self):
                 if mapping.key is not None:
                     yield mapping.key, mapping.value
 
-    def set_as_environment_variables(self, override=False):
-        # type: (bool) -> bool
+    def set_as_environment_variables(self):
+        # type: () -> bool
         """
         Load the current dotenv as system environemt variable.
         """
         for k, v in self.dict().items():
-            if k in os.environ and not override:
+            if k in os.environ and not self.override:
                 continue
             if v is not None:
                 os.environ[to_env(k)] = to_env(v)
@@ -205,8 +206,8 @@ def unset_key(dotenv_path, key_to_unset, quote_mode="always"):
     return removed, key_to_unset
 
 
-def resolve_variables(values):
-    # type: (Iterable[Tuple[Text, Optional[Text]]]) -> Mapping[Text, Optional[Text]]
+def resolve_variables(values, override):
+    # type: (Iterable[Tuple[Text, Optional[Text]]], bool) -> Mapping[Text, Optional[Text]]
 
     new_values = {}  # type: Dict[Text, Optional[Text]]
 
@@ -216,8 +217,12 @@ def resolve_variables(values):
         else:
             atoms = parse_variables(value)
             env = {}  # type: Dict[Text, Optional[Text]]
-            env.update(os.environ)  # type: ignore
-            env.update(new_values)
+            if override:
+                env.update(os.environ)  # type: ignore
+                env.update(new_values)
+            else:
+                env.update(new_values)
+                env.update(os.environ)  # type: ignore
             result = "".join(atom.resolve(env) for atom in atoms)
 
         new_values[name] = result
@@ -299,10 +304,11 @@ def load_dotenv(dotenv_path=None, stream=None, verbose=False, override=False, in
                   Defaults to `False`.
     """
     f = dotenv_path or stream or find_dotenv()
-    return DotEnv(f, verbose=verbose, interpolate=interpolate, **kwargs).set_as_environment_variables(override=override)
+    dotenv = DotEnv(f, verbose=verbose, interpolate=interpolate, override=override, **kwargs)
+    return dotenv.set_as_environment_variables()
 
 
 def dotenv_values(dotenv_path=None, stream=None, verbose=False, interpolate=True, **kwargs):
     # type: (Union[Text, _PathLike, None], Optional[_StringIO], bool, bool, Union[None, Text]) -> Dict[Text, Optional[Text]]  # noqa: E501
     f = dotenv_path or stream or find_dotenv()
-    return DotEnv(f, verbose=verbose, interpolate=interpolate, **kwargs).dict()
+    return DotEnv(f, verbose=verbose, interpolate=interpolate, override=True, **kwargs).dict()
diff --git a/tests/test_main.py b/tests/test_main.py
@@ -257,6 +257,28 @@ def test_load_dotenv_existing_variable_override(dotenv_file):
     assert os.environ == {"a": "b"}
 
 
+@mock.patch.dict(os.environ, {"a": "c"}, clear=True)
+def test_load_dotenv_redefine_var_used_in_file_no_override(dotenv_file):
+    with open(dotenv_file, "w") as f:
+        f.write('a=b\nd="${a}"')
+
+    result = dotenv.load_dotenv(dotenv_file)
+
+    assert result is True
+    assert os.environ == {"a": "c", "d": "c"}
+
+
+@mock.patch.dict(os.environ, {"a": "c"}, clear=True)
+def test_load_dotenv_redefine_var_used_in_file_with_override(dotenv_file):
+    with open(dotenv_file, "w") as f:
+        f.write('a=b\nd="${a}"')
+
+    result = dotenv.load_dotenv(dotenv_file, override=True)
+
+    assert result is True
+    assert os.environ == {"a": "b", "d": "b"}
+
+
 @mock.patch.dict(os.environ, {}, clear=True)
 def test_load_dotenv_utf_8():
     stream = StringIO("a=à")
