Improve fuzz targets #318
Merged
Improve fuzz targets #318
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Update the fuzzing utilities to stop stripping newline characters from arguments. This expands the fuzzing scope by including all strings that contain newlines.
Avoid dealing with newline characters when fuzzing cmd.exe. It appears that, when used programatically, everything after newline characters is ignored when invoking a command with cmd.exe. For that reason, the fuzz targets will continue not dealing with newlines, but only for cmd.exe. Also add a new seed to the fuzz corpus, one that contains the `\r` (carriage return) character, that is relevant to the cmd.exe behaviour.
Expand the fuzz target for child_process.exec to include a check for usage without having a shell specified (both with and without the interpolation option set). Also, be more methodical about the checks, following the "design" of the other fuzz targets.
This enables it to be used both with and without a shell specified from fuzz targets. The existing approach, where _common.cjs looks up the fuzz shell itself, didn't always work because getting a prepared argument as well as the expected output may depend on the shell being used. In the event where the fuzz target wanted to test without specifying a shell, but a shell was specified for fuzzing, this would go wrong. The API for getting the prepared arg and the expected output has been updated to improve readability, making more of it's arguments named by using a paramater object as first argument. All fuzz targets have been updated accordingly.
Update the exec.test.cjs fuzz target to not strip out all whitespace and instead let whitespace be in scope for fuzzing. This required some changes to the prep and expected arg logic to accomodate the ways in which CMD and PowerShell work with whitespace between arguments. The additions to the fuzz corpus all relate to the way whitespace is handled by CMD and PowerShell - they're for calibrating the fuzzing. Unix shells were not tested as part of this commit.
ericcornelissen
commented
Jul 5, 2022
Newlines when not quoting the argument whatsoever is difficult to deal
with. So for now, when fuzzing with `{interpolation:true}` don't bother
with newlines.
Note that for cmd.exe it's currently necessary to always ignore newlines
because they also cause issues when the argument is quoted.
Change the implementation of normalizing whitespace for `getExpectedOutput` to replace only space (` `) and tab (`\t`) for a space on most shells, and all whitespace only for PowerShell. This is because the (tested) shells on Unix (seem to) behave the same as CMD.
Update the fuzzing with interpolation option implementation to maintain any leading and trailing whitespace in the argument being tested and just verify, through the `getExpectedOutput`, it's trimmed. This also nicely avoids the need for a custom trim function as any null chars are taken care of prior to trimming.
Omit the useless assignment to `result` and put space between the if statements for readability.
This was referenced Jul 5, 2022
Based on experimental results.
ericcornelissen
added
fuzz
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.