changing query parameter name from sql to q

unify query validatrion
This commit is contained in:
Simon Martín 2018-05-22 15:42:57 +02:00
parent b9474e7fc3
commit e347985465
2 changed files with 29 additions and 28 deletions

View File

@ -34,6 +34,7 @@ CopyController.prototype.route = function (app) {
authorizationMiddleware(this.metadataBackend), authorizationMiddleware(this.metadataBackend),
connectionParamsMiddleware(this.userDatabaseService), connectionParamsMiddleware(this.userDatabaseService),
timeoutLimitsMiddleware(this.metadataBackend), timeoutLimitsMiddleware(this.metadataBackend),
validateCopyQuery(),
handleCopyFrom(), handleCopyFrom(),
responseCopyFrom(), responseCopyFrom(),
errorMiddleware() errorMiddleware()
@ -48,6 +49,7 @@ CopyController.prototype.route = function (app) {
authorizationMiddleware(this.metadataBackend), authorizationMiddleware(this.metadataBackend),
connectionParamsMiddleware(this.userDatabaseService), connectionParamsMiddleware(this.userDatabaseService),
timeoutLimitsMiddleware(this.metadataBackend), timeoutLimitsMiddleware(this.metadataBackend),
validateCopyQuery(),
handleCopyTo(this.statsClient), handleCopyTo(this.statsClient),
errorMiddleware() errorMiddleware()
]; ];
@ -59,22 +61,13 @@ CopyController.prototype.route = function (app) {
function handleCopyTo (statsClient) { function handleCopyTo (statsClient) {
return function handleCopyToMiddleware (req, res, next) { return function handleCopyToMiddleware (req, res, next) {
const { sql } = req.query; const sql = req.query.q;
const filename = req.query.filename || 'carto-sql-copyto.dmp'; const filename = req.query.filename || 'carto-sql-copyto.dmp';
if (!sql) {
throw new Error("Parameter 'sql' is missing");
}
// Only accept SQL that starts with 'COPY'
if (!sql.toUpperCase().startsWith("COPY ")) {
throw new Error("SQL must start with COPY");
}
let metrics = { let metrics = {
size: 0, size: 0,
time: null, time: null,
format: getFormatFromCopyQuery(req.query.sql), format: getFormatFromCopyQuery(sql),
total_rows: null total_rows: null
}; };
@ -111,16 +104,7 @@ function handleCopyTo (statsClient) {
function handleCopyFrom () { function handleCopyFrom () {
return function handleCopyFromMiddleware (req, res, next) { return function handleCopyFromMiddleware (req, res, next) {
const { sql } = req.query; const sql = req.query.q;
if (!sql) {
return next(new Error("Parameter 'sql' is missing, must be in URL or first field in POST"));
}
// Only accept SQL that starts with 'COPY'
if (!sql.toUpperCase().startsWith("COPY ")) {
return next(new Error("SQL must start with COPY"));
}
res.locals.copyFromSize = 0; res.locals.copyFromSize = 0;
@ -174,7 +158,7 @@ function responseCopyFrom () {
if (req.profiler) { if (req.profiler) {
const copyFromMetrics = { const copyFromMetrics = {
size: res.locals.copyFromSize, //bytes size: res.locals.copyFromSize, //bytes
format: getFormatFromCopyQuery(req.query.sql), format: getFormatFromCopyQuery(req.query.q),
time: res.body.time, //seconds time: res.body.time, //seconds
total_rows: res.body.total_rows, total_rows: res.body.total_rows,
gzip: req.get('content-encoding') === 'gzip' gzip: req.get('content-encoding') === 'gzip'
@ -188,4 +172,21 @@ function responseCopyFrom () {
}; };
} }
function validateCopyQuery () {
return function validateCopyQueryMiddleware (req, res, next) {
const sql = req.query.q;
if (!sql) {
next(new Error("SQL is missing"));
}
// Only accept SQL that starts with 'COPY'
if (!sql.toUpperCase().startsWith("COPY ")) {
next(new Error("SQL must start with COPY"));
}
next();
};
}
module.exports = CopyController; module.exports = CopyController;

View File

@ -21,7 +21,7 @@ describe('copy-endpoints', function() {
it('should work with copyfrom endpoint', function(done){ it('should work with copyfrom endpoint', function(done){
assert.response(server, { assert.response(server, {
url: "/api/v1/sql/copyfrom?" + querystring.stringify({ url: "/api/v1/sql/copyfrom?" + querystring.stringify({
sql: "COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)" q: "COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)"
}), }),
data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'),
headers: {host: 'vizzuality.cartodb.com'}, headers: {host: 'vizzuality.cartodb.com'},
@ -49,7 +49,7 @@ describe('copy-endpoints', function() {
it('should fail with copyfrom endpoint and unexisting table', function(done){ it('should fail with copyfrom endpoint and unexisting table', function(done){
assert.response(server, { assert.response(server, {
url: "/api/v1/sql/copyfrom?" + querystring.stringify({ url: "/api/v1/sql/copyfrom?" + querystring.stringify({
sql: "COPY unexisting_table (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)" q: "COPY unexisting_table (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)"
}), }),
data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'), data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv'),
headers: {host: 'vizzuality.cartodb.com'}, headers: {host: 'vizzuality.cartodb.com'},
@ -69,7 +69,7 @@ describe('copy-endpoints', function() {
it('should fail with copyfrom endpoint and without csv', function(done){ it('should fail with copyfrom endpoint and without csv', function(done){
assert.response(server, { assert.response(server, {
url: "/api/v1/sql/copyfrom?" + querystring.stringify({ url: "/api/v1/sql/copyfrom?" + querystring.stringify({
sql: "COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)" q: "COPY copy_endpoints_test (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)"
}), }),
headers: {host: 'vizzuality.cartodb.com'}, headers: {host: 'vizzuality.cartodb.com'},
method: 'POST' method: 'POST'
@ -96,7 +96,7 @@ describe('copy-endpoints', function() {
assert.deepEqual( assert.deepEqual(
JSON.parse(res.body), JSON.parse(res.body),
{ {
error:["Parameter 'sql' is missing, must be in URL or first field in POST"] error:["SQL is missing"]
} }
); );
done(); done();
@ -106,7 +106,7 @@ describe('copy-endpoints', function() {
it('should work with copyto endpoint', function(done){ it('should work with copyto endpoint', function(done){
assert.response(server, { assert.response(server, {
url: "/api/v1/sql/copyto?" + querystring.stringify({ url: "/api/v1/sql/copyto?" + querystring.stringify({
sql: 'COPY copy_endpoints_test TO STDOUT', q: 'COPY copy_endpoints_test TO STDOUT',
filename: '/tmp/output.dmp' filename: '/tmp/output.dmp'
}), }),
headers: {host: 'vizzuality.cartodb.com'}, headers: {host: 'vizzuality.cartodb.com'},
@ -128,7 +128,7 @@ describe('copy-endpoints', function() {
it('should work with copyfrom and gzip', function(done){ it('should work with copyfrom and gzip', function(done){
assert.response(server, { assert.response(server, {
url: "/api/v1/sql/copyfrom?" + querystring.stringify({ url: "/api/v1/sql/copyfrom?" + querystring.stringify({
sql: "COPY copy_endpoints_test2 (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)" q: "COPY copy_endpoints_test2 (id, name) FROM STDIN WITH (FORMAT CSV, DELIMITER ',', HEADER true)"
}), }),
data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv.gz'), data: fs.createReadStream(__dirname + '/../support/csv/copy_test_table.csv.gz'),
headers: { headers: {