geddy / model

Datastore-agnostic ORM in JavaScript
265 stars 55 forks source link

Null character in text body crashes PG adapter #245

Open danfinlay opened 9 years ago

danfinlay commented 9 years ago

There's a character I'm getting as user input (have captured as a test), and it seems illegal to assign as a value in a text field with the Postgres adapter.

In VIM it appears as ^@, when I call toString() on the object, it displays as \\u0000\\n. This seems to be the null character.

I thought the adapter was escaping text in a way that this wouldn't happen. If this can crash Model's connection to PG, how I should be sanitizing my input differently to prevent this or other characters in the future from doing this?

Testing failing comment body
'{"type":"Comment","createdAt":"2014-12-23T01:06:31.112Z","body":"\\u0000\\n"}'
Caught exception: AssertionError: {"name":"error","length":74,"severity":"ERROR","code":"08P01","detail":"undefined","hint":"undefined","position":"undefined","in == null
{ [AssertionError: {"name":"error","length":74,"severity":"ERROR","code":"08P01","detail":"undefined","hint":"undefined","position":"undefined","in == null]
  name: 'AssertionError',
  actual: 
   { [error: invalid message format]
     name: 'error',
     length: 74,
     severity: 'ERROR',
     code: '08P01',
     detail: undefined,
     hint: undefined,
     position: undefined,
     internalPosition: undefined,
     internalQuery: undefined,
     where: undefined,
     file: 'pqformat.c',
     line: '652',
     routine: 'pq_getmsgend' },
  expected: null,
  operator: '==',
  message: '{"name":"error","length":74,"severity":"ERROR","code":"08P01","detail":"undefined","hint":"undefined","position":"undefined","in == null' }
jake aborted.
mde commented 9 years ago

There is SQL escaping, but it's pretty minimal: https://github.com/geddy/model/blob/master/lib/datatypes.js#L62 That could use a lot of improvement, but could you at least add the null character there?

mde commented 9 years ago

@flyswatter, I've invited you to the org, so you can just make the change and push it. If you need it in a release, make the change in the release branch, and merge back to master. I can push a version to NPM whenever you'd like.

danfinlay commented 9 years ago

Oh wow, honored. Taking my shot at this!

I have had some trouble running the model tests. I'm getting:

ReferenceError: publishTask is not defined
    at Object.<anonymous> (/Users/danielfinlay/Documents/Development/model/Jakefile:24:1)
    at Module._compile (module.js:456:26)
(See full trace by running task with --trace)

This is right in the Jakefile, so am I missing something? I've npm i'd, I figure you're the guy to ask Jake questions.

mde commented 9 years ago

You're probably running an older (global) version of Jake. You can try updating your global Jake, or running it with the one installed locally (./node_modules/jake/bin/cli.js).

danfinlay commented 9 years ago

I still mean to get to this, but in the meanwhile, including another note:

Backslash characters are also not being escaped for postgres correctly.