Release 1.3.5

Requires windshaft 0.13.7
Jira ref CDB-414

.travis.yml

@@ -0,0 +1,14 @@
+before_install:
+  - sudo apt-add-repository --yes ppa:mapnik/v2.1.0
+  - sudo apt-add-repository --yes ppa:ubuntugis/ppa
+  - sudo apt-get update -q
+  - sudo apt-get install -q libmapnik libmapnik-dev postgresql-9.1-postgis libsigc++-dev
+  - createdb template_postgis
+  - psql -c "CREATE EXTENSION postgis" template_postgis
+
+env:
+  - NPROCS=1 JOBS=1
+
+language: node_js
+node_js:
+  - "0.8"

NEWS.md

@@ -1,3 +1,13 @@
+1.3.5 -- 2013-10-03
+-------------------
+
+* Fixing apostrophes in CartoCSS
+* Fix "sql/table must contain zoom variable" error when using
+  "[ zoom > 3]" CartoCSS snippets (note the space)
+* Fix backward compatibility handling of sqlapi.host configuration (#82)
+* Fix error for invalid text-name in CartoCSS (#81)
+* Do not let anonymous requests use authorized renderer caches 
+
 1.3.4
 ------
 

README.md

@@ -1,7 +1,7 @@
 Windshaft-CartoDB
 ==================
 
-NOTE: requires node-0.8.x
+[![Build Status](https://travis-ci.org/CartoDB/Windshaft-cartodb.png)](http://travis-ci.org/CartoDB/Windshaft-cartodb)
 
 This is the CartoDB map tiler. It extends Windshaft with some extra
 functionality and custom filters for authentication
@@ -18,7 +18,7 @@ Requirements
 ------------
 
  [core]
- - node-0.6.x+
+ - node-0.8.x+
  - PostgreSQL-8.3+
  - PostGIS-1.5.0+
  - Redis 2.2.0+ (http://www.redis.io)

lib/cartodb/carto_data.js

@@ -143,7 +143,7 @@ module.exports = function() {
                     that.getId(req, function(err, user_id) {
                         if (err) throw err;
                         var dbuser = _.template(global.settings.postgres_auth_user, {user_id: user_id});
-                        _.extend(req, {dbuser:dbuser});
+                        _.extend(req.params, {dbuser:dbuser});
                         callback(err, true); 
                     });
                 } else {

lib/cartodb/cartodb_windshaft.js

@@ -17,7 +17,7 @@ var CartodbWindshaft = function(serverOptions) {
 
     serverOptions.beforeStateChange = function(req, callback) {
         var err = null;
-        if ( ! req.hasOwnProperty('dbuser') ) {
+        if ( ! req.params.hasOwnProperty('dbuser') ) {
           err = new Error("map state cannot be changed by unauthenticated request!");
         }
         callback(err, req);

lib/cartodb/server_options.js

@@ -9,7 +9,9 @@ var   _          = require('underscore')
   ;
 
 // This is for backward compatibility with 1.3.3
-if ( ! global.environment.sqlapi.domain ) global.environment.sqlapi.host;
+if ( _.isUndefined(global.environment.sqlapi.domain) ) {
+  global.environment.sqlapi.domain = global.environment.sqlapi.host;
+}
 
 module.exports = function(){
 
@@ -84,7 +86,7 @@ module.exports = function(){
               callback(new Error(msg));
               console.log('unexpected response status (' + res.statusCode + ') for sql query: ' + sql + ': ' + msg);
               return;
-          } 
+          }
           callback(null, body.rows);
       });
     };
@@ -162,7 +164,7 @@ module.exports = function(){
       if ( me.channelCache.hasOwnProperty(cacheKey) ) {
         callback(null, me.channelCache[cacheKey]);
         return;
-      } 
+      }
       else if ( req.params.token ) {
         // cached cache channel for token-based access should be constructed
         // at cache creation time
@@ -233,7 +235,7 @@ module.exports = function(){
         me.generateCacheChannel(req, function(err, channel){
             if ( ! err ) {
               res.header('X-Cache-Channel', channel);
-              cb(null, channel); 
+              cb(null, channel);
             } else {
               console.log('ERROR generating cache channel: ' + ( err.message ? err.message : err ));
               // TODO: evaluate if we should bubble up the error instead
@@ -264,6 +266,7 @@ module.exports = function(){
         // take place before proceeding. Error will be logged
         // asyncronously
         cartoData.incMapviewCount(username, mapconfig.stat_tag, function(err) {
+          if (req.profiler) req.profiler.done('incMapviewCount');
           if ( err ) console.log("ERROR: failed to increment mapview count for user '" + username + "': " + err);
           done();
         });
@@ -281,12 +284,14 @@ module.exports = function(){
         var cacheKey = dbName + ':' + token;
 
         me.affectedTables(usr, key, sql, function(err, tableNames) {
+            if (req.profiler) req.profiler.done('affectedTables');
 
             if ( err ) { done(err); return; }
             var cacheChannel = me.buildCacheChannel(dbName,tableNames);
             me.channelCache[cacheKey] = cacheChannel; // store for caching
             // find last updated
             me.findLastUpdated(usr, key, tableNames, function(err, lastUpdated) {
+              if (req.profiler) req.profiler.done('findLastUpdated');
               if ( err ) { done(err); return; }
               response.layergroupid = response.layergroupid + ':' + lastUpdated; // use epoch
               response.last_updated = new Date(lastUpdated).toISOString(); // TODO: use ISO format
@@ -322,7 +327,7 @@ module.exports = function(){
                 delete req.query.lzma
                 _.extend(req.query, JSON.parse(result))
                 me.req2params(req, callback);
-              } catch (err) { 
+              } catch (err) {
                 callback(new Error('Error parsing lzma as JSON: ' + err));
               }
             },
@@ -354,7 +359,7 @@ module.exports = function(){
         req.params.interactivity = req.params.interactivity || 'cartodb_id';
 
         req.params.processXML = function(req, xml, callback) {
-          var dbuser = req.dbuser ? req.dbuser : global.settings.postgres.user;
+          var dbuser = req.params.dbuser || global.settings.postgres.user;
           if ( ! me.rx_dbuser ) me.rx_dbuser = /(<Parameter name="user"><!\[CDATA\[)[^\]]*(]]><\/Parameter>)/g;
           xml = xml.replace(me.rx_dbuser, "$1" + dbuser + "$2");
           callback(null, xml);

npm-shrinkwrap.json

@@ -1,6 +1,6 @@
 {
   "name": "windshaft-cartodb",
-  "version": "1.3.4",
+  "version": "1.3.5",
   "dependencies": {
     "node-varnish": {
       "version": "0.1.1"
@@ -9,14 +9,14 @@
       "version": "1.3.3"
     },
     "windshaft": {
-      "version": "0.13.4",
+      "version": "0.13.7",
       "dependencies": {
         "grainstore": {
-          "version": "0.13.8",
+          "version": "0.13.11",
           "dependencies": {
             "carto": {
-              "version": "0.9.3-cdb3",
-              "from": "git://github.com/CartoDB/carto.git#cdb-0.9.3-cdb3",
+              "version": "0.9.3-cdb6",
+              "from": "git://github.com/CartoDB/carto.git#0.9.3-cdb6",
               "dependencies": {
                 "mapnik-reference": {
                   "version": "5.0.0-cdb1",
@@ -33,14 +33,13 @@
               }
             },
             "mapnik-reference": {
-              "version": "5.0.4"
+              "version": "5.0.6"
             },
             "millstone": {
-              "version": "0.6.0-cdb1",
-              "from": "git://github.com/CartoDB/millstone.git#cdb-0.6.0-cdb1",
+              "version": "0.6.5",
               "dependencies": {
                 "underscore": {
-                  "version": "1.5.1"
+                  "version": "1.5.2"
                 },
                 "request": {
                   "version": "2.26.0",
@@ -101,7 +100,7 @@
                       "version": "1.4.1"
                     },
                     "form-data": {
-                      "version": "0.1.1",
+                      "version": "0.1.2",
                       "dependencies": {
                         "combined-stream": {
                           "version": "0.0.4",
@@ -119,13 +118,56 @@
                   }
                 },
                 "srs": {
-                  "version": "0.3.2"
+                  "version": "0.3.3"
                 },
                 "zipfile": {
-                  "version": "0.4.0"
+                  "version": "0.4.1"
                 },
                 "sqlite3": {
-                  "version": "2.1.15"
+                  "version": "2.1.17",
+                  "dependencies": {
+                    "progress": {
+                      "version": "1.0.1"
+                    },
+                    "tar.gz": {
+                      "version": "0.1.1",
+                      "dependencies": {
+                        "fstream": {
+                          "version": "0.1.24",
+                          "dependencies": {
+                            "rimraf": {
+                              "version": "2.2.2"
+                            },
+                            "graceful-fs": {
+                              "version": "2.0.1"
+                            },
+                            "inherits": {
+                              "version": "2.0.1"
+                            }
+                          }
+                        },
+                        "tar": {
+                          "version": "0.1.18",
+                          "dependencies": {
+                            "inherits": {
+                              "version": "2.0.1"
+                            },
+                            "block-stream": {
+                              "version": "0.0.7"
+                            }
+                          }
+                        },
+                        "commander": {
+                          "version": "1.1.1",
+                          "dependencies": {
+                            "keypress": {
+                              "version": "0.1.0"
+                            }
+                          }
+                        }
+                      }
+                    }
+                  }
                 },
                 "mime": {
                   "version": "1.2.11"
@@ -140,7 +182,7 @@
                       "version": "0.0.2"
                     },
                     "minimist": {
-                      "version": "0.0.2"
+                      "version": "0.0.5"
                     }
                   }
                 }
@@ -191,7 +233,7 @@
           "from": "git://github.com/Vizzuality/tilelive-mapnik.git#5908346",
           "dependencies": {
             "eio": {
-              "version": "0.2.1"
+              "version": "0.2.2"
             },
             "mime": {
               "version": "1.2.11"

package.json

@@ -1,7 +1,7 @@
 {
     "private": true,
     "name": "windshaft-cartodb",
-    "version": "1.3.4",
+    "version": "1.3.5",
     "description": "A map tile server for CartoDB",
     "url": "https://github.com/CartoDB/Windshaft-cartodb",
     "licenses": [{
@@ -21,7 +21,7 @@
     "dependencies": {
         "node-varnish": "0.1.1",
         "underscore"  : "~1.3.3",
-        "windshaft"  : "~0.13.4",
+        "windshaft"  : "~0.13.7",
         "step": "0.0.x",
         "generic-pool": "~2.0.3",
         "redis": "~0.8.3",

run_tests.sh

@@ -1,9 +1,5 @@
 #!/bin/sh
 
-# Must match redis_port in config/environments/test.js
-# TODO: read from there
-REDIS_PORT=6333
-
 OPT_CREATE=yes # create the test environment
 OPT_DROP=yes   # drop the test environment
 
@@ -11,6 +7,9 @@ cd $(dirname $0)
 BASEDIR=$(pwd)
 cd -
 
+REDIS_PORT=`node -e "console.log(require('${BASEDIR}/config/environments/test.js').redis.port)"`
+export REDIS_PORT
+
 cleanup() {
   if test x"$OPT_DROP" = xyes; then
     if test x"$PID_REDIS" = x; then

test/acceptance/multilayer.js

@@ -468,19 +468,19 @@ suite('multilayer', function() {
            { options: {
                sql: 'select * from test_table_private_1 where cartodb_id=1',
                cartocss: '#layer { marker-fill:red; marker-width:32; marker-allow-overlap:true; }', 
-               cartocss_version: '2.1.1',
+               cartocss_version: '2.1.0',
                interactivity: 'cartodb_id'
              } },
            { options: {
                sql: 'select * from test_table_private_1 where cartodb_id=2',
                cartocss: '#layer { marker-fill:blue; marker-allow-overlap:true; }', 
-               cartocss_version: '2.1.1',
+               cartocss_version: '2.1.0',
                interactivity: 'cartodb_id'
              } }
         ]
       };
 
-      var expected_token = "50cb56d0ebe9142ca4ed97bc8dac3ee1";
+      var expected_token = "b4ed64d93a411a59f330ab3d798e4009";
       Step(
         function do_post()
         {
@@ -559,6 +559,54 @@ suite('multilayer', function() {
               next(err);
           });
         },
+        function do_get_tile_unauth(err)
+        {
+          if ( err ) throw err;
+          var next = this;
+          assert.response(server, {
+              url: '/tiles/layergroup/' + expected_token + ':cb0/0/0/0.png',
+              method: 'GET',
+              headers: {host: 'localhost' },
+              encoding: 'binary'
+          }, {}, function(res) {
+              assert.equal(res.statusCode, 401);
+              var re = RegExp('permission denied');
+              assert.ok(res.body.match(re), 'No "permission denied" error: ' + res.body);
+              next(err);
+          });
+        },
+        function do_get_grid_layer0_unauth(err)
+        {
+          if ( err ) throw err;
+          var next = this;
+          assert.response(server, {
+              url: '/tiles/layergroup/' + expected_token
+                 + '/0/0/0/0.grid.json',
+              headers: {host: 'localhost' },
+              method: 'GET'
+          }, {}, function(res) {
+              assert.equal(res.statusCode, 401);
+              var re = RegExp('permission denied');
+              assert.ok(res.body.match(re), 'No "permission denied" error: ' + res.body);
+              next(err);
+          });
+        },
+        function do_get_grid_layer1_unauth(err)
+        {
+          if ( err ) throw err;
+          var next = this;
+          assert.response(server, {
+              url: '/tiles/layergroup/' + expected_token
+                 + '/1/0/0/0.grid.json',
+              headers: {host: 'localhost' },
+              method: 'GET'
+          }, {}, function(res) {
+              assert.equal(res.statusCode, 401);
+              var re = RegExp('permission denied');
+              assert.ok(res.body.match(re), 'No "permission denied" error: ' + res.body);
+              next(err);
+          });
+        },
         function finish(err) {
           var errors = [];
           if ( err ) {
@@ -578,6 +626,62 @@ suite('multilayer', function() {
       );
     });
 
+    // https://github.com/cartodb/Windshaft-cartodb/issues/81
+    test("invalid text-name in CartoCSS", function(done) {
+
+      var layergroup =  {
+        version: '1.0.1',
+        layers: [
+           { options: {
+               sql: "select 1 as cartodb_id, 'SRID=3857;POINT(0 0)'::geometry as the_geom_webmercator",
+               cartocss: '#sample { text-name: cartodb_id; text-face-name: "Dejagnu"; }',
+               cartocss_version: '2.1.0',
+             } }
+        ]
+      };
+
+      assert.response(server, {
+          url: '/tiles/layergroup?',
+          method: 'POST',
+          headers: {host: 'localhost', 'Content-Type': 'application/json' },
+          data: JSON.stringify(layergroup)
+      }, {}, function(res) {
+          assert.equal(res.statusCode, 400, res.statusCode + ': ' + res.body);
+          var parsed = JSON.parse(res.body);
+          assert.deepEqual(parsed, {"errors":["style0:1:10 Invalid value for text-name, the type expression is expected. cartodb_id (of type keyword)  was given."]});
+          done();
+      });
+    });
+
+    test("quotes CartoCSS", function(done) {
+
+      var layergroup =  {
+        version: '1.0.1',
+        layers: [
+           { options: {
+               sql: "select 'single''quote' as n, 'SRID=3857;POINT(0 0)'::geometry as the_geom_webmercator",
+               cartocss: '#s [n="single\'quote" ] { marker-fill:red; }',
+               cartocss_version: '2.1.0',
+             } },
+           { options: {
+               sql: "select 'double\"quote' as n, 'SRID=3857;POINT(2 0)'::geometry as the_geom_webmercator",
+               cartocss: '#s [n="double\\"quote" ] { marker-fill:red; }',
+               cartocss_version: '2.1.0',
+             } }
+        ]
+      };
+
+      assert.response(server, {
+          url: '/tiles/layergroup?',
+          method: 'POST',
+          headers: {host: 'localhost', 'Content-Type': 'application/json' },
+          data: JSON.stringify(layergroup)
+      }, {}, function(res) {
+          assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body);
+          done();
+      });
+    });
+
     suiteTeardown(function(done) {
 
         // This test will add map_style records, like

test/acceptance/server.js

@@ -1020,6 +1020,23 @@ suite('server', function() {
         );
     });
 
+    // Zoom is a special variable
+    test("Specifying zoom level in CartoCSS does not need a 'zoom' variable in SQL output", function(done){
+        // NOTE: may fail if grainstore < 0.3.0 is used by Windshaft
+        var query = querystring.stringify({
+          sql: "SELECT 'SRID=3857;POINT(0 0)'::geometry as the_geom_webmercator, 1::int as cartodb_id",
+          style: '#gadm4 [ zoom>=3] { marker-fill:red; }'
+        });
+        assert.response(server, {
+            headers: {host: 'localhost'},
+            url: '/tiles/gadm4/0/0/0.png?' + query,
+            method: 'GET'
+        },{}, function(res) {
+          assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body);
+          done();
+        });
+    });
+
     /////////////////////////////////////////////////////////////////////////////////
     //
     // DELETE CACHE 

test/support/config.js

@@ -1,5 +1,8 @@
 var _ = require('underscore');
 
+
+require(__dirname + '/test_helper');
+
 module.exports = function(opts) {
 
     var config = {
@@ -7,7 +10,7 @@ module.exports = function(opts) {
             max: 10, 
             idleTimeoutMillis: 1, 
             reapIntervalMillis: 1,
-            port: 6333 // TODO: read from test env ?
+            port: global.environment.redis.port
         }
     }
 

test/support/prepare_db.sh

@@ -17,7 +17,7 @@ die() {
 }
 
 TEST_DB="cartodb_test_user_1_db"
-REDIS_PORT=6333
+if test -z "$REDIS_PORT"; then REDIS_PORT=6333; fi
 
 echo "preparing postgres..."
 dropdb "${TEST_DB}"

test/unit/cartodb/redis_pool.test.js

@@ -38,6 +38,7 @@ suite('redis_pool', function() {
     
     test('calling aquire returns a redis client object that can get/set', function(done){
       redis_pool.acquire(0, function(err, client){
+        if ( err ) { done(err); return; }
         client.set("key","value");
         client.get("key", function(err,data){      
           assert.equal(data, "value");      
@@ -49,6 +50,7 @@ suite('redis_pool', function() {
     
     test('calling aquire on another DB returns a redis client object that can get/set', function(done){
       redis_pool.acquire(2, function(err, client){
+        if ( err ) { done(err); return; }
         client.set("key","value");
         client.get("key", function(err,data){      
           assert.equal(data, "value");      

test/unit/cartodb/req2params.test.js

@@ -21,7 +21,7 @@ suite('req2params', function() {
           assert.ok(req.hasOwnProperty('params'), 'request has params');
           assert.ok(req.params.hasOwnProperty('interactivity'), 'request params have interactivity');
           assert.equal(req.params.dbname, 'cartodb_test_user_1_db', 'could forge dbname: '+ req.params.dbname);
-          assert.ok(!req.hasOwnProperty('dbuser'), 'could inject dbuser ('+req.params.dbuser+')');
+          assert.ok(!req.params.hasOwnProperty('dbuser'), 'could inject dbuser ('+req.params.dbuser+')');
           done();
       });
     });
@@ -53,11 +53,11 @@ suite('req2params', function() {
           // database_name for user "localhost" (see test/support/prepare_db.sh)
           assert.equal(req.params.dbname, 'cartodb_test_user_1_db');
           // id for user "localhost" (see test/support/prepare_db.sh)
-          assert.equal(req.dbuser, 'test_cartodb_user_1');
+          assert.equal(req.params.dbuser, 'test_cartodb_user_1');
  
           opts.req2params({headers: { host:'localhost' }, query: {map_key: '1235'} }, function(err, req) {
               // wrong key resets params to no user
-              assert.ok(!req.hasOwnProperty('dbuser'), 'could inject dbuser ('+req.params.dbuser+')');
+              assert.ok(!req.params.hasOwnProperty('dbuser'), 'could inject dbuser ('+req.params.dbuser+')');
               done();
           });
       });

tools/multilayer_token

@@ -23,11 +23,11 @@ if test -z "$cfg"; then
   exit 1
 fi
 
-cmd="curl -skH Content-Type:application/json --data-binary @${cfg} ${tiler_url}"
+cmd="curl -skH Content-Type:application/json --data-binary @- ${tiler_url}"
 if test x${verbose} = xyes; then
   cmd="${cmd} -v"
 fi
-res=`${cmd}`
+res=`cat ${cfg} | tr '\n' ' ' | ${cmd}`
 if test $? -gt 0; then
   echo "curl command failed: ${cmd}"
 fi