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.

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

Parameterized Queries #37

Open
brettlaforge opened this issue Jul 31, 2014 · 6 comments
Open

Parameterized Queries #37

brettlaforge opened this issue Jul 31, 2014 · 6 comments

Comments

@brettlaforge
Copy link

It would be nice if the client.query method could take sql and a list of parameters into order to avoid sql injection. It would be super nice to be able to use named parameters.

Here is how Postgres does it:
https://github.com/brianc/node-postgres/wiki/Client#method-query-parameterized

Also parsing Javascript Date objects into Vertica date strings would be super super nice.

@wvanbergen
Copy link
Owner

I tried to implement this on Vertica 5.x and 6.x, but as it turns out in only supports that protocol if there's 0 parameters in the query. Not very useful.

They may have fixed this in Vertica 7, IIRC the changelog mentioned something along those lines. I won't have time to implement this any time soon though. Can anybody confirm that Vertica supports this now?

@jmaitrehenry
Copy link

Yes Vertica support it now, I'm using it with odbc.

@wvanbergen
Copy link
Owner

@jmaitrehenry Did you confirm parameters are bound on the Vertica server, instead of in the ODBC driver on the client-side? AFAIK server-side binding is still not supported by Vertica 7.0. :(

@brettlaforge
Copy link
Author

We ended up building a library that binds parameters to SQL templates in javascript :(
Obviously we understand the risks of doing this ourselves and have made every attempt to avoid SQL injection and have proper escaping. I'd be willing to share the code that we use if you are interested in adding it to this library.

I can see why you wouldn't want too as well.

@wvanbergen
Copy link
Owner

Yeah, this can easily live on the client side - the tricky part is doing the parsing of the query and substitution of placeholders; the quoting/escaping is already there.

It's unlikely that I will work on this myself. I don't see too much value in doing this on the client. It doesn't have any of the performance improvements that a server-side implementation comes with, and it's quite easy to do this yourself, e.g.: sql = util.format("SELECT * FROM table WHERE uuid = %s", Vertica.quote(uuid))

Having said that; if you have an implementation ready that can be merged it, I will gladly accept it.

@mrbbdx
Copy link

mrbbdx commented Jun 12, 2015

Forgive me for not forking and providing a pull request, but if anyone needs a helper, here's a decent start:

var util = require( 'util' );
var vertica = require( 'vertica' );

/**
 * Create a safe Vertica query from parameters. Inspired by: https://github.com/wvanbergen/node-vertica/issues/37 (at the very end)
 * @param {string} query A query that will be formatted with https://nodejs.org/api/util.html#util_util_format_format
 * @param {...*} param One or more arguments passed to this function that will be used to format the returned query.
 * @returns {string}
 */
module.exports.parameterizeQuery = function( query, param ) {

    // Vertica-quote the arguments
    var quotedParameters = [].slice.call( arguments ).map( function( value, i ) {

        // But don't quote the first argument (the query string)
        return ( i !== 0 ) ? vertica.quote( value ) : value;
    });

    // Make our sanitized query
    return util.format.apply( this, quotedParameters );
};

So now you can use with something like:

var insertQuery = "INSERT INTO stats ( username, count, other ) VALUES ( %s, %s, %s )";
var parameterizedQuery = myVerticaHelpers.parameterizeQuery( insertQuery, req.query.username, req.query.count, req.query.other );
myVerticaConnection.query( parameterizedQuery, verticaResultHandler );

@brettlaforge @wvanbergen

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

No branches or pull requests

4 participants