search

masterzen / winrm

Command-line tool and library for Windows remote command execution in Go
Apache License 2.0
425 stars 129 forks source link

`DefaultParameters` looks bug-prone. #164

Open seiyab opened 7 months ago

seiyab commented 7 months ago

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. https://github.com/masterzen/winrm/blob/e811dad5ac77d4d981dc85fd43a587843274bca0/README.md#L139-L140

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

seiyab commented 7 months ago

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