From d23416cc6064570cd4996e8a41d42fc4dd24bcdc Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 15 Oct 2012 13:20:37 +0200 Subject: [PATCH] Set X-Cache-Channel to NONE when the SQL may write to the database Note that "may write" allows for false positive, so there could be less cache hits than possibly allowable. If this will be a problem for any real use case we could still improve the regular expression used to detect "writing" queries. Automated tests are added to check for the X-Cache-Channel header with both writing and read-only queries performed by authenticated requests. Closes #27 Closes #43 --- app/controllers/app.js | 36 +++++++++-- test/acceptance/app.test.js | 126 +++++++++++++++++++++++++++++++++++- 2 files changed, 154 insertions(+), 8 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index b1faa5a2..bc7f6752 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -38,6 +38,21 @@ app.all('/api/v1/sql', function(req, res) { handleQuery(req, res) } ); app.all('/api/v1/sql.:f', function(req, res) { handleQuery(req, res) } ); app.get('/api/v1/cachestatus', function(req, res) { handleCacheStatus(req, res) } ); +// Return true of the given query may write to the database +// +// NOTE: this is a fuzzy check, the return could be true even +// if the query doesn't really write anything. +// But you can be pretty sure of a false return. +// +function queryMayWrite(sql) { + var mayWrite = false; + var pattern = RegExp("(insert|update|delete|create|drop)", "i"); + if ( pattern.test(sql) ) { + mayWrite = true; + } + return mayWrite; +} + // request handlers function handleQuery(req, res) { @@ -81,6 +96,8 @@ function handleQuery(req, res) { // placeholder for connection var pg; + var authenticated; + // 1. Get database from redis via the username stored in the host header subdomain // 2. Run the request through OAuth to get R/W user id if signed // 3. Get the list of tables affected by the query @@ -119,12 +136,14 @@ function handleQuery(req, res) { // store postgres connection pg = new PSQL(user_id, database, limit, offset); + authenticated = ! _.isNull(user_id); + // get all the tables from Cache or SQL if (!_.isNull(tableCache[sql_md5]) && !_.isUndefined(tableCache[sql_md5])){ tableCache[sql_md5].hits++; return true; - } else{ - pg.query("SELECT CDB_QueryTables($quotesql$" + sql + "$quotesql$)", this); + } else { + pg.query("SELECT CDB_QueryTables($quotesql$" + sql + "$quotesql$)", this); } }, function queryResult(err, result){ @@ -133,6 +152,7 @@ function handleQuery(req, res) { // store explain result in local Cache if (_.isUndefined(tableCache[sql_md5])){ tableCache[sql_md5] = result; + tableCache[sql_md5].may_write = queryMayWrite(sql); tableCache[sql_md5].hits = 1; //initialise hit counter } @@ -176,8 +196,8 @@ function handleQuery(req, res) { // set cache headers res.header('Last-Modified', new Date().toUTCString()); - res.header('Cache-Control', 'no-cache,max-age=3600,must-revalidate, public'); - res.header('X-Cache-Channel', generateCacheKey(database, tableCache[sql_md5])); + res.header('Cache-Control', 'no-cache,max-age=3600,must-revalidate,public'); + res.header('X-Cache-Channel', generateCacheKey(database, tableCache[sql_md5], authenticated)); return result; }, @@ -392,8 +412,12 @@ function setCrossDomain(res){ res.header("Access-Control-Allow-Headers", "X-Requested-With"); } -function generateCacheKey(database,tables){ - return database + ":" + tables.rows[0].cdb_querytables.split(/^\{(.*)\}$/)[1]; +function generateCacheKey(database,tables,is_authenticated){ + if ( is_authenticated && tables.may_write ) { + return "NONE"; + } else { + return database + ":" + tables.rows[0].cdb_querytables.split(/^\{(.*)\}$/)[1]; + } } function generateMD5(data){ diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 7b488d38..6bff0872 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -24,7 +24,7 @@ app.setMaxListeners(0); suite('app.test', function() { -var real_oauth_header = 'OAuth realm="http://vizzuality.testhost.lan/",oauth_consumer_key="fZeNGv5iYayvItgDYHUbot1Ukb5rVyX6QAg8GaY2",oauth_token="l0lPbtP68ao8NfStCiA3V3neqfM03JKhToxhUQTR",oauth_signature_method="HMAC-SHA1", oauth_signature="o4hx4hWP6KtLyFwggnYB4yPK8xI%3D",oauth_timestamp="1313581372",oauth_nonce="W0zUmvyC4eVL8cBd4YwlH1nnPTbxW0QBYcWkXTwe4",oauth_version="1.0"'; +var expected_cache_control = 'no-cache,max-age=3600,must-revalidate,public'; // use dec_sep for internationalization var checkDecimals = function(x, dec_sep){ @@ -54,11 +54,14 @@ test('GET /api/v1/sql with SQL parameter on SELECT only. No oAuth included ', fu method: 'GET' },{ }, function(res) { assert.equal(res.statusCode, 200, res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'cartodb_test_user_1_db:untitle_table_4'); + assert.equal(res.headers['cache-control'], expected_cache_control); done(); }); }); - test('GET /api/v1/sql with SQL parameter on SELECT only. no database param, just id using headers', function(done){ assert.response(app, { url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4', @@ -70,6 +73,22 @@ test('GET /api/v1/sql with SQL parameter on SELECT only. no database param, just }); }); +test('GET /api/v1/sql with SQL parameter on SELECT only. no database param, just id using headers. Authenticated.', +function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20cartodb_id*2%20FROM%20untitle_table_4&api_key=1234', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res) { + assert.equal(res.statusCode, 200, res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'cartodb_test_user_1_db:untitle_table_4'); + assert.equal(res.headers['cache-control'], expected_cache_control); + done(); + }); +}); + test('POST /api/v1/sql with SQL parameter on SELECT only. no database param, just id using headers', function(done){ assert.response(app, { @@ -129,6 +148,10 @@ test('INSERT returns affected rows', function(done){ assert.ok(out.hasOwnProperty('time')); assert.equal(out.total_rows, 2); assert.equal(out.rows.length, 0); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'NONE'); + assert.equal(res.headers['cache-control'], expected_cache_control); done(); }); }); @@ -151,6 +174,10 @@ test('UPDATE returns affected rows', function(done){ assert.ok(out.hasOwnProperty('time')); assert.equal(out.total_rows, 2); assert.equal(out.rows.length, 0); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'NONE'); + assert.equal(res.headers['cache-control'], expected_cache_control); done(); }); }); @@ -173,6 +200,10 @@ test('DELETE returns affected rows', function(done){ assert.ok(out.hasOwnProperty('time')); assert.equal(out.total_rows, 2); assert.equal(out.rows.length, 0); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'NONE'); + assert.equal(res.headers['cache-control'], expected_cache_control); done(); }); }); @@ -261,6 +292,97 @@ test('GET /api/v1/sql with SQL parameter on DROP DATABASE only.header based db - }); }); +test('CREATE TABLE with GET and auth', function(done){ + assert.response(app, { + url: "/api/v1/sql?" + querystring.stringify({ + q: 'CREATE TABLE create_table_test(a int)', + api_key: 1234 + }), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{}, function(res) { + assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'NONE'); + assert.equal(res.headers['cache-control'], expected_cache_control); + done(); + }); +}); + +// TODO: test COPY +//test('COPY TABLE with GET and auth', function(done){ +// assert.response(app, { +// url: "/api/v1/sql?" + querystring.stringify({ +// q: 'COPY TABLE create_table_test FROM stdin; 1\n\\.\n', +// api_key: 1234 +// }), +// headers: {host: 'vizzuality.cartodb.com'}, +// method: 'GET' +// },{}, function(res) { +// assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); +// // Check cache headers +// // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 +// assert.equal(res.headers['x-cache-channel'], 'NONE'); +// assert.equal(res.headers['cache-control'], expected_cache_control); +// done(); +// }); +//}); + +test('DROP TABLE with GET and auth', function(done){ + assert.response(app, { + url: "/api/v1/sql?" + querystring.stringify({ + q: 'DROP TABLE create_table_test', + api_key: 1234 + }), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{}, function(res) { + assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'NONE'); + assert.equal(res.headers['cache-control'], expected_cache_control); + done(); + }); +}); + +test('CREATE FUNCTION with GET and auth', function(done){ + assert.response(app, { + url: "/api/v1/sql?" + querystring.stringify({ + q: 'CREATE FUNCTION create_func_test(a int) RETURNS INT AS \'SELECT 1\' LANGUAGE \'sql\'', + api_key: 1234 + }), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{}, function(res) { + assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'NONE'); + assert.equal(res.headers['cache-control'], expected_cache_control); + done(); + }); +}); + +test('DROP FUNCTION with GET and auth', function(done){ + assert.response(app, { + url: "/api/v1/sql?" + querystring.stringify({ + q: 'DROP FUNCTION create_func_test(a int)', + api_key: 1234 + }), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{}, function(res) { + assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'NONE'); + assert.equal(res.headers['cache-control'], expected_cache_control); + done(); + }); +}); + test('GET /api/v1/sql with SQL parameter and geojson format, ensuring content-disposition set to geojson', function(done){ assert.response(app, { url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&format=geojson',