From 8592c37bf63e3f32ec8e77977d7be046fa358a61 Mon Sep 17 00:00:00 2001
From: Julian Simioni <julian@simioni.org>
Date: Wed, 16 Sep 2015 17:21:55 -0400
Subject: [PATCH] Expect multiple ids to be specified as a comma-delimited
 string

Disallow the other way that Node.js allows, which is to list the id
parameter multiple times in the querystring. See #272.
---
 sanitiser/_id.js             | 22 ++++++++++++++++------
 test/unit/sanitiser/place.js | 20 ++++++++++++++++----
 2 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/sanitiser/_id.js b/sanitiser/_id.js
index caeefb11..0cea0317 100644
--- a/sanitiser/_id.js
+++ b/sanitiser/_id.js
@@ -16,19 +16,29 @@ function sanitize( raw, clean ){
   // error & warning messages
   var messages = { errors: [], warnings: [] };
 
-  // 'raw.id' can be an array
-  var rawIds = check.array( raw.id ) ? _.unique( raw.id ) : [];
-
-  // 'raw.id' can be a string
-  if( check.unemptyString( raw.id ) ){
-    rawIds.push( raw.id );
+  // 'raw.id' can be an array if id is specified multiple times
+  // see https://github.com/pelias/api/issues/272
+  var rawIdString;
+  if (check.array( raw.id )) {
+    rawIdString = raw.id[0];
+    messages.warnings.push( '`id` parameter specified multiple times. Using first value.' );
+  } else if (check.unemptyString( raw.id )) {
+    rawIdString = raw.id;
+  } else {
+    rawIdString = '';
   }
 
+  // split string into array of values
+  var rawIds = rawIdString.split(',');
+
   // no ids provided
   if( !rawIds.length ){
     messages.errors.push( errorMessage('id') );
   }
 
+  // deduplicate
+  rawIds = _.unique(rawIds);
+
   // ensure all elements are valid non-empty strings
   rawIds = rawIds.filter( function( uc ){
     if( !check.unemptyString( uc ) ){
diff --git a/test/unit/sanitiser/place.js b/test/unit/sanitiser/place.js
index 634b49ea..38b1e3df 100644
--- a/test/unit/sanitiser/place.js
+++ b/test/unit/sanitiser/place.js
@@ -73,7 +73,7 @@ module.exports.tests.sanitize_id = function(test, common) {
 
 module.exports.tests.sanitize_ids = function(test, common) {
   test('ids: invalid input with multiple values', function(t) {
-    var req = { query: { id: inputs.invalid } };
+    var req = { query: { id: inputs.invalid.join(',') } };
     var expected = [
       'invalid param \'id\': text length, must be >0',
       'invalid param \':\': text length, must be >0',
@@ -99,7 +99,7 @@ module.exports.tests.sanitize_ids = function(test, common) {
       expected.ids.push({ id: input_parts[1], type: input_parts[0] });
     });
     expected.private = false;
-    var req = { query: { id: inputs.valid } };
+    var req = { query: { id: inputs.valid.join(',') } };
     sanitize(req, function(){
       t.deepEqual( req.errors, [], 'no errors' );
       t.deepEqual( req.clean, expected, 'clean set correctly' );
@@ -108,6 +108,18 @@ module.exports.tests.sanitize_ids = function(test, common) {
   });
 };
 
+module.exports.tests.array_of_ids = function(test, common) {
+  // see https://github.com/pelias/api/issues/272
+  test('array of ids sent by queryparser', function(t) {
+    var req = { query: { id: ['geoname:2', 'oswmay:4'] } };
+    sanitize(req, function() {
+      t.deepEqual( req.warnings, ['`id` parameter specified multiple times. Using first value.'], 'warning sent' );
+      t.deepEqual( req.clean.ids, [{ id: '2', type: 'geoname' }], 'only first value used in clean' );
+      t.end();
+    });
+  });
+};
+
 module.exports.tests.sanitize_private = function(test, common) {
   var invalid_values = [null, -1, 123, NaN, 'abc'];
   invalid_values.forEach(function(value) {
@@ -161,7 +173,7 @@ module.exports.tests.sanitize_private = function(test, common) {
 
 module.exports.tests.multiple_ids = function(test, common) {
   var expected = { ids: [ { id: '1', type: 'geoname' }, { id: '2', type: 'osmnode' } ], private: false };
-  var req = { query: { id: ['geoname:1', 'osmnode:2'] } };
+  var req = { query: { id: 'geoname:1,osmnode:2' } };
   test('duplicate ids', function(t) {
     sanitize( req, function(){
       t.deepEqual( req.errors, [], 'no errors' );
@@ -174,7 +186,7 @@ module.exports.tests.multiple_ids = function(test, common) {
 
 module.exports.tests.de_dupe = function(test, common) {
   var expected = { ids: [ { id: '1', type: 'geoname' }, { id: '2', type: 'osmnode' } ], private: false };
-  var req = { query: { id: ['geoname:1', 'osmnode:2', 'geoname:1'] } };
+  var req = { query: { id: 'geoname:1,osmnode:2,geoname:1' } };
   test('duplicate ids', function(t) {
     sanitize( req, function(){
       t.deepEqual( req.errors, [], 'no errors' );