Skip to content
This repository has been archived by the owner on Jun 28, 2021. It is now read-only.

Unsafe call to JSON.parse in CsvError class #262

Closed
loriamichele opened this issue Oct 18, 2019 · 2 comments
Closed

Unsafe call to JSON.parse in CsvError class #262

loriamichele opened this issue Oct 18, 2019 · 2 comments

Comments

@loriamichele
Copy link

Hi there 馃憢
First, thank you guys for the awesome work on this library!

I might have found a bug in the implementation of the CsvError class: if a malformed CSV file is being parsed, and by malformed I mean one of the rows have less values than expected, a proper error is thrown from the __onRow internal handler, but when the CsvError constructor is reached, an unsafe call to JSON.parse is made.

Here's the stack trace of that error with the latest version (4.6.3) of the library:

SyntaxError: Unexpected token u in JSON at position 0
        at JSON.parse (<anonymous>)

      at new CsvError (/*****/node_modules/csv-parse/lib/index.js:960:26)
      at Parser.__onRow (/*****/node_modules/csv-parse/lib/index.js:636:15)
      at Parser.__parse (/*****/node_modules/csv-parse/lib/index.js:527:40)
      at Parser._transform (/*****/node_modules/csv-parse/lib/index.js:341:22)

This error comes from an anonymous function and is not catchable by the stream error event handler because it's not emitted at all, which makes it impossible to handle it properly.

Just for reference, here's the stack trace for the expected error (got by replacing the JSON.parse(JSON.stringify(value)); with JSON.parse(JSON.stringify(value || ''));):

Error: Invalid Record Length: header length is 24, got 23 on line 3
      at Parser.__onRow (/*****/node_modules/csv-parse/lib/index.js:636:15)
      at Parser.__parse (/*****/node_modules/csv-parse/lib/index.js:527:40)
      at Parser._transform (/*****/node_modules/csv-parse/lib/index.js:341:22)
      at Parser.Transform._read (_stream_transform.js:190:10)
      at Parser.Transform._write (_stream_transform.js:178:12)
      at doWrite (_stream_writable.js:415:12)
      at writeOrBuffer (_stream_writable.js:399:5)
      at Parser.Writable.write (_stream_writable.js:299:11)
      at ReadStream.ondata (_stream_readable.js:709:20)
      at ReadStream.emit (events.js:198:13)

If you are fine with the proposed quick fix, I can also open a pull request.
Otherwise, just let me know what you think and/or how can this be fixed in a better way.

Cheers!

@wdavidw
Copy link
Member

wdavidw commented Oct 19, 2019

Could you write us a simple example reproducing the issue?

@loriamichele
Copy link
Author

Right, sorry, I completely forgot to provide a way to reproduce this 馃槄

Basically, you need to have a CSV file that looks like this:

col_a,col_b,col_c
foo,bar
foo,bar,baz

If you use the parser without any options, you get the expected behavior.

However, the bug comes in only if you happen to use the feature from the columns configuration option that allows you to skip entire columns in the parsed objects by passing an array and setting a certain value to undefined, null or false.

So, here's the code to reproduce it (having input.csv in the same folder as the following NodeJS script, with both csv-parse and csv-stringify installed as NPM deps):

const {resolve} = require('path');
const {createReadStream} = require('fs');
const parse = require('csv-parse');
const stringify = require('csv-stringify');

const parser = parse({columns: ['a', 'b', null]});
const stringifier = stringify();

createReadStream(resolve(__dirname, './input.csv'))
  .pipe(parser)
  .pipe(stringifier)
  .pipe(process.stdout);

I also tried passing undefined or false as values of the columns array, but they all throw the same way.

Let me know if I can provide additional information or anything else. And as I mentioned before, I'll be glad to help solving it 鉁岋笍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants