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

DefaultParameters looks bug-prone. #164

Open
seiyab opened this issue Apr 15, 2024 · 1 comment
Open

DefaultParameters looks bug-prone. #164

seiyab opened this issue Apr 15, 2024 · 1 comment

Comments

@seiyab
Copy link

seiyab commented Apr 15, 2024

I think DefaultParameters tend to be unintentionally mutated. Because DefaultParameters is a pointer, following code will pollute it.

params := winrm.DefaultParameters
params.Timeout = timeout // ⚠️ Ouch! winrm.DefaultParameters is also changed!

You can see such codes even in README.md.

winrm/README.md

Lines 139 to 140 in e811dad

params := DefaultParameters
params.TransportDecorator = func() Transporter { return &ClientNTLM{} }

I also put a real-world example here.
https://github.com/hashicorp/terraform/blob/8f7744da0959334f461c18ccf15f8b19d8c09fe6/internal/communicator/winrm/communicator.go#L63-L64

@seiyab
Copy link
Author

seiyab commented Apr 15, 2024

I suggest to add something like NewDefaultParameters() and add // Deprecated into DefaultParameters.

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

No branches or pull requests

1 participant