fix EnvironmentManager updateEnv

[Timo972]

This PR fixes an issue inside the updateEnv method of the EnvironmentManager.

Previous output when updating mail config:

# ...
""""""""MAIL_FROM_NAME="XXX"""""""""

MAIL_FROM_NAME="XXX"

Fixed output:

# ...
MAIL_FROM_NAME="XXX"

I have also added the MAIL_FROM_NAME and MAIL_FROM_ADDRESS keys to the .env.example file to be able to set these for the Docker container. The inject.sh file inside the Docker container only replaces existing env vars and since these two are not existing in the example, one cannot set them.

[gdarko]

Hey @Timo972

Regarding the encode() call, can you give more context on the removed line? Thanks.

I will wait on the Docker change because we are working to re-factor the Docker code to fix some ongoing issues.

Best Regards,
Darko

[Timo972]

Hey @gdarko,

I will elaborate on the logic of the updateEnv method.

In the current state we accept a dictionary which shall either be appended to or updated in the environment array. In order to achieve this we are looping over the dictionary. Inside one iteration of this loop we iterate the WHOLE environment array and check if the key of the env string matches the current $data_key. In case it matches, we are replacing the env array value with our newly encoded key=value pair. Otherwise (this is what I deleted) we re-encode the WHOLE environment line which means for example 'MAIL_FROM_NAME="XX X"' turns into '"MAIL_FROM_NAME="XX X""'. This is flawed behaviour, since if the keys don't match we shouldn't touch the env line at all. The reason it worked so far is because the encode method does nothing unless it detects a special character, which is often only the case for the MAIL_FROM_NAME env.

Now that I took another look at it, I noticed we could reduce the runtime even further: Right now its O(m*n) because of the two nested for loops where m equals the length of the dictionary and n equals the count of the environment variables. We could cut it down to O(m+n) by simply iterating the environment array first and updating all values that match and afterwards appending the missing env's to the end.
However I'm not sure if its worth the effort though since the updateEnv method is only called for updating settings.

Best regards

[gdarko]

Hey @Timo972 - I reviewed this throughly. Thank you so much for the dedication. Merging it now.