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

Environment copied before being mutated #520

Closed
lionello opened this issue Jan 2, 2024 · 3 comments · Fixed by #549
Closed

Environment copied before being mutated #520

lionello opened this issue Jan 2, 2024 · 3 comments · Fixed by #549

Comments

@lionello
Copy link
Contributor

lionello commented Jan 2, 2024

This line initializes the Environment, when not initialized by the caller:

configDetails.Environment = map[string]string{}

But the environment gets copied (implicitly) earlier on this line:

LookupValue: configDetails.LookupEnv,

So, when Environment is nil, the set of COMPOSE_PROJECT_NAME in the map is not visible by the call to LookupValue.

A potential fix is to change func (cd ConfigDetails) LookupEnv(…) to a pointer receiver to avoid the copy.

@ndeloof
Copy link
Collaborator

ndeloof commented Jan 22, 2024

afaict, LookupValue: configDetails.LookupEnv captures LookupEnv func with configDetails receiver. Later configDetails.Environment value is set, this doesn't change reference to configDetails used as receiver and as such execution of this func will use this map. But maybe I'm wrong with actual reference in use.

Do you have an explicit scenario which demonstrates such an issue ?

@lionello
Copy link
Contributor Author

That's correct, but only if it's not nil to begin with. If initialized to a new map, all's good and the project name is being picked up (even though it's added after the capture), but if it's nil, then the project name is added to the new map, but the captured value is still nil.

@ndeloof
Copy link
Collaborator

ndeloof commented Jan 24, 2024

I don't get it. we capture LookupEnv func, so actually the configDetails receiver, then when executed this function will access receiver field Environment, even this one was set after.
Please provide an illustration example (either code or docker compose usage) to demonstrate a bug

lionello added a commit to DefangLabs/compose-go that referenced this issue Jan 24, 2024
This avoids capturing the nil Environment

Signed-off-by: Lionello Lunesu <[email protected]>
ndeloof pushed a commit that referenced this issue Jan 25, 2024
This avoids capturing the nil Environment

Signed-off-by: Lionello Lunesu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants