Merge pull request #3093 from getredash/backend-parameter-templating

Remove Mustache templating from frontend

client/app/services/query-result.js

@@ -532,10 +532,16 @@ function QueryResultService($resource, $timeout, $q, QueryResultError) {
       return `${queryName.replace(/ /g, '_') + moment(this.getUpdatedAt()).format('_YYYY_MM_DD')}.${fileType}`;
     }
 
-    static get(dataSourceId, query, maxAge, queryId) {
+    static get(dataSourceId, query, parameters, maxAge, queryId) {
       const queryResult = new QueryResult();
 
-      const params = { data_source_id: dataSourceId, query, max_age: maxAge };
+      const params = {
+        data_source_id: dataSourceId,
+        parameters,
+        query,
+        max_age: maxAge,
+      };
+
       if (queryId !== undefined) {
         params.query_id = queryId;
       }

client/app/services/query.js

@@ -416,7 +416,7 @@ function QueryResource(
     if (!this.query) {
       return new QueryResultError("Can't execute empty query.");
     }
-    let queryText = this.query;
+    const queryText = this.query;
 
     const parameters = this.getParameters();
     const missingParams = parameters.getMissing();
@@ -438,8 +438,6 @@ function QueryResource(
     }
 
     if (parameters.isRequired()) {
-      queryText = Mustache.render(queryText, parameters.getValues());
-
       // Need to clear latest results, to make sure we don't use results for different params.
       this.latest_query_data = null;
       this.latest_query_data_id = null;
@@ -456,7 +454,7 @@ function QueryResource(
         this.queryResult = QueryResult.getById(this.latest_query_data_id);
       }
     } else if (this.data_source_id) {
-      this.queryResult = QueryResult.get(this.data_source_id, queryText, maxAge, this.id);
+      this.queryResult = QueryResult.get(this.data_source_id, queryText, parameters.getValues(), maxAge, this.id);
     } else {
       return new QueryResultError('Please select data source to run this query.');
     }

redash/handlers/query_results.py

@@ -5,22 +5,42 @@ from flask import make_response, request
 from flask_login import current_user
 from flask_restful import abort
 from redash import models, settings
-from redash.tasks import QueryTask
-from redash.permissions import require_permission, not_view_only, has_access, require_access, view_only
 from redash.handlers.base import BaseResource, get_object_or_404
-from redash.utils import (collect_query_parameters,
-                          collect_parameters_from_request,
-                          gen_query_hash,
-                          json_dumps,
-                          utcnow,
-                          mustache_render)
+from redash.permissions import (has_access, not_view_only, require_access,
+                                require_permission, view_only)
+from redash.tasks import QueryTask, record_event
 from redash.tasks.queries import enqueue_query
+from redash.utils import (collect_parameters_from_request,
+                          collect_query_parameters, gen_query_hash, json_dumps,
+                          utcnow)
+from redash.utils.sql_query import SQLInjectionError, SQLQuery
 
 
 def error_response(message):
     return {'job': {'status': 4, 'error': message}}, 400
 
 
+def apply_parameters(template, parameters, data_source):
+    query = SQLQuery(template).apply(parameters)
+
+    # for now we only log `SQLInjectionError` to detect false positives
+    try:
+        text = query.text
+    except SQLInjectionError:
+        record_event({
+            'action': 'sql_injection',
+            'object_type': 'query',
+            'query': template,
+            'parameters': parameters,
+            'timestamp': time.time(),
+            'org_id': data_source.org_id
+        })
+    finally:
+        text = query.query
+
+    return text
+
+
 #
 # Run a parameterized query synchronously and return the result
 # DISCLAIMER: Temporary solution to support parameters in queries. Should be
@@ -33,8 +53,7 @@ def run_query_sync(data_source, parameter_values, query_text, max_age=0):
     if missing_params:
         raise Exception('Missing parameter value for: {}'.format(", ".join(missing_params)))
 
-    if query_parameters:
-        query_text = mustache_render(query_text, parameter_values)
+    query_text = apply_parameters(query_text, parameter_values, data_source)
 
     if max_age <= 0:
         query_result = None
@@ -84,8 +103,7 @@ def run_query(data_source, parameter_values, query_text, query_id, max_age=0):
 
         return error_response(message)
 
-    if query_parameters:
-        query_text = mustache_render(query_text, parameter_values)
+    query_text = apply_parameters(query_text, parameter_values, data_source)
 
     if max_age == 0:
         query_result = None
@@ -112,13 +130,14 @@ class QueryResultListResource(BaseResource):
                                 any cached result, or executes if not available. Set to zero to
                                 always execute.
         :qparam number data_source_id: ID of data source to query
+        :qparam object parameters: A set of parameter values to apply to the query.
         """
         params = request.get_json(force=True)
-        parameter_values = collect_parameters_from_request(request.args)
 
         query = params['query']
         max_age = int(params.get('max_age', -1))
         query_id = params.get('query_id', 'adhoc')
+        parameters = params.get('parameters', collect_parameters_from_request(request.args))
 
         data_source = models.DataSource.get_by_id_and_org(params.get('data_source_id'), self.current_org)
 
@@ -129,9 +148,11 @@ class QueryResultListResource(BaseResource):
             'action': 'execute_query',
             'object_id': data_source.id,
             'object_type': 'data_source',
-            'query': query
+            'query': query,
+            'query_id': query_id,
+            'parameters': parameters
         })
-        return run_query(data_source, parameter_values, query, query_id, max_age)
+        return run_query(data_source, parameters, query, query_id, max_age)
 
 
 ONE_YEAR = 60 * 60 * 24 * 365.25

redash/utils/__init__.py

@@ -1,6 +1,6 @@
+import codecs
 import cStringIO
 import csv
-import codecs
 import datetime
 import decimal
 import hashlib
@@ -10,15 +10,16 @@ import re
 import uuid
 import binascii
 
+from six import string_types
+
 import pystache
 import pytz
 import simplejson
 from funcy import distinct, select_values
-from six import string_types
+from redash import settings
 from sqlalchemy.orm.query import Query
 
 from .human_time import parse_human_time
-from redash import settings
 
 COMMENTS_REGEX = re.compile("/\*.*?\*/")
 WRITER_ENCODING = os.environ.get('REDASH_CSV_WRITER_ENCODING', 'utf-8')

tests/handlers/test_query_results.py

@@ -76,6 +76,15 @@ class TestQueryResultListAPI(BaseTestCase):
         self.assertEquals(rv.status_code, 400)
         self.assertIn('job', rv.json)
 
+        rv = self.make_request('post', '/api/query_results',
+                               data={'data_source_id': self.factory.data_source.id,
+                                     'query': query,
+                                     'parameters': {'param': 1},
+                                     'max_age': 0})
+
+        self.assertEquals(rv.status_code, 200)
+        self.assertIn('job', rv.json)
+
         rv = self.make_request('post', '/api/query_results?p_param=1',
                                data={'data_source_id': self.factory.data_source.id,
                                      'query': query,