Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Null character in text body crashes PG adapter #245

Open
danfinlay opened this issue Dec 23, 2014 · 5 comments · May be fixed by #250
Open

Null character in text body crashes PG adapter #245

danfinlay opened this issue Dec 23, 2014 · 5 comments · May be fixed by #250

Comments

@danfinlay
Copy link
Contributor

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
Copy link
Contributor

mde commented Jan 2, 2015

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
Copy link
Contributor

mde commented Jan 2, 2015

@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
Copy link
Contributor Author

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
Copy link
Contributor

mde commented Jan 2, 2015

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
Copy link
Contributor Author

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

Backslash characters are also not being escaped for postgres correctly.

danfinlay pushed a commit to danfinlay/model that referenced this issue Feb 6, 2015
@danfinlay
Added null escaping
47c61d3 
Added failing null character escaping test
Fixes geddy#245
@danfinlay danfinlay linked a pull request Feb 6, 2015 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Added null character escaping to Postgres adapter danfinlay/model
2 participants
@mde @danfinlay