From 42a165b38bba4074fb00c5b43bb541a324ba1f48 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Mon, 26 Mar 2012 16:32:28 -0700 Subject: [PATCH 1/5] added query util for json query strings --- lib/router.js | 60 +--------------------------------------------- lib/server.js | 6 +++++ test/query.test.js | 43 +++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 59 deletions(-) create mode 100644 test/query.test.js diff --git a/lib/router.js b/lib/router.js index d4d27dd..e389243 100644 --- a/lib/router.js +++ b/lib/router.js @@ -73,62 +73,4 @@ var router = module.exports = function (req, res, next) { next({status: 404}); } }) -} - - - - - - - - - - - - - - -// module.exports = function (req, res, next) { -// var parsed = url.parse(req.url).path.split('?')[0].replace('/', '').split('/') -// , collections = req.collections = [] -// , references = req.references = [] -// , method = req.method -// , path = '/' -// ; -// -// // parse url into references and collections -// parsed.forEach(function (part, i) { -// (i % 2 ? references : collections).push(part); -// }); -// -// // cat collection reference -// path += collections[0]; -// -// // route to the first collection -// resources.get({path: path}).first(function (err, resource) { -// -// if(internals[path]) { -// if(req.isRemote && !req.isRoot) { -// // remote requests must have a registered key -// return next({status: 401}); -// } else { -// req.resource = { -// require: internals[path], -// path: path -// }; -// -// next(); -// } -// } else { -// // for future reference -// req.resource = resource; -// -// if(!resource) { -// err = {status: 404}; -// } -// -// // continue -// next(err); -// } -// }); -// } \ No newline at end of file +} \ No newline at end of file diff --git a/lib/server.js b/lib/server.js index 993715a..e1c697a 100644 --- a/lib/server.js +++ b/lib/server.js @@ -39,6 +39,12 @@ middleware.listen = function (callback) { // by default pause the request req.pause(); + // query sugar for JSON based query strings + // eg ?q={"foo": {"bar": true}} + if(req.query && req.query.q && req.query.q[0] === '{') { + req.query = JSON.parse(req.query.q); + } + if(req.method === 'GET' || req.method === 'DELETE') return next(); // check for json content type diff --git a/test/query.test.js b/test/query.test.js new file mode 100644 index 0000000..ce7b1fc --- /dev/null +++ b/test/query.test.js @@ -0,0 +1,43 @@ +describe('Queries', function(){ + describe('GET /todos?title=testing title', function(){ + it('should return the documents that match the query', function(done) { + todos.post({title: 'testing title'}, function (err) { + todos.use('?title=testing%20title').get(function (err, todos) { + expect(todos).to.exist; + expect(todos).to.have.length(1); + done(err); + }) + }) + }) + }) + + describe('GET /todos?q={"title": "testing title"}', function(){ + it('should parse the JSON and return the documents that match the query', function(done) { + todos.post({title: 'testing title'}, function (err) { + todos.use('?q=' + encodeURI(JSON.stringify({title: 'testing title'}))).get(function (err, todos) { + expect(todos).to.exist; + expect(todos).to.have.length(1); + done(err); + }) + }) + }) + }) +}) + +describe('Advanced Queries', function(){ + describe('GET /todos?q={"title": {$regex: "^title"}', function(){ + it('should parse the JSON and return the documents that match the query', function(done) { + todos.post({title: 'title one'}, function (err) { + todos.post({title: 'title two'}, function (err) { + todos.post({title: 'another title'}, function (err) { + todos.use('?q=' + encodeURI(JSON.stringify({title: {$regex: "^title"}}))).get(function (err, todos) { + expect(todos).to.exist; + expect(todos).to.have.length(2); + done(err); + }) + }) + }) + }) + }) + }) +}) \ No newline at end of file From 55752ac596bcf2a32d75e35b3f6350e8d7d15943 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Mon, 26 Mar 2012 17:29:01 -0700 Subject: [PATCH 2/5] added input sanitization --- lib/validation.js | 10 +++++++++- test/validation.test.js | 13 ++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/validation.js b/lib/validation.js index 6a0cc82..48b38d8 100644 --- a/lib/validation.js +++ b/lib/validation.js @@ -15,6 +15,7 @@ module.exports = function (req, res, next) { , resource = req.resource , validation , err + , sanitized = {} ; // rewrite queries from references @@ -37,7 +38,14 @@ module.exports = function (req, res, next) { } // if trying to write data - if((method === 'POST' || method === 'PUT') && resource && resource.properties) { + if((method === 'POST' || method === 'PUT') && req.body && resource && resource.properties) { + // sanitize data + Object.keys(resource.properties).forEach(function (key) { + sanitized[key] = req.body[key]; + }) + + // replace input with sanitized data + req.body = req.data = sanitized; // validate JSON validation = revalidator.validate(req.body, resource); diff --git a/test/validation.test.js b/test/validation.test.js index 3c8ded2..bcbf947 100644 --- a/test/validation.test.js +++ b/test/validation.test.js @@ -12,7 +12,7 @@ describe('Resource Actions', function(){ describe('POST /todos', function(){ it('should return an error when provided invalid data', function(done) { - todos.post({foo: 'bar', bat: 'baz'}, function (err, todo, req, res) { + todos.post({title: 123}, function (err, todo, req, res) { expect(err).to.exist; expect(err.valid).to.equal(false); expect(err.errors).to.have.length(1); @@ -21,6 +21,17 @@ describe('Resource Actions', function(){ }) }) + it('should ignore properties outside the schema', function(done) { + todos.post({title: 'foo', bat: 'baz'}, function (err, todo, req, res) { + todos.get(function (err, todos) { + var todo = todos[0]; + expect(todo.title).to.equal('foo'); + expect(todo.bat).to.not.exist; + done(err); + }) + }) + }) + it('should save the todo when valid', function(done) { todos.post({title: 'feed the cat'}, function (err, todo) { expect(todo._id).to.exist; From f78778a9b49397fd92d5f3341af0fc20e87441cb Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Mon, 26 Mar 2012 17:48:39 -0700 Subject: [PATCH 3/5] added human readable errors --- lib/validation.js | 40 ++++++++++++++++++++++++++++++++++++++-- test/validation.test.js | 5 +++-- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/lib/validation.js b/lib/validation.js index 48b38d8..1522a00 100644 --- a/lib/validation.js +++ b/lib/validation.js @@ -49,11 +49,47 @@ module.exports = function (req, res, next) { // validate JSON validation = revalidator.validate(req.body, resource); - err = validation.valid ? err : validation; + err = validation.valid ? err : transform(validation); + next(err); } else { // continue next(err); } -} \ No newline at end of file +} + +/** + * Transform revalidator errors into human redable errors. + */ + +function transform(validation) { + var err = {} + , errors = validation.errors + , e + , prop + ; + + for(var i = 0, len = errors.length; i < len; i++) { + e = errors[i]; + prop = e.property; + + switch(e.attribute) { + case 'type': + err[prop] = 'must be a ' + e.expected; + break; + case 'required': + err[prop] = 'is required'; + break; + default: + err[prop] = 'is not valid' + break; + } + } + + // rename and add human readable errors + validation.validation = validation.errors; + validation.errors = err; + + return validation; +} diff --git a/test/validation.test.js b/test/validation.test.js index bcbf947..adc30e6 100644 --- a/test/validation.test.js +++ b/test/validation.test.js @@ -12,10 +12,11 @@ describe('Resource Actions', function(){ describe('POST /todos', function(){ it('should return an error when provided invalid data', function(done) { - todos.post({title: 123}, function (err, todo, req, res) { + todos.post({foo: 123, completed: 'flarg'}, function (err, todo, req, res) { expect(err).to.exist; expect(err.valid).to.equal(false); - expect(err.errors).to.have.length(1); + expect(err.validation).to.have.length(2); + expect(err.errors).to.be.a('object'); expect(todo).to.not.exist; done(); }) From 64fd4210ce99d92f549f2369d9e325c83601988d Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Tue, 27 Mar 2012 09:32:49 -0700 Subject: [PATCH 4/5] added collection renaming --- lib/collections/resources.js | 11 +++++++++++ test/resources.test.js | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/lib/collections/resources.js b/lib/collections/resources.js index a1bea3a..86e6f7c 100644 --- a/lib/collections/resources.js +++ b/lib/collections/resources.js @@ -20,6 +20,17 @@ module.exports = , errs = [] ; + end(function (req, res, next) { + if(!collectionPath) return next(); + + console.log('rename to', collectionPath.replace('/', '')); + + collection.use(collectionPath).rename(collectionPath.replace('/', ''), function (err) { + next(err); + }); + }) + + properties && Object.keys(properties).forEach(function (key) { prop = properties[key]; rename = prop.$renameFrom; diff --git a/test/resources.test.js b/test/resources.test.js index 06c39df..7630397 100644 --- a/test/resources.test.js +++ b/test/resources.test.js @@ -62,6 +62,26 @@ describe('Application Resource Types', function(){ }) }) + describe('PUT /resources/', function(){ + it('should rename the resource collection', function(done) { + resources.get(function (e, all) { + var res = all[0]; + res.path = '/tasks'; + + todos.post({title: 'foo'}, function () { + resources.use('/' + res._id).put(res, function (err, upd) { + unauthed.use('/tasks').get(function (ee, r) { + expect(r).to.exist; + client.use('/tasks').del(function () { + done(ee || e || err); + }) + }) + }) + }) + }) + }) + }) + describe('PUT /resources/', function(){ it('should rename change properties on any existing data', function(done) { var exTodo = {title: 'feed fido', completed: true}; From c1b7fd0a3bcd77202c0904e17a3f68250a4ce5a6 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Tue, 27 Mar 2012 09:34:51 -0700 Subject: [PATCH 5/5] removed log --- lib/collections/resources.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/collections/resources.js b/lib/collections/resources.js index 86e6f7c..cf3728d 100644 --- a/lib/collections/resources.js +++ b/lib/collections/resources.js @@ -23,8 +23,6 @@ module.exports = end(function (req, res, next) { if(!collectionPath) return next(); - console.log('rename to', collectionPath.replace('/', '')); - collection.use(collectionPath).rename(collectionPath.replace('/', ''), function (err) { next(err); });