Fix #161 - Wrong queries causes unhandled exceptions

loopbackio · Nov 20, 2019 · 4189787 · 4189787
1 parent deb461e
commit 4189787
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 1 deletion.
diff --git a/lib/sql.js b/lib/sql.js
@@ -1207,15 +1207,25 @@ SQLConnector.prototype.buildOrderBy = function(model, order) {
   if (typeof order === 'string') {
     order = [order];
   }
+  const props = self.getModelDefinition(model).properties;
   const clauses = [];
   for (let i = 0, n = order.length; i < n; i++) {
     const t = order[i].split(/[\s,]+/);
+    const key = t[0];
+    if (!props[key]) {
+      // Unknown property, ignore it
+      debug('Unknown property %s is skipped for model %s', key, model);
+      continue;
+    }
     if (t.length === 1) {
       clauses.push(self.columnEscaped(model, order[i]));
     } else {
       clauses.push(self.columnEscaped(model, t[0]) + ' ' + t[1]);
     }
   }
+  if (!clauses.length) {
+    return '';
+  }
   return 'ORDER BY ' + clauses.join(',');
 };
 
@@ -1456,7 +1466,12 @@ SQLConnector.prototype.all = function find(model, filter, options, cb) {
   const self = this;
   // Order by id if no order is specified
   filter = filter || {};
-  const stmt = this.buildSelect(model, filter, options);
+  let stmt;
+  try {
+    stmt = this.buildSelect(model, filter, options);
+  } catch (err) {
+    return cb(err, []);
+  }
   this.execute(stmt.sql, stmt.params, options, function(err, data) {
     if (err) {
       return cb(err, []);

diff --git a/test/sql.test.js b/test/sql.test.js
@@ -270,6 +270,21 @@ describe('sql connector', function() {
     expect(orderBy).to.eql('ORDER BY `NAME` ASC,`VIP` DESC');
   });
 
+  it('builds order by with non existent field filtered out', function() {
+    const orderBy = connector.buildOrderBy('customer', ['nam?e', 'name']);
+    expect(orderBy).to.eql('ORDER BY `NAME`');
+  });
+
+  it('builds order by with non existent field with direction filtered out', function() {
+    const orderBy = connector.buildOrderBy('customer', ['nam?e ASC', 'name']);
+    expect(orderBy).to.eql('ORDER BY `NAME`');
+  });
+
+  it('builds order by with only non existent fields', function() {
+    const orderBy = connector.buildOrderBy('customer', ['nam?e', 'n?ame', '?name DESC']);
+    expect(orderBy).to.eql('');
+  });
+
   it('builds fields for columns', function() {
     const fields = connector.buildFields('customer',
       {name: 'John', vip: true, unknown: 'Random'});
@@ -503,4 +518,8 @@ describe('sql connector', function() {
     expect(function() { runExecute(); }).to.not.throw();
     ds.connected = true;
   });
+
+  it('should not throw if invalid sql statement is created by all', function(done) {
+    connector.all('customer', {order: 'n?ame'}, {}, done);
+  });
 });