Giter Club home page Giter Club logo

Comments (8)

Artoria2e5 avatar Artoria2e5 commented on May 3, 2024 1

Updated test case for clarity.

from zx.

catpea avatar catpea commented on May 3, 2024

There is an error in the example/unit-test above, the dollar sign is missing right before the {a},
The the last comma in 'a,b,' is the trailing comma in printf '%s,' I am easily startled, and this got me going. (update: Artoria2e5 has since changed the printf field separator to pipe, now things are much easier to understand. Thank you Artoria2e5!)
Let us fix it up.

My apologies Artoria2e5, let us simplify your test a touch, I know your wrote it quickly just to show the problem, I got ya, I'll fix it.

First a small demonstration and explanation of the problem:

#!/usr/bin/env zx
const exampleArray = ['Mój', 'poduszkowiec', 'jest', 'pełen', 'węgorzy'];
$`echo ${exampleArray}`;

Prints the following in the terminal:

$ echo 'Mój,poduszkowiec,jest,pełen,węgorzy'
Mój,poduszkowiec,jest,pełen,węgorzy

The commas come from the exampleArray array being automatically cast to string by JavaScript, the same thing occurs outside of zx when we concatenate array with a string. I will use #!/usr/bin/env node here just to remove zx for a second.

#!/usr/bin/env node
const exampleArray = ['Mój', 'poduszkowiec', 'jest', 'pełen', 'węgorzy'];
const concatenation = "Hello! " + exampleArray;
console.log(concatenation);

Prints the following in the terminal:

Hello! Mój,poduszkowiec,jest,pełen,węgorzy

Now let us re-create your test Artoria2e.

  • I will use echo as people are more familiar with it.
  • I am using echo -n to prevent echo from adding a trailing new-line, to keep things simple here.
  • And becasue we are going from JavaScript to sh, the array is a space separated ordeal, thus we would expect "a b", and not the "a,b" that we are getting.
  • Please note, that I attempt to stay true to Artoria2e5 code and I make no effort to quote the output (spaces will mangle the array further), nor do I attempt to sanitize anything to prevent shellcode injection.
  • If #47 is recognized as an actual problem that people really have...
    • then the author will have to decide on the type of quotes to use,
    • whether or not to allow variable interpolation (Bash, env, $BASHPID, etc)
    • and consider string sanitization (should someone accept raw input from the internet and attempt to pass it directly into the zx/shell).

Steps to Reproduce the Problem

#!/usr/bin/env zx
import assert from 'assert';
const a = ["a", "b"];
const result = await $`echo -n ${a}`;
const actual = result.stdout;
const expected = 'a b'; // adjust this when you figure out format, interpolation, sanitization, etc...
assert.equal(actual, expected, "Array contents were cheerfully mangled on their way to the shell.");

Prints the following in the terminal:

[captain-obvious@gibson ~]$ ./zexy-test.mjs
$ echo -n a,b
a,bnode:internal/process/esm_loader:74
    internalBinding('errors').triggerUncaughtException(
                              ^

AssertionError [ERR_ASSERTION]: Array contents were cheerfully mangled on their way to the shell.
    at file:///home/captain-obvious/zexy-test.mjs:54:8
    at processTicksAndRejections (node:internal/process/task_queues:94:5) {
  generatedMessage: false,
  code: 'ERR_ASSERTION',
  actual: 'a,b',
  expected: 'a b',
  operator: '=='
}

The error being:

{
  generatedMessage: false,
  code: 'ERR_ASSERTION',
  actual: 'a,b',
  expected: 'a b',
  operator: '=='
}

from zx.

catpea avatar catpea commented on May 3, 2024

I have not looked at the tagged template function in zx (my apologies), but think that zx should throw on complex data types in the current version, if it can. Now, that is just for simplicity sake, and the sake of a solid dollop of reality, and maintenance and care of the lonesome and forlorn coder debugging JSON as argument on a Friday at 4:58PM, exit early, crash and burn, don't support arrays, or nested objects as the case will become.

But, since we live in 2021, there is another consideration here:

Join programs like jq in the "JSON in the shell" revolution.

There is already another program that can heavily rely on JSON as input: zx

The scenario I am hinting on is passing data between zx scripts as JSON.

"I guess you guys aren't ready for that, yet. But your kids are gonna love it." --Marty McFly

./zx-script.mjs --flarp "[\"a\",\"b\"]"

./another-zx-script.mjs --some-array-friendly-argument "[\"Mój\",\"poduszkowiec\",\"jest\",\"pełen\",\"węgorzy\"]" --and-a-normal-argument [email protected]

In this frivolous example both ./zx-script.mjs and ./another-zx-script.mjs use #!/usr/bin/env zx as the shebang, and JSON.parse() --flarp and --some-array-friendly-argument to rehydrate or deserialize the damn thing.

If this became a feature, some users will no doubt ask about automatic detection and deserialization of JSON arguments, so that may become another feature, and probably should (though possibly under a zx set/setopt/flag)

Artoria2e5 the author of this issue wishes for zx to bash array compatibility. Artoria2e5 wants the bash array=("a" "b") type support. What I am talking about here, is not that at all. This does not help Artoria2e5 case. What I am describing here is a crime, an atrocity, a bold move, a revolution, a curious new thing, and the torture of a coder needing to debug scripts that accept JSON as input argument.

(Security notice: Please note, the code example featuring zx-script.mjs and ./another-zx-script.mj is just a quick back of the envelope scribble, I have not considered properly escaping the JSON. The example, needs to be single quote with escaped $ and further security considerations must be contemplated. There are other issues here such as use of different shells, and dear Windows attempting to eat glue, and so on. The example is only meant to symbolize the brave use of JSON in the command line, with a fine disregard for shell script conventions of today. )

from zx.

catpea avatar catpea commented on May 3, 2024

Just a quick note, while the title of this issue is speaking about JavaScript arrays, I want to make it clear that the following are not Bash arrays, but rather just command arguments that the command will interpret as its own idea of a list.

printf '%s|' string1 string2 string3
tar -cf files.tar file1 file2 file3

This is a Bash array:

declare -a array_ex=(1 2 "3 4")

echo examples:

echo ${array_ex[@]@Q} # needs recent bash
echo ${array_ex[@]}

prints:

'1' '2' '3 4'
1 2 3 4
printf '%s|' "${array_ex[@]" # double quotes are important here

prints:

1|2|3 4|

So, we still want to fail fast and hard if someone tries to stick a JavaScript array into zx's $``. They have to serialize the array themselves. Just like #83 is doing:

$`tar czvf ${outputPath} ${inputs.join(" ")}`

This is purrfect, this is the price we pay for zx.

I am assuming what is causing the problem is that the $`` tagged template is single quoting the arguments it is getting in the tagged template JavaScript function(){}, if that is the case, then it is single quoting to prevent Bash variable expansion/interpolation, and then to preserve spaces in stings in things like:

$`espeak ${some_text}`
espeak 'This is a test' # espeak wants quotes

But we can't do that, as the following reveals:

tar -cf files.tar ${['file1', 'file2', 'file3'].join(' ')} # tar does not want quotes

tar says

tar: file1 file2 file3  : Cannot stat: No such file or directory

Note: tar is looking for a single file named file1 file2 file3.

See, we can't automatically quote input into $`` because some of that may wish to be unquoted.

Therefore, the way to run a command that demands quotes, or a string, is to get the human to put it in there (warning this creates a shell injection scenario, this is just to show that humans know which quotes to use.):

$`espeak '${some_text}'` // note manual single quotes
//or
$`espeak "${some_text}"` // note manual double quotes as some commands may wish that too

This becomes a gotcha, a security problem, and a bit of a nuisance for the user as they have to remember to quote their strings.

I took a look at the zx code and shq behaviour, and what is going on is that shq wraps the entire string in single quote, it mangles arguments that are meant to be perceived by the command as a list. When /bin/sh is parsing the command line, it treats quoted strings as a one long item in arg aray.

To fix this, we likely have to break everything else, as we have to remove the single quotations that shq is adding

For completion sake here is the test I ran on shq

import shq from 'shq';
const str = shq('a b c');
console.log(str); // prints 'a b c' because it adds ' around string
console.log(str.substr(1, str.length-2)); // prints a b c which is correct and would make ```tar -cf files.tar file1 file2 file3``` work OK.
[meow@black foo]$ node test.js # print example
'a b c' # we do not want this as it mangles tar
a b c # we want this but we also have to ask the users to remember to quote their strings.

TLDR; zx is automagically quoting strings (by means of shq) but some of the strings are meant to be left unquoted in order to be interpreted as repeating-arguments (lists) by the command receiving the argv from /bin/sh

from zx.

catpea avatar catpea commented on May 3, 2024

I just want to add, that the reason why I mention Bash arrays, and JSON above, is that the obvious solution of applying shq to each item of array basically says the only notion of an array that zx supports is a chain of quoted strings, not even a bash array.

Maybe (I just lost half my audience, dammnit), maybe... this is an opportunity to fix up ffmpeg's Complex Filter Graphs.

This would mean custom serializers/stringifiers, which could even include a JSON one.

Take a look at the last graphviz graph on this article https://medium.com/craftsmenltd/ffmpeg-basic-filter-graphs-74f287dc104e as it demonstrates that filter graphs are easy but getting them squished into a single command line is error prone.

So a custom serializer could take a https://js.cytoscape.org/ data structure and automatically convert it into a ffmpeg notation.

It could also do json, spawn arg arrays, nested arrays, arrays, and quoted strings, and probably even sub-processes.

Here is that ffmpeg example:

const struct = [[[...]]];
$`ffmpeg -y -i input.mkv -filter_complex ${$.filtergraph(struct)} -map [hd] -map [a1] hd.mov -map [sd] -map [a2] sd-flipped.mp4 -map [thumbnails] thumbnail-%03d.jpg`

and here is our tar from #83

const args = process.argv.slice(3);
const outputPath = args[0];
const inputs = args.slice(1);

await $`tar czvf ${outputPath} ${$.list(inputs)}`

So $.filtergraph(struct) serializer does complex trees, $.list(inputs) serializer does lists, we can have a $.dangerously(inputs) serializer which does "keeping sysadmins awake" $.martyMcFly(object) which does freaking JSON serialization (that may turn out to be useful for programs like jq), and users could also write their own.

There is also an opportunity for dark magic here where you create a sub process that you then feed into an argument as dirty equivalent of Bash's tar czvf output.tar.gz $(ls), but this one probably falls hard into the Marty McFly category, but this would be a $.subprocess() stringifier that may just be nasty sugar for another $`` a line above in the script.

Oh, and $.q() and $.qq() for safe, smart single quote and double quotes that will use shq, and the reason why q and qq would be better than user just putting quotes around their input as in my $`espeak "${some_text}"` example is securely escaping code injection.

from zx.

antonmedv avatar antonmedv commented on May 3, 2024

I just thinking about allowing arrays as args and quote items separately and join with space.

from zx.

catpea avatar catpea commented on May 3, 2024

Yup, this will work too.

Your approach will depend on recognizing if arg is a string or an array, perhaps by using if(Array.isArray()) and then use shq directly if it is a string, and .map(o=>shq(o)) if it is an array.

Now, what this also does. Is, makes the same assumption that we had before: all strings should be quoted.

Which is fair, it is a command runner that will turn a JavaScript array into an argument list for a command.

Keeping things simple, is the way to go here.

But there is one more teeny-tiny thing that you can add... if people complain in the future.

Right where you are testing if(Array.isArray()) add a test to see if you are getting a function(), and if so, execute it expecting a string that you will insert into that spot RAW/unquoted.

The function will be a custom serializer/stringifier allowing the user to do their own serialization.

This way, you get your automatic .map(o=>shq(o)) array serialization, but also the user can still do strange things such as serialize an object tree into an ffmpeg filtergraph string.

TLDR;

  • leave stings the way they are
  • detect arrays and map over every element with shq
  • but also consider detecting if the argument is a Function (or instance of CustomStringifier object that you ask your users to extend) and use the return value in unquoted form, leaving everything up to the user.
  • TLDRTLDR; support strings, arrays, and a custom function that returns a string that you won't auto-quote with shq.

from zx.

antonmedv avatar antonmedv commented on May 3, 2024

Fixed.

from zx.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.