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

Preserve whitespace when interpolation: true (Unix) #584

Merged
merged 10 commits into from
Dec 30, 2022

Conversation

ericcornelissen
Copy link
Owner

@ericcornelissen ericcornelissen commented Dec 13, 2022

Partially addresses #346


Summary

Add support for preserving whitespace when using escape(., { interpolation: true })/escapeAll(., { interpolation: true }) for unquoted and "unprotected"1 arguments on Unix. The result(/goal) should be that escaped arguments are no longer split on whitespace and that the original whitespace character is maintained.

Footnotes

  1. unlike protected, e.g. using escape with execFile.

Example implementation of escaping whitespace, in this case tabs, for
Unix shells. In particular when escaping unquoted (and "unprotected",
unlike e.g. calling `execFile` with a falsy `shell` value) arguments.
@ericcornelissen ericcornelissen added the enhancement New feature or request label Dec 13, 2022
@codecov
Copy link

codecov bot commented Dec 13, 2022

Codecov Report

Merging #584 (6c48806) into main (dbaa0fd) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #584   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines          840       843    +3     
=========================================
+ Hits           840       843    +3     
Flag Coverage Δ
e2e-MacOS 83.51% <0.00%> (-0.30%) ⬇️
e2e-Ubuntu 83.03% <0.00%> (-0.30%) ⬇️
e2e-Windows 83.39% <0.00%> (-0.30%) ⬇️
integration-MacOS 89.79% <100.00%> (+0.03%) ⬆️
integration-Ubuntu 90.03% <100.00%> (+0.03%) ⬆️
integration-Windows 87.18% <0.00%> (-0.32%) ⬇️
unit 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/unix.js 100.00% <100.00%> (ø)

Example implementation of escaping whitespace, in this case spaces, for
Unix shells. In particular when escaping unquoted (and "unprotected",
unlike e.g. calling `execFile` with a falsy `shell` value) arguments.

Since whitespace may change how other things are escaped, to be on the
conservative it is now escaped last. To minimize changes `\n` is escaped
after `[\t ]` (for now).

Fuzzing required some changes as a previous side effect is removed as a
result of this change. In particular, spaces (` `) introduced as a
result of replacing leading/trailing newlines would be removed due to
the trimming logic. This no longer happens due to the source code
changes in this commit. This is overcome by using an intermediate
representation that's guarenteed not to be in the string (due to line
55).

The "Hello world" sample string test case has been removed as it would
actually belong in the `"<space> (' ')"` data group. Since this kind of
string is already covered it's removed.
@ericcornelissen

This comment was marked as resolved.

@ericcornelissen ericcornelissen marked this pull request as ready for review December 27, 2022 11:33
@ericcornelissen ericcornelissen changed the title Preserve whitespace when interpolation: true Preserve whitespace when interpolation: true (Unix) Dec 28, 2022
src/unix.js Outdated Show resolved Hide resolved
With ` ` and `\t` updated to not split arguments anymore, update
escaping for Bash, Dash, and Zsh, to also escape `\n` such that it does
not result in argument splitting.
CHANGELOG.md Outdated Show resolved Hide resolved
@ericcornelissen ericcornelissen merged commit bc339a2 into main Dec 30, 2022
@ericcornelissen ericcornelissen deleted the 346-escape-perserve-whitespace branch December 30, 2022 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant