support bash's ANSI-C quoted strings #366

verhovsky · 2021-03-15T12:34:32Z

Closes #346

verhovsky · 2021-03-15T12:38:59Z

Except for numbers and upper/lower case ASCII letters, I actually haven't implemented control code escapes the way bash implements them

Examples of how bash implements control codes

$ echo -n $'ф' | xxd
00000000: d184                                     ..
$ echo -n $'\cф' | xxd
00000000: 1184                                     ..
$ echo -n $'🟥' | xxd
00000000: f09f 9fa5                                ....
$ echo -n $'\c🟥' | xxd
00000000: 109f 9fa5                                ....
$ echo -n $'^M' | xxd
00000000: 0d                                       .
$ echo -n $'\c^M' | xxd
00000000: 0d                                       .
$ echo -n $'1' | xxd
00000000: 31                                       1
$ echo -n $'\c1' | xxd
00000000: 11                                       .

$ echo -n ' ' | xxd -b
00000000: 00100000                                                
$ echo -n $'\c ' | xxd -b  # null
$ echo -n '!' | xxd -b
00000000: 00100001                                               !
$ echo -n $'\c!' | xxd -b
00000000: 00000001                                               .
$ echo -n '/' | xxd -b
00000000: 00101111                                               /
$ echo -n $'\c/' | xxd -b
00000000: 00001111                                               .
$ echo -n '0' | xxd -b
00000000: 00110000                                               0
$ echo -n $'\c0' | xxd -b
00000000: 00010000                                               .
$ echo -n '?' | xxd -b
00000000: 00111111                                               ?
$ echo -n $'\c?' | xxd -b
00000000: 01111111                                               .
$ echo -n '@' | xxd -b
00000000: 01000000                                               @
$ echo -n $'\c@' | xxd -b  # null value
$ echo -n 'A' | xxd -b
00000000: 01000001                                               A
$ echo -n $'\cA' | xxd -b
00000000: 00000001                                               .
$ echo -n 'O' | xxd -b
00000000: 01001111                                               O
$ echo -n $'\cO' | xxd -b
00000000: 00001111                                               .
$ echo -n 'P' | xxd -b
00000000: 01010000                                               P
$ echo -n $'\cP' | xxd -b
00000000: 00010000                                               .
$ echo -n '_' | xxd -b
00000000: 01011111                                               _
$ echo -n $'\c_' | xxd -b
00000000: 00011111                                               .
$ echo -n '`' | xxd -b
00000000: 01100000                                               `
$ echo -n $'\c`' | xxd -b
$ echo -n 'a' | xxd -b
00000000: 01100001                                               a
$ echo -n $'\ca' | xxd -b
00000000: 00000001                                               .
$ echo -n 'o' | xxd -b
00000000: 01101111                                               o
$ echo -n $'\co' | xxd -b
00000000: 00001111                                               .
$ echo -n 'p' | xxd -b
00000000: 01110000                                               p
$ echo -n $'\cp' | xxd -b
00000000: 00010000                                               .
$ echo -n '~' | xxd -b
00000000: 01111110                                               ~
$ echo -n $'\c~' | xxd -b
00000000: 00011110                                               .
$ echo -n ' ' | xxd -b
00000000: 00100000                                                
$ echo -n $'\c ' | xxd -b  # null char
$ echo -n '!' | xxd -b
00000000: 00100001                                               !
$ echo -n $'\c!' | xxd -b
00000000: 00000001                                               .
$ echo -n 罿 | xxd -b
00000000: 11100111 10111101 10111111                             ...
$ echo -n $'\c罿' | xxd -b
00000000: 00000111 10111101 10111111                             ...

If someone finds it in the bash source code and tells me what it's doing, I can implement it correctly.

Also, ANSI-C quoted strings are a bash thing, not a sh thing, we may want to have a flag to let users toggle this behavior.

verhovsky · 2021-03-17T22:56:29Z

There will be a mismatch in parsing because in bash you can go past the last code point 0x10FFFF, but you can't do that in node (or Python):

$ echo -n $'\U10FFFF'  | xxd
00000000: f48f bfbf                                ....
$ echo -n $'\U110000'  | xxd
00000000: f490 8080                                ....
$ node
Welcome to Node.js v14.14.0.
Type ".help" for more information.
> String.fromCodePoint(0x10FFFF)
'�'
> String.fromCodePoint(0x110000)
Uncaught RangeError: Invalid code point 1114112
    at Function.fromCodePoint (<anonymous>)
> 
$ python3
Python 3.9.0+ (default, Oct 19 2020, 09:51:18) 
[GCC 10.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> "\U0010FFFF"
'\U0010ffff'
>>> "\U00110000"
  File "<stdin>", line 1
    "\U00110000"
                ^
SyntaxError: (unicode error) 'unicodeescape' codec can't decode bytes in position 0-9: illegal Unicode character

verhovsky · 2021-03-19T15:39:37Z

Control characters are proving to be a real pain. It's complicated by the fact that my bash uses UTF-8 (and other encodings are supported) but JS uses UTF-16. I'm thinking to only do the control characters that make sense and throwing an error for the rest.

I tried reverse engineering what bash does with \c codes by adding this code in test/yargs-parser.cjs:

    function toBinaryString (buf) {
      let result = []
      for (let b of buf) {
        result.push(b.toString(2).padStart(8, '0'))
      }
      return result.join(' ')
    }

    async function check (ansiString, encoding = 'utf8') {
      const result = parser(['--foo', ansiString]).foo + '' // convert integers back to str
      const resultBuffer = Buffer.from(result, encoding)

      const { stdout } = await exec(
        'printf "%s" ' + ansiString,
        { shell: '/bin/bash', encoding: 'buffer' }
      )
      if (!resultBuffer.equals(stdout)) {
        const binaryResult = toBinaryString(resultBuffer);
        const binaryStdout = stdout.length ? toBinaryString(stdout) : '00000000 (null)';
        if (binaryResult !== binaryStdout) {
          console.log(JSON.stringify(ansiString), ansiString[4], result.length, resultBuffer, resultBuffer.length, stdout, stdout.length)
          console.log(toBinaryString(Buffer.from(ansiString[4], 'utf8')))
          console.log(binaryStdout)
          // con sole.log(binaryResult)
          console.log()
        }
        if (stdout.length) {
          // throw 'aoeu'
        }
      } else {
        // console.log(JSON.stringify(ansiString), 'works correctly')
      }
      // resultBuffer.should.deep.equal(stdout)
    }

    it('parses all control codes in ANSI-C quoted strings like bash', async () => {
      for (let i = 1; i <= 0x10FFFF; i++) {
        let chr =  String.fromCodePoint(i);
        if (chr === "'" || chr === "\\") {  // TODO: escape
          continue
        }
        await check("$'\\c" + chr + "'")
        // await check("$'\\c" + chr + " '")
      }
    // }).timeout(5 * 60 * 60 * 1000)
    }).timeout(60 * 1000)

jonluca · 2021-03-30T14:18:36Z

@bcoe could we get this merged into upstream?

verhovsky · 2021-03-30T20:01:48Z

@jonluca I still haven't finished control codes, at a minimum it needs to raise an error for the control codes that aren't handled.

If you'd like, feel free to copy paste parts of this code to curlconverter. Take a look at the source of where Chrome generates the curl command, you don't need to handle everything (like control codes) https://github.com/ChromeDevTools/devtools-frontend/blob/90e5470c94875d4417e5d31e2ab84f9b0682f33c/front_end/network/NetworkLogView.js#L2297-L2326

bcoe · 2021-03-31T23:11:12Z

@verhovsky @jonluca I am supportive of this feature, but since it's breaking and fairly specialized, I'd like for it to be behind the configuration option --bash-ansi-strings, then folks who want the behavior for their library just need to turn the feature on in their package.json, by adding it to a yargs stanza.

bcoe

I'd expect the throughput of the string parsing to be 1000s or 100000s of operations a second, so I don't see why we should need the timeouts added to test cases.

Added a couple additional comments, along with the note I added about putting this behind a feature flag.

bcoe · 2021-04-06T15:09:45Z

test/yargs-parser.cjs

+        }
+        await checkAll('', i.toString(8), 3, 'ascii')
+      }
+    }).timeout(5000)


I'm surprised by the need to extend the timeout, the parsing of strings should be quite fast.

This test runs a bash command through exec 511 times. On my desktop it completes in slightly under the default 2 second timeout, but I added some headroom.

I removed these tests, they were more to help me understand how bash does the parsing while I was working on this.

bcoe · 2021-04-06T15:10:14Z

lib/yargs-parser.ts

@@ -629,6 +632,59 @@ export class YargsParser {
      return value
    }

+    // ANSI-C quoted strings are a bash-only feature and have the form $'some text'
+    // https://www.gnu.org/software/bash/manual/html_node/ANSI_002dC-Quoting.html
+    function parseAnsiCQuote (str: string): string {


Let's pul lib/string-utils.ts.

bcoe · 2021-04-06T15:11:02Z

test/yargs-parser.cjs

+
+      // TODO: echo -n $'\U110000' produces f4 90 80 80 but String.fromCodePoint(0x110000)
+      // raises an error in node: Uncaught RangeError: Invalid code point 1114112
+    }).timeout(6000 * 1000)


Likewise, let's move these tests into test/string-utils.ts.

verhovsky · 2021-04-07T10:04:09Z

I named it --parse-bash-ansi-c-strings instead of --bash-ansi-strings. It's more verbose but it's more correct, there's no such thing as "ANSI strings". We could name it --bash-c-strings or --bash-c-quoted-strings. The point is that they are bash-specific string syntax that uses C's quoting rules.

verhovsky · 2021-04-07T11:14:29Z

README.md

+
+```console
+> example.js $'hello\\nworld'
+{ _: [ "$'hello\\nworld'" ] }


I wrote this example by hand. how do I generate it properly?

Easiest thing to do might be to clone yargs and yargs-parser, then do this:

cd yargs-parser npm link . cd yargs npm link yargs-parser // write your example in a file that includes the local copy of yargs.

bcoe · 2021-04-07T16:31:47Z

test/string-utils.cjs

+    }
+
+    // Skip exhaustive testing because it takes a long time
+    xit('parses all octal codes in ANSI-C quoted strings like bash', async () => {


How do you run these tests? Perhaps you could add an additional test-exhaustive script to package.json, and we could run them just when we merge to the main branch.

bcoe · 2021-06-20T17:16:51Z

@verhovsky I'm getting back to open source after a break earlier this year. Could I bother you for a rebase 👏

Working on driving yargs-parser down to zero open PRs.

bcoe · 2021-07-12T20:35:24Z

@verhovsky this feature looks quite solid to me, except for a couple a broken test in optimize, is this ready to move out of draft?

Thank you for the contribution 👍

bcoe · 2021-07-22T20:55:49Z

@verhovsky friendly nudge, need any help getting this over finish line?

verhovsky · 2021-07-30T18:22:11Z

@bcoe sorry for the inactivity. I realized I put this feature in the wrong place. It needs to go in the tokenizer and I think I have to improve the tokenizer to parse $-prefixed strings first.

yargs has 2 modes of operation, either someone (or a script) calls a JS script through bash, which means bash takes the user input command string and parses it (this is the step that parses ANSI-C quoted strings), splitting it into words which then get passed to Node and it exposes that as an array of strings which then get passed to yargs. The other mode is when yargs gets passed a string, presumably this string is a raw bash command that needs to be parsed using bash's rules to build the array of arguments first.

I added my code to work after the first step (i.e. to operate on the array), which is not the right place for this to go. It needs to happen in lib/tokenize-arg-string.ts but I noticed the bash parsing there is quite rudimentary, I thought it would be good to do real parsing. So I marked the PR as draft and started looking into how to parse bash in JavaScript. I had these ideas:

bash's parsing is implemented with yacc, here https://git.savannah.gnu.org/cgit/bash.git/tree/parse.y . I thought we could run it with a JS yacc implementation (i.e. jison), but if I understand it correctly, the yacc grammar is used to generate a C program, so you'd still need to rewrite that stuff in JavaScript
use someone else's JS bash parser. I only see js-shell-parse and bash-parser, both are kinda unmaintained and neither seem to handle ANSI-C quoted strings. What else do they not handle?
convert the C code that does shell parsing in a simpler shell such as dash https://git.kernel.org/pub/scm/utils/dash/dash.git/tree/src/parser.c
compile parsing logic from either bash or dash to Wasm
build on the shell parsing from curl-to-go, which seems a bit better

(Edit: I ended up using tree-sitter-bash instead of yargs for parsing Bash and implementing option parsing myself)

You already had basically this conversation in #302

bcoe · 2021-10-23T21:05:39Z

@verhovsky thanks for keeping me in the loop 👍

One thought, perhaps we could add an approach for providing an extension to the tokenizer, so that you could inject a parser like tree-sitter-bash?

I'm going to go ahead and close this, since we've stalled. But please feel free to open the conversation back up at any point.

verhovsky force-pushed the master branch from 168ad5a to a16e57f Compare March 15, 2021 12:36

verhovsky force-pushed the master branch 5 times, most recently from d61116c to c1420f7 Compare March 15, 2021 13:48

verhovsky changed the title ~~suport bash's ANSI-C quoted strings~~ support bash's ANSI-C quoted strings Mar 15, 2021

verhovsky force-pushed the master branch 7 times, most recently from 3000ecb to 94fd708 Compare March 17, 2021 22:50

verhovsky force-pushed the master branch from 94fd708 to 280f5f9 Compare March 19, 2021 05:04

bcoe requested changes Apr 6, 2021

View reviewed changes

verhovsky force-pushed the master branch 2 times, most recently from c698d28 to 2cfd1bc Compare April 7, 2021 10:00

verhovsky force-pushed the master branch 3 times, most recently from 06a1a2c to 65701b9 Compare April 7, 2021 10:30

verhovsky commented Apr 7, 2021

View reviewed changes

bcoe reviewed Apr 7, 2021

View reviewed changes

verhovsky force-pushed the master branch 3 times, most recently from 0b1d9ff to ffefd57 Compare July 4, 2021 18:06

verhovsky requested a review from bcoe July 4, 2021 18:07

verhovsky force-pushed the master branch 2 times, most recently from ab6d8b6 to 732964a Compare July 4, 2021 18:11

convert bash's ANSI-C quoted strings to their value

1a73836

verhovsky force-pushed the master branch from 732964a to 1a73836 Compare July 4, 2021 18:14

verhovsky marked this pull request as draft July 11, 2021 04:38

Merge branch 'main' into master

e1d5fb9

verhovsky mentioned this pull request Sep 22, 2021

feat: separate field + node to access strings content tree-sitter/tree-sitter-bash#103

Closed

bcoe closed this Oct 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support bash's ANSI-C quoted strings #366

support bash's ANSI-C quoted strings #366

verhovsky commented Mar 15, 2021 •

edited

Loading

verhovsky commented Mar 15, 2021 •

edited

Loading

verhovsky commented Mar 17, 2021 •

edited

Loading

verhovsky commented Mar 19, 2021

jonluca commented Mar 30, 2021

verhovsky commented Mar 30, 2021

bcoe commented Mar 31, 2021

bcoe left a comment

bcoe Apr 6, 2021

verhovsky Apr 7, 2021

verhovsky Jul 4, 2021

bcoe Apr 6, 2021

bcoe Apr 6, 2021

verhovsky commented Apr 7, 2021 •

edited

Loading

verhovsky Apr 7, 2021

bcoe Apr 7, 2021

bcoe Apr 7, 2021

bcoe commented Jun 20, 2021

bcoe commented Jul 12, 2021

bcoe commented Jul 22, 2021

verhovsky commented Jul 30, 2021 •

edited

Loading

bcoe commented Oct 23, 2021

support bash's ANSI-C quoted strings #366

support bash's ANSI-C quoted strings #366

Conversation

verhovsky commented Mar 15, 2021 • edited Loading

verhovsky commented Mar 15, 2021 • edited Loading

verhovsky commented Mar 17, 2021 • edited Loading

verhovsky commented Mar 19, 2021

jonluca commented Mar 30, 2021

verhovsky commented Mar 30, 2021

bcoe commented Mar 31, 2021

bcoe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

verhovsky commented Apr 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcoe commented Jun 20, 2021

bcoe commented Jul 12, 2021

bcoe commented Jul 22, 2021

verhovsky commented Jul 30, 2021 • edited Loading

bcoe commented Oct 23, 2021

verhovsky commented Mar 15, 2021 •

edited

Loading

verhovsky commented Mar 15, 2021 •

edited

Loading

verhovsky commented Mar 17, 2021 •

edited

Loading

verhovsky commented Apr 7, 2021 •

edited

Loading

verhovsky commented Jul 30, 2021 •

edited

Loading