GH952: Refactor DotNetCore args string->ArgumentBuilder #953
Conversation
devlead
force-pushed
the
feature/GH-952
branch
2 times, most recently
from
d85242b
to
4cd7964
Compare
| /// Copies all the arhuments of the current <see cref="ProcessArgumentBuilder"/> to target <see cref="ProcessArgumentBuilder"/>. | ||
| /// </summary> | ||
| /// <param name="target">The copy target.</param> | ||
| public void CopyTo(ProcessArgumentBuilder target) |
There was a problem hiding this comment.
Maybe it makes more sense exposing the tokens as a IReadOnlyList<IProcessArgument> and add those two methods as extension methods since they only require read access.
There was a problem hiding this comment.
Couldn't that potentially leak sensitive tokens?
There was a problem hiding this comment.
Well, the person who got access to the token could just as well use the Render method, or am I missing something?
gep13
changed the title
devlead
force-pushed
the
feature/GH-952
branch
2 times, most recently
from
6244715
to
7ba346b
Compare
devlead
changed the title
|
@patriksvensson refactored CopyTo / IsNullOrEmpty to pure extension methods, so think I've addressed your feedback. Also rebased against latest develop. |
| @@ -380,5 +380,39 @@ public static ProcessArgumentBuilder AppendQuotedSecret(this ProcessArgumentBuil | |||
| } | |||
| return builder; | |||
| } | |||
|
|
|||
| /// <summary> | |||
| /// Indicates whether a <see cref="ProcessArgumentBuilder"/> is null or renders empty. | |||
There was a problem hiding this comment.
Did I just have a stroke? I could have sworn it said indiates when I read it.
|
@devlead Left some minor comments. Looks good to me otherwise. |
|
@patriksvensson think I've addressed your feedback now. |
|
Merged. Awesome work 👍 |
This is a proposed fix for #952
Currently the DotNetCore DotNetCoreExecute and DotNetCoreRun uses a quoted single string for args, this breaks in some scenarios also wont allow filtering sensitive data.
This PR
CopyTo(ProcessArgumentBuilder target)method toProcessArgumentBuilderProcessArgumentBuilderto anotherIsEmpty()method toProcessArgumentBuilderProcessArgumentBuilderhas any args / would render empty.