nevill / zongji

A mysql binlog listener running on Node.js.
Other
376 stars 118 forks source link

Null value in JSON column causing buffer RangeError #40

Open vlasky opened 8 years ago

vlasky commented 8 years ago

When we insert a new row into a table with a JSON column that is empty or null, we get an error like this that causes MeteorJS to exit:

W20160505-15:47:28.917(10)? (STDERR) RangeError: Trying to access beyond buffer lengt h W20160505-15:47:28.918(10)? (STDERR) at checkOffset (buffer.js:582:11) W20160505-15:47:28.918(10)? (STDERR) at Buffer.readUInt8 (buffer.js:588:5) W20160505-15:47:28.918(10)? (STDERR) at parseBinaryBuffer (C:\Users\developer\App Data\Local.meteor\packages\wj32_mysql\1.1.9\npm\nodemodules\mysql-live-select\node modules\zongji\lib\json_decode.js:55:24) W20160505-15:47:28.918(10)? (STDERR) at module.exports (C:\Users\developer\AppDat a\Local.meteor\packages\wj32_mysql\1.1.9\npm\node_modules\mysql-live-select\node_mod ules\zongji\lib\json_decode.js:28:25) W20160505-15:47:28.919(10)? (STDERR) at Object.exports.readMysqlValue (C:\Users\d eveloper\AppData\Local.meteor\packages\wj32_mysql\1.1.9\npm\node_modules\mysql-live- select\node_modules\zongji\lib\common.js:446:16) W20160505-15:47:28.919(10)? (STDERR) at UpdateRows._fetchOneRow (C:\Users\develop er\AppData\Local.meteor\packages\wj32_mysql\1.1.9\npm\node_modules\mysql-live-select \node_modules\zongji\lib\rows_event.js:144:13) W20160505-15:47:28.919(10)? (STDERR) at readRow (C:\Users\developer\AppData\Local .meteor\packages\wj32_mysql\1.1.9\npm\node_modules\mysql-live-select\node_modules\zo ngji\lib\rows_event.js:113:16) W20160505-15:47:28.919(10)? (STDERR) at UpdateRows.RowsEvent (C:\Users\developer\ AppData\Local.meteor\packages\wj32_mysql\1.1.9\npm\node_modules\mysql-live-select\no de_modules\zongji\lib\rows_event.js:64:27) W20160505-15:47:28.919(10)? (STDERR) at new UpdateRows (C:\Users\developer\AppDat a\Local.meteor\packages\wj32_mysql\1.1.9\npm\node_modules\mysql-live-select\node_mod ules\zongji\lib\rows_event.js:136:13) W20160505-15:47:28.919(10)? (STDERR) at BinlogHeader.parse (C:\Users\developer\Ap pData\Local.meteor\packages\wj32_mysql\1.1.9\npm\node_modules\mysql-live-select\node _modules\zongji\lib\packet\binlog_header.js:47:23) => Exited with code: 8

I believe the problem lies in line 55 of function parseBinaryBuffer() of /lib/json_decode.js

var jsonType = input.readUInt8(offset);

There are no prior checks to ensure that input is not empty before executing this line of code. I would fix this by inserting a line just above with the following check :

if (input.length === 0) return null;

numtel commented 8 years ago

I didn't even think about the null JSON string since all the other data types use the same output for null values.

diff --git a/test/types.js b/test/types.js
index a8c1302..412b41f 100644
--- a/test/types.js
+++ b/test/types.js
@@ -357,6 +357,8 @@ defineTypeTest('datetime_then_decimal', [
 defineTypeTest('json', [
   'JSON NULL'
 ], [
+  ['\'null\''],
+  ['\'{"key1": null}\''],
   // Small Object
   ['\'{"key1": "value1", "key2": "value2", "key3": 34}\''],
   // Small Object with nested object

Adding these to the JSON type test does not cause it to fail against master. I don't know how to reproduce this.

A JSON string cannot be empty it seems:

> JSON.parse('')
SyntaxError: Unexpected end of JSON input
    at Object.parse (native)
    at repl:1:6
    at REPLServer.defaultEval (repl.js:272:27)
    at bound (domain.js:280:14)
    at REPLServer.runBound [as eval] (domain.js:293:12)
    at REPLServer.<anonymous> (repl.js:441:10)
    at emitOne (events.js:101:20)
    at REPLServer.emit (events.js:188:7)
    at REPLServer.Interface._onLine (readline.js:219:10)
    at REPLServer.Interface._line (readline.js:561:8)
> JSON.parse('null')
null
vlasky commented 8 years ago

I confirm that my proposed fix solved the problem.

Our use case was:

  1. Create a table that includes a JSON field that allows NULL, e.g. a table with an INT field and a JSON field.
  2. Insert a new row into that table without assigning the value of the JSON field, e.g. just the INT value

That is sufficient to cause the RangeError.

numtel commented 8 years ago

That scenario does not cause an error. This type test passes:

defineTypeTest('int_and_json', [
  'INT SIGNED NULL',
  'JSON NULL'
], [
  [ 5, null ],
  [ null, '\'null\'' ],
  [ 7, '\'null\'' ],
], '5.7.8');