-
Notifications
You must be signed in to change notification settings - Fork 158
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
Cross-Platform .rsp File Support #1016
base: main
Are you sure you want to change the base?
Conversation
Makes support for .rsp files cross-platform
[heart] Martin Reznik reacted to your message:
…________________________________
From: lukaskohl-msft ***@***.***>
Sent: Thursday, October 10, 2024 9:24:08 AM
To: microsoft/binskim ***@***.***>
Cc: Martin Reznik ***@***.***>; Review requested ***@***.***>
Subject: [microsoft/binskim] Cross-Platform .rsp File Support (PR #1016)
Unwrapping/expanding .rsp files used to be supported on Windows only due to FFI calling to shell32.dll. This PR removes FFI in favor of a code native solution.
the sarif-sdk submodule contains a reimplementation of CommandLineToArgvW. Rather than updating sarif-sdk, we've moved the relevant code into the BinSkim codebase, and pointed to the reimplementation instead to slightly lessen our reliance on sarif-sdk, while simultaneously increasing our supported platforms.
Alongside this change, tests were ported and altered over into the BinSkim codebase.
________________________________
You can view, comment on, or merge this pull request online at:
#1016
Commit Summary
* 06c1fac<06c1fac> RSP File Support for Linux
* e076a1d<e076a1d> Argument & RSP support file segmentation
* e157372<e157372> Argument Splitting and RSP Support Tests
File Changes
(3 files<https://github.com/microsoft/binskim/pull/1016/files>)
* M src/BinSkim.Driver/BinSkim.cs<https://github.com/microsoft/binskim/pull/1016/files#diff-31497e69e30b8b3b693a042e83ca6e00c291ad54ba7faf6b6dd48c47a1031517> (2)
* A src/BinSkim.Driver/ExpandArguments.cs<https://github.com/microsoft/binskim/pull/1016/files#diff-d6c363f4cf8cf3d83a4c8d747eeac13c05c2e69399e056f4847a379c0d60d4a1> (57)
* A src/Test.UnitTests.BinSkim.Driver/ExpandArgumentsUnitTests.cs<https://github.com/microsoft/binskim/pull/1016/files#diff-4d6596a28b13ae2fbc9406e29103cb4af98a93e886836ae94ad3812429a9f984> (154)
Patch Links:
* https://github.com/microsoft/binskim/pull/1016.patch
* https://github.com/microsoft/binskim/pull/1016.diff
—
Reply to this email directly, view it on GitHub<#1016>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADUOOUZUK5CKS63C5IRMY73Z2ZBTRAVCNFSM6AAAAABPWKSNVKVHI2DSMVQWIX3LMV43ASLTON2WKOZSGU3TQMRQGU2TOMQ>.
You are receiving this because your review was requested.Message ID: ***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left a few comments / questions.
|
||
private static bool IsResponseFileArgument(string argument) | ||
{ | ||
return argument.Length > 1 && argument[0] == '@'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: You may want to extract @
into some reasonably named constant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure!
Example of an .rsp file: "/example/to/binary/example.exe" # Binary # ---- Libraries ---- "/example/to/libraries/library1.dll" "/example/to/libraries/library2.dll" would result in: "/example/to/binary/example.exe" "/example/to/libraries/library1.dll" "/example/to/libraries/library2.dll"
a22509c
to
54b8982
Compare
foreach (string argument in args) | ||
{ | ||
string trimArgument = argument.Trim('"'); | ||
if (!IsResponseFileArgument(trimArgument)) | ||
{ | ||
expandedArguments.Add(trimArgument); | ||
continue; | ||
} | ||
|
||
string responseFile = trimArgument.Substring(1); | ||
|
||
responseFile = environmentVariables.ExpandEnvironmentVariables(responseFile); | ||
responseFile = fileSystem.PathGetFullPath(responseFile); | ||
|
||
string[] responseFileLines = fileSystem.FileReadAllLines(responseFile); | ||
ExpandResponseFile(responseFileLines, expandedArguments); | ||
} |
Check notice
Code scanning / CodeQL
Missed opportunity to use Select Note
Unwrapping/expanding .rsp files used to be supported on Windows only due to FFI calling to
shell32.dll
. This PR removes FFI in favor of a code native solution.the
sarif-sdk
submodule contains a reimplementation ofCommandLineToArgvW
. Rather than updatingsarif-sdk
, we've moved the relevant code into the BinSkim codebase, and pointed to the reimplementation instead to slightly lessen our reliance onsarif-sdk
, while simultaneously increasing our supported platforms.Alongside this change, tests were ported and altered over into the BinSkim codebase.