mysqljs / sqlstring

Simple SQL escape and format for MySQL
MIT License
403 stars 78 forks source link

format() returns object string instead of value #40

Closed dan-willett closed 5 years ago

dan-willett commented 5 years ago

When using the format function to prepare a statement, if the data is an object, it is returning [object object] instead of stringifying the object.

for example:

const sqlStr = require(sqlstring);

let query = "INSERT INTO table (cola, colb, colc) VALUES ?";
let data = [["text",0,{test1: "text", val1: 1 }]];
let pquery = sqlStr.format(query,data);

Here is what sqlStr.format returns: "INSERT INTO table (cola, colb, colc) VALUES ('text',0,'[object Object]')"

Here is what it SHOULD return:

"INSERT INTO table (cola, colb, colc) VALUES ('text',0,'{"test1":"text","val1":1}')

dougwilson commented 5 years ago

Yes, that is correct. This is the default behavior for objects passed in as a secondary value. The toString() method is called and that returned value is escaped into a sql string.

value is an object, toString() is called on it and the returned value is used.

If you want that particular object to become a JSON formatted string (there are many other ways to represent objects into a column, so this library does not have any idea which is the appropriate one for your use case), you can three options:

  1. Stringy the value prior to passing into .format() . Many times this is the most straight forward method.
  2. Set the toString method on your object to perform a stringification that is different from the default javascript method. The value from this will then be properly escaped into sql.
  3. Set the toSqlString method on your obje t to perform a to sql conversion. The expected result from this is raw sql, so for your given use case (2) would be easier.

I hope this helps!

dan-willett commented 5 years ago

I can't think of a single instance where " [ object Object ] " would be a useful value when preparing an SQL statement.

Although this did give me an idea for an alternative workaround:

Object.prototype.toString = function() {
    if (this === null) return "null";
    else return JSON.stringify(this);
}

While the above appears to work fine in testing in a browser, the above solution will not work in a NodeJS production environment do to the circular reference, and while the circular reference can be solved with a package such as circular-json or flatted, this will cause problems with other packages that rely on the default behavior of Object.prototype.toString.

I propose the following change to SqlString.escape() which will fix the problem without breaking any current functionality:

else if (stringifyObjects) {
    return escapeString(val.toString());
}

would become:

else if (stringifyObjects) {
   let returnval = val.toString();
   if (returnval === "[object Object]") returnval = JSON.stringify(val);
    return escapeString(returnval);
}