feat(disableEval): add static parsers (#3365)

* add static parser, currently disabled * fix PR comments * apply linting fixes * fix linting in tests * Update lib/connection_config.js Co-authored-by: Weslley Araújo <[email protected]> * Update lib/parsers/static_text_parser.js Co-authored-by: Weslley Araújo <[email protected]> * chore: improve assertion identification * chore: use `verbose` test reporter * ci: increase timeout * ci: test static parser through all tests and variations * chore: ensure Buffer when TypeCast is false * chore: implement `nestTables` to static parser * chore: implement `jsonStrings` to static parser * ci: check static parser coverage * ci: improve test coverage identification * chore: improve dynamic properties security * refactor: use `disableEval` instead of `useStaticParser` * feat(execute): add binary static parser * ci: adapt coverage limits --------- Co-authored-by: Andrey Sidorov <[email protected]> Co-authored-by: Weslley Araújo <[email protected]>
sidorares · Feb 4, 2025 · 51da653 · 51da653
1 parent 409d2f9
commit 51da653
Show file tree

Hide file tree

Showing 15 changed files with 506 additions and 86 deletions.
diff --git a/.github/workflows/ci-coverage.yml b/.github/workflows/ci-coverage.yml
@@ -3,7 +3,7 @@ name: CI - Coverage
 on:
   pull_request:
   push:
-    branches: [ master ]
+    branches: [master]
 
   workflow_dispatch:
 
@@ -20,14 +20,16 @@ jobs:
       fail-fast: false
       matrix:
         node-version: [20.x]
-        mysql-version: ["mysql:5.7", "mysql:8.0.33"]
+        mysql-version: ['mysql:5.7', 'mysql:8.0.33']
         use-compression: [0, 1]
         use-tls: [0, 1]
-        mysql_connection_url_key: [""]
+        static-parser: [0, 1]
+        mysql_connection_url_key: ['']
     env:
       MYSQL_CONNECTION_URL: ${{ secrets[matrix.mysql_connection_url_key] }}
+      STATIC_PARSER: ${{ matrix.static-parser }}
 
-    name: Coverage ${{ matrix.node-version }} - DB ${{ matrix.mysql-version }}${{ matrix.mysql_connection_url_key }} - SSL=${{matrix.use-tls}} Compression=${{matrix.use-compression}}
+    name: Coverage ${{ matrix.node-version }} - DB ${{ matrix.mysql-version }}${{ matrix.mysql_connection_url_key }} - SSL=${{matrix.use-tls}} Compression=${{matrix.use-compression}} Static Parser=${{matrix.static-parser}}
 
     steps:
       - uses: actions/checkout@v4
@@ -62,5 +64,5 @@ jobs:
         uses: codecov/codecov-action@v4
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
-          flags: compression-${{ matrix.use-compression }},tls-${{ matrix.use-tls }}
-          name: codecov-umbrella-${{ matrix.node-version }}-${{ matrix.mysql-version }}-compression-${{ matrix.use-compression }}-tls-${{ matrix.use-tls }}
+          flags: compression-${{ matrix.use-compression }},tls-${{ matrix.use-tls }},static-parser-${{ matrix.static-parser }}
+          name: codecov-umbrella-${{ matrix.node-version }}-${{ matrix.mysql-version }}-compression-${{ matrix.use-compression }}-tls-${{ matrix.use-tls }}-static-parser-${{ matrix.static-parser }}
diff --git a/.github/workflows/ci-linux.yml b/.github/workflows/ci-linux.yml
@@ -22,12 +22,15 @@ jobs:
         mysql-version: ['mysql:8.0.33']
         use-compression: [0, 1]
         use-tls: [0, 1]
+        static-parser: [0, 1]
         mysql_connection_url_key: ['']
         # TODO - add mariadb to the matrix. currently few tests are broken due to mariadb incompatibilities
+
     env:
       MYSQL_CONNECTION_URL: ${{ secrets[matrix.mysql_connection_url_key] }}
+      STATIC_PARSER: ${{ matrix.static-parser }}
 
-    name: Node.js ${{ matrix.node-version }} - DB ${{ matrix.mysql-version }}${{ matrix.mysql_connection_url_key }} - SSL=${{matrix.use-tls}} Compression=${{matrix.use-compression}}
+    name: Node.js ${{ matrix.node-version }} - DB ${{ matrix.mysql-version }}${{ matrix.mysql_connection_url_key }} - SSL=${{matrix.use-tls}} Compression=${{matrix.use-compression}} Static Parser=${{matrix.static-parser}}
 
     steps:
       - uses: actions/checkout@v4
@@ -57,7 +60,7 @@ jobs:
 
       - name: Run tests
         run: FILTER=${{matrix.filter}} MYSQL_USE_TLS=${{ matrix.use-tls }} MYSQL_USE_COMPRESSION=${{ matrix.use-compression }} npm run test
-        timeout-minutes: 5
+        timeout-minutes: 10
 
   tests-linux-bun:
     runs-on: ubuntu-latest
@@ -68,8 +71,12 @@ jobs:
         mysql-version: ['mysql:8.0.33']
         use-compression: [0, 1]
         use-tls: [0, 1]
+        static-parser: [0, 1]
+
+    env:
+      STATIC_PARSER: ${{ matrix.static-parser }}
 
-    name: Bun ${{ matrix.bun-version }} - DB ${{ matrix.mysql-version }} - SSL=${{matrix.use-tls}} Compression=${{matrix.use-compression}}
+    name: Bun ${{ matrix.bun-version }} - DB ${{ matrix.mysql-version }} - SSL=${{matrix.use-tls}} Compression=${{matrix.use-compression}} Static Parser=${{matrix.static-parser}}
 
     steps:
       - uses: actions/checkout@v4
@@ -108,7 +115,7 @@ jobs:
           MYSQL_USE_TLS: ${{ matrix.use-tls }}
           FILTER: test-select-1|test-select-ssl
         run: bun run test:bun
-        timeout-minutes: 1
+        timeout-minutes: 10
 
   tests-linux-deno-v1:
     runs-on: ubuntu-latest
@@ -118,14 +125,18 @@ jobs:
         deno-version: [v1.x]
         mysql-version: ['mysql:8.0.33']
         use-compression: [0, 1]
+        static-parser: [0, 1]
         # TODO: investigate error when using SSL (1)
         #
         # errno: -4094
         # code: "UNKNOWN"
         # syscall: "read"
         use-tls: [0]
 
-    name: Deno ${{ matrix.deno-version }} - DB ${{ matrix.mysql-version }} - SSL=${{matrix.use-tls}} Compression=${{matrix.use-compression}}
+    env:
+      STATIC_PARSER: ${{ matrix.static-parser }}
+
+    name: Deno ${{ matrix.deno-version }} - DB ${{ matrix.mysql-version }} - SSL=${{matrix.use-tls}} Compression=${{matrix.use-compression}} Static Parser=${{matrix.static-parser}}
 
     steps:
       - uses: actions/checkout@v4
@@ -162,7 +173,7 @@ jobs:
           MYSQL_USE_COMPRESSION: ${{ matrix.use-compression }}
           MYSQL_USE_TLS: ${{ matrix.use-tls }}
         run: deno task test:deno -- --denoCjs='.js,.cjs'
-        timeout-minutes: 5
+        timeout-minutes: 10
 
   tests-linux-deno-v2:
     runs-on: ubuntu-latest
@@ -172,14 +183,18 @@ jobs:
         deno-version: [v2.x, canary]
         mysql-version: ['mysql:8.0.33']
         use-compression: [0, 1]
+        static-parser: [0, 1]
         # TODO: investigate error when using SSL (1)
         #
         # errno: -4094
         # code: "UNKNOWN"
         # syscall: "read"
         use-tls: [0]
 
-    name: Deno ${{ matrix.deno-version }} - DB ${{ matrix.mysql-version }} - SSL=${{matrix.use-tls}} Compression=${{matrix.use-compression}}
+    env:
+      STATIC_PARSER: ${{ matrix.static-parser }}
+
+    name: Deno ${{ matrix.deno-version }} - DB ${{ matrix.mysql-version }} - SSL=${{matrix.use-tls}} Compression=${{matrix.use-compression}} Static Parser=${{matrix.static-parser}}
 
     steps:
       - uses: actions/checkout@v4
@@ -216,4 +231,4 @@ jobs:
           MYSQL_USE_COMPRESSION: ${{ matrix.use-compression }}
           MYSQL_USE_TLS: ${{ matrix.use-tls }}
         run: deno task test:deno
-        timeout-minutes: 5
+        timeout-minutes: 10
diff --git a/.nycrc b/.nycrc
@@ -3,10 +3,10 @@
   "include": ["index.js", "promise.js", "lib/**/*.js"],
   "exclude": ["mysqldata/**", "node_modules/**", "test/**"],
   "reporter": ["text", "lcov", "cobertura"],
-  "statements": 86,
-  "branches": 84,
+  "statements": 80,
+  "branches": 80,
   "functions": 77,
-  "lines": 86,
+  "lines": 80,
   "checkCoverage": true,
   "clean": true
 }
diff --git a/lib/commands/execute.js b/lib/commands/execute.js
@@ -5,6 +5,7 @@ const Query = require('./query.js');
 const Packets = require('../packets/index.js');
 
 const getBinaryParser = require('../parsers/binary_parser.js');
+const getStaticBinaryParser = require('../parsers/static_binary_parser.js');
 
 class Execute extends Command {
   constructor(options, callback) {
@@ -25,12 +26,16 @@ class Execute extends Command {
     this._executeOptions = options;
     this._resultIndex = 0;
     this._localStream = null;
-    this._unpipeStream = function() {};
+    this._unpipeStream = function () {};
     this._streamFactory = options.infileStreamFactory;
     this._connection = null;
   }
 
   buildParserFromFields(fields, connection) {
+    if (this.options.disableEval) {
+      return getStaticBinaryParser(fields, this.options, connection.config);
+    }
+
     return getBinaryParser(fields, this.options, connection.config);
   }
 
@@ -42,7 +47,7 @@ class Execute extends Command {
       this.statement.id,
       this.parameters,
       connection.config.charsetNumber,
-      connection.config.timezone
+      connection.config.timezone,
     );
     //For reasons why this try-catch is here, please see
     // https://github.com/sidorares/node-mysql2/pull/689
@@ -68,7 +73,7 @@ class Execute extends Command {
     //  this.statement.columns[this._receivedFieldsCount] : new Packets.ColumnDefinition(packet);
     const field = new Packets.ColumnDefinition(
       packet,
-      connection.clientEncoding
+      connection.clientEncoding,
     );
     this._receivedFieldsCount++;
     this._fields[this._resultIndex].push(field);
@@ -87,7 +92,7 @@ class Execute extends Command {
     }
     this._rowParser = new (this.buildParserFromFields(
       this._fields[this._resultIndex],
-      connection
+      connection,
     ))();
     return Execute.prototype.row;
   }

diff --git a/lib/commands/query.js b/lib/commands/query.js
@@ -8,6 +8,7 @@ const Readable = require('stream').Readable;
 const Command = require('./command.js');
 const Packets = require('../packets/index.js');
 const getTextParser = require('../parsers/text_parser.js');
+const staticParser = require('../parsers/static_text_parser.js');
 const ServerStatus = require('../constants/server_status.js');
 
 const EmptyPacket = new Packets.Packet(0, Buffer.allocUnsafe(4), 0, 4);
@@ -30,7 +31,7 @@ class Query extends Command {
     this._receivedFieldsCount = 0;
     this._resultIndex = 0;
     this._localStream = null;
-    this._unpipeStream = function () { };
+    this._unpipeStream = function () {};
     this._streamFactory = options.infileStreamFactory;
     this._connection = null;
   }
@@ -55,7 +56,7 @@ class Query extends Command {
 
     const cmdPacket = new Packets.Query(
       this.sql,
-      connection.config.charsetNumber
+      connection.config.charsetNumber,
     );
     connection.writePacket(cmdPacket.toPacket(1));
     return Query.prototype.resultsetHeader;
@@ -120,7 +121,7 @@ class Query extends Command {
     if (connection.config.debug) {
       // eslint-disable-next-line
       console.log(
-        `        Resultset header received, expecting ${rs.fieldCount} column definition packets`
+        `        Resultset header received, expecting ${rs.fieldCount} column definition packets`,
       );
     }
     if (this._fieldCount === 0) {
@@ -140,7 +141,7 @@ class Query extends Command {
       this._localStream = this._streamFactory(path);
     } else {
       this._localStreamError = new Error(
-        `As a result of LOCAL INFILE command server wants to read ${path} file, but as of v2.0 you must provide streamFactory option returning ReadStream.`
+        `As a result of LOCAL INFILE command server wants to read ${path} file, but as of v2.0 you must provide streamFactory option returning ReadStream.`,
       );
       connection.writePacket(EmptyPacket);
       return this.infileOk;
@@ -159,14 +160,14 @@ class Query extends Command {
       const dataWithHeader = Buffer.allocUnsafe(data.length + 4);
       data.copy(dataWithHeader, 4);
       connection.writePacket(
-        new Packets.Packet(0, dataWithHeader, 0, dataWithHeader.length)
+        new Packets.Packet(0, dataWithHeader, 0, dataWithHeader.length),
       );
     };
     const onEnd = () => {
       connection.removeListener('error', onConnectionError);
       connection.writePacket(EmptyPacket);
     };
-    const onError = err => {
+    const onError = (err) => {
       this._localStreamError = err;
       connection.removeListener('error', onConnectionError);
       connection.writePacket(EmptyPacket);
@@ -196,7 +197,7 @@ class Query extends Command {
     if (this._fields[this._resultIndex].length !== this._fieldCount) {
       const field = new Packets.ColumnDefinition(
         packet,
-        connection.clientEncoding
+        connection.clientEncoding,
       );
       this._fields[this._resultIndex].push(field);
       if (connection.config.debug) {
@@ -212,7 +213,15 @@ class Query extends Command {
     if (this._receivedFieldsCount === this._fieldCount) {
       const fields = this._fields[this._resultIndex];
       this.emit('fields', fields);
-      this._rowParser = new (getTextParser(fields, this.options, connection.config))(fields);
+      if (this.options.disableEval) {
+        this._rowParser = staticParser(fields, this.options, connection.config);
+      } else {
+        this._rowParser = new (getTextParser(
+          fields,
+          this.options,
+          connection.config,
+        ))(fields);
+      }
       return Query.prototype.fieldsEOF;
     }
     return Query.prototype.readField;
@@ -242,7 +251,7 @@ class Query extends Command {
       row = this._rowParser.next(
         packet,
         this._fields[this._resultIndex],
-        this.options
+        this.options,
       );
     } catch (err) {
       this._localStreamError = err;
@@ -274,13 +283,13 @@ class Query extends Command {
       }
       stream.emit('result', row, resultSetIndex); // replicate old emitter
     });
-    this.on('error', err => {
+    this.on('error', (err) => {
       stream.emit('error', err); // Pass on any errors
     });
     this.on('end', () => {
       stream.push(null); // pushing null, indicating EOF
     });
-    this.on('fields', fields => {
+    this.on('fields', (fields) => {
       stream.emit('fields', fields); // replicate old emitter
     });
     stream.on('end', () => {
@@ -292,10 +301,7 @@ class Query extends Command {
   _setTimeout() {
     if (this.timeout) {
       const timeoutHandler = this._handleTimeoutError.bind(this);
-      this.queryTimeout = Timers.setTimeout(
-        timeoutHandler,
-        this.timeout
-      );
+      this.queryTimeout = Timers.setTimeout(timeoutHandler, this.timeout);
     }
   }