From a3c2fe54965399eae1c61f58aa01c624ccbc0143 Mon Sep 17 00:00:00 2001
From: Jason Williams <jaswilli@gmail.com>
Date: Tue, 27 May 2014 21:57:53 +0000
Subject: [PATCH] Fix up unit tests. Check config url with isURL.

No issue
-validate url in config.js using validator.isURL
-fix up async flow in bootstrap unit tests
-make sure done handler is called on async tests
---
 core/bootstrap.js                             |   6 +-
 core/test/integration/api/api_db_spec.js      |  10 +-
 core/test/integration/api/api_mail_spec.js    |   2 +-
 .../integration/api/api_notifications_spec.js |   6 +-
 .../test/integration/api/api_settings_spec.js |   4 +-
 core/test/integration/api/api_slugs_spec.js   |   2 +-
 core/test/integration/api/api_themes_spec.js  |  10 +-
 core/test/integration/update_check_spec.js    |   6 +-
 core/test/unit/bootstrap_spec.js              | 267 ++++++++++++------
 core/test/unit/mail_spec.js                   |   9 +-
 core/test/unit/middleware_spec.js             |   6 +-
 core/test/unit/permissions_spec.js            |  14 +-
 12 files changed, 216 insertions(+), 126 deletions(-)

diff --git a/core/bootstrap.js b/core/bootstrap.js
index 9ce1e8ee41..67664ca936 100644
--- a/core/bootstrap.js
+++ b/core/bootstrap.js
@@ -7,6 +7,7 @@
 var fs      = require('fs'),
     url     = require('url'),
     when    = require('when'),
+    validator = require('validator'),
     errors  = require('./server/errors'),
     config  = require('./server/config'),
 
@@ -73,12 +74,13 @@ function validateConfigEnvironment() {
     }
 
     // Check that our url is valid
-    parsedUrl = url.parse(config.url || 'invalid', false, true);
-    if (!parsedUrl.host) {
+    if (!validator.isURL(config.url, { protocols: ['http', 'https'], require_protocol: true })) {
         errors.logError(new Error('Your site url in config.js is invalid.'), config.url, 'Please make sure this is a valid url before restarting');
         return when.reject(rejectMessage);
     }
 
+    parsedUrl = url.parse(config.url || 'invalid', false, true);
+
     if (/\/ghost(\/|$)/.test(parsedUrl.pathname)) {
         errors.logError(new Error('Your site url in config.js cannot contain a subdirectory called ghost.'), config.url, 'Please rename the subdirectory before restarting');
         return when.reject(rejectMessage);
diff --git a/core/test/integration/api/api_db_spec.js b/core/test/integration/api/api_db_spec.js
index d3420000ff..d2576e7843 100644
--- a/core/test/integration/api/api_db_spec.js
+++ b/core/test/integration/api/api_db_spec.js
@@ -54,9 +54,7 @@ describe('DB API', function () {
                 results.posts.length.should.equal(0);
                 done();
             });
-        }).catch(function (error) {
-            done(error);
-        });
+        }).catch(done);
     });
 
     it('delete all content is denied', function (done) {
@@ -77,7 +75,7 @@ describe('DB API', function () {
         }).catch(function (error) {
             error.type.should.eql('NoPermissionError');
             done();
-        });
+        }).catch(done);
     });
 
     it('export content is denied', function (done) {
@@ -98,7 +96,7 @@ describe('DB API', function () {
         }).catch(function (error) {
             error.type.should.eql('NoPermissionError');
             done();
-        });
+        }).catch(done);
     });
 
     it('import content is denied', function (done) {
@@ -119,6 +117,6 @@ describe('DB API', function () {
         }).catch(function (error) {
             error.type.should.eql('NoPermissionError');
             done();
-        });
+        }).catch(done);
     });
 });
\ No newline at end of file
diff --git a/core/test/integration/api/api_mail_spec.js b/core/test/integration/api/api_mail_spec.js
index 156e937796..62bbf746b1 100644
--- a/core/test/integration/api/api_mail_spec.js
+++ b/core/test/integration/api/api_mail_spec.js
@@ -45,6 +45,6 @@ describe('Mail API', function () {
         }).catch(function (error) {
             error.type.should.eql('EmailError');
             done();
-        });
+        }).catch(done);
     });
 });
\ No newline at end of file
diff --git a/core/test/integration/api/api_notifications_spec.js b/core/test/integration/api/api_notifications_spec.js
index 582e05608a..1493adc9cd 100644
--- a/core/test/integration/api/api_notifications_spec.js
+++ b/core/test/integration/api/api_notifications_spec.js
@@ -42,7 +42,7 @@ describe('Notifications API', function () {
                 results.notifications.length.should.be.above(0);
                 testUtils.API.checkResponse(results.notifications[0], 'notification');
                 done();
-            });
+            }).catch(done);
         });
     });
 
@@ -64,7 +64,7 @@ describe('Notifications API', function () {
             notification.location.should.equal('bottom');
 
             done();
-        });
+        }).catch(done);
     });
 
     it('can add, adds id and status', function (done) {
@@ -87,7 +87,7 @@ describe('Notifications API', function () {
             notification.status.should.equal('persistent')
             
             done();
-        });
+        }).catch(done);
     });
 
     it('can destroy', function (done) {
diff --git a/core/test/integration/api/api_settings_spec.js b/core/test/integration/api/api_settings_spec.js
index 931181a17a..f30b300b99 100644
--- a/core/test/integration/api/api_settings_spec.js
+++ b/core/test/integration/api/api_settings_spec.js
@@ -125,7 +125,7 @@ describe('Settings API', function () {
             should.exist(error);
             error.type.should.eql('NoPermissionError');
             done();
-        });
+        }).catch(done);
     });
 
     it('can read core settings if an internal request', function (done) {
@@ -172,7 +172,7 @@ describe('Settings API', function () {
                 err.type.should.eql('NoPermissionError');
 
                 done();
-            });
+            }).catch(done);
     });
 
     it('can edit a core setting with an internal request', function (done) {
diff --git a/core/test/integration/api/api_slugs_spec.js b/core/test/integration/api/api_slugs_spec.js
index 82dec22ea0..5dee1f43c8 100644
--- a/core/test/integration/api/api_slugs_spec.js
+++ b/core/test/integration/api/api_slugs_spec.js
@@ -60,7 +60,7 @@ describe('Slug API', function () {
         }, function (error) {
             error.type.should.eql('BadRequestError');
             done();
-        });
+        }).catch(done);
     });
 
 });
\ No newline at end of file
diff --git a/core/test/integration/api/api_themes_spec.js b/core/test/integration/api/api_themes_spec.js
index 2ba55929bc..7228358bd6 100644
--- a/core/test/integration/api/api_themes_spec.js
+++ b/core/test/integration/api/api_themes_spec.js
@@ -19,7 +19,7 @@ describe('Themes API', function () {
     before(function (done) {
         testUtils.clearData().then(function () {
             done();
-        }, done);
+        }).catch(done);
     });
 
     beforeEach(function (done) {
@@ -53,14 +53,14 @@ describe('Themes API', function () {
             });
 
             done();
-        }, done);
+        }).catch(done);
     });
 
     afterEach(function (done) {
         testUtils.clearData().then(function () {
             sandbox.restore();
             done();
-        }, done);
+        }).catch(done);
     });
 
     it('can browse', function (done) {
@@ -77,7 +77,7 @@ describe('Themes API', function () {
             done();
         }).catch(function (error) {
             done(new Error(JSON.stringify(error)));
-        });
+        }).catch(done);
     });
 
     it('can edit', function (done) {
@@ -96,6 +96,6 @@ describe('Themes API', function () {
             done();
         }).catch(function (error) {
             done(new Error(JSON.stringify(error)));
-        });
+        }).catch(done);
     })
 });
\ No newline at end of file
diff --git a/core/test/integration/update_check_spec.js b/core/test/integration/update_check_spec.js
index d5b63b6c9a..9e45a19336 100644
--- a/core/test/integration/update_check_spec.js
+++ b/core/test/integration/update_check_spec.js
@@ -18,7 +18,7 @@ describe('Update Check', function () {
             return testUtils.clearData();
         }).then(function () {
             done();
-        }, done);
+        }).catch(done);
     });
 
     after(function () {
@@ -36,13 +36,13 @@ describe('Update Check', function () {
             return permissions.init();
         }).then(function () {
             done();
-        }, done);
+        }).catch(done);
     });
 
     afterEach(function (done) {
         testUtils.clearData().then(function () {
             done();
-        }, done);
+        }).catch(done);
     });
 
     it('should report the correct data', function (done) {
diff --git a/core/test/unit/bootstrap_spec.js b/core/test/unit/bootstrap_spec.js
index cf63b9f35c..fe338e1381 100644
--- a/core/test/unit/bootstrap_spec.js
+++ b/core/test/unit/bootstrap_spec.js
@@ -20,9 +20,8 @@ describe('Bootstrap', function () {
             bootstrap.__set__("readConfigFile",  sandbox.stub().returns(
                 _.extend({}, defaultConfig, newConfig)
             ));
-        };
-
-
+        },
+        expectedError = new Error('expected bootstrap() to throw error but none thrown');
 
     beforeEach(function () {
         sandbox = sinon.sandbox.create();
@@ -84,7 +83,7 @@ describe('Bootstrap', function () {
         }).catch(done);
     });
 
-    it('accepts valid urls', function (done) {
+    it('accepts urls with a valid scheme', function (done) {
         // replace the config file with invalid data
         overrideConfig({url: 'http://testurl.com'});
 
@@ -109,118 +108,220 @@ describe('Bootstrap', function () {
         }).then(function (localConfig) {
             localConfig.url.should.equal('http://testurl.com/ghostly/');
 
-            // Next test
-            overrideConfig({url: '//testurl.com'});
-            return bootstrap();
-        }).then(function (localConfig) {
-            localConfig.url.should.equal('//testurl.com');
+            done();
+        }).catch(done);
+    });
+
+    it('rejects a fqdn without a scheme', function (done) {
+
+        overrideConfig({ url: 'example.com' });
+
+        bootstrap().then(function () {
+            done(expectedError);
+        }).catch(function (err) {
+            should.exist(err);
+            err.should.contain(rejectMessage);
 
             done();
         }).catch(done);
     });
 
-    it('rejects invalid urls', function (done) {
-        // replace the config file with invalid data
-        overrideConfig({url: 'notvalid'});
+    it('rejects a hostname without a scheme', function (done) {
 
-        bootstrap().catch(function (error) {
-            error.should.include(rejectMessage);
+        overrideConfig({ url: 'example' });
 
-            // Next test
-            overrideConfig({url: 'something.com'});
-            return bootstrap();
-        }).catch(function (error) {
-            error.should.include(rejectMessage);
+        bootstrap().then(function () {
+            done(expectedError);
+        }).catch(function (err) {
+            should.exist(err);
+            err.should.contain(rejectMessage);
 
             done();
-        }).then(function () {
-            should.fail('no error was thrown when it should have been');
-            done();
-        });
+        }).catch(done);
     });
 
-    it('does not permit subdirectories named ghost', function (done) {
-        // replace the config file with invalid data
-        overrideConfig({url: 'http://testurl.com/ghost/'});
+    it('rejects a hostname with a scheme', function (done) {
 
-        bootstrap().catch(function (error) {
-            error.should.include(rejectMessage);
+        overrideConfig({ url: 'https://example' });
 
-             // Next test
-            overrideConfig({url: 'http://testurl.com/ghost/blog/'});
-            return bootstrap();
-        }).catch(function (error) {
-            error.should.include(rejectMessage);
-
-            // Next test
-            overrideConfig({url: 'http://testurl.com/blog/ghost'});
-            return bootstrap();
-        }).catch(function (error) {
-            error.should.include(rejectMessage);
+        bootstrap().then(function () {
+            done(expectedError);
+        }).catch(function (err) {
+            should.exist(err);
+            err.should.contain(rejectMessage);
 
             done();
-        }).then(function () {
-            should.fail('no error was thrown when it should have been');
-            done();
-        });
+        }).catch(done);
     });
 
-    it('requires a database config', function (done) {
+    it('rejects a url with an unsupported scheme', function (done) {
+
+        overrideConfig({ url: 'ftp://example.com' });
+
+        bootstrap().then(function () {
+            done(expectedError);
+        }).catch(function (err) {
+            should.exist(err);
+            err.should.contain(rejectMessage);
+
+            done();
+        }).catch(done);
+    });
+
+     it('rejects a url with a protocol relative scheme', function (done) {
+
+        overrideConfig({ url: '//example.com' });
+
+        bootstrap().then(function () {
+            done(expectedError);
+        }).catch(function (err) {
+            should.exist(err);
+            err.should.contain(rejectMessage);
+
+            done();
+        }).catch(done);
+    });
+
+    it('does not permit the word ghost as a url path', function (done) {
+        overrideConfig({ url: 'http://example.com/ghost/' });
+
+        bootstrap().then(function () {
+            done(expectedError);
+        }).catch(function (err) {
+            should.exist(err);
+            err.should.contain(rejectMessage);
+
+            done();
+        }).catch(done);
+    });
+
+    it('does not permit the word ghost to be a component in a url path', function (done) {
+        overrideConfig({ url: 'http://example.com/blog/ghost/' });
+
+        bootstrap().then(function () {
+            done(expectedError);
+        }).catch(function (err) {
+            should.exist(err);
+            err.should.contain(rejectMessage);
+
+            done();
+        }).catch(done);
+    });
+
+    it('does not permit the word ghost to be a component in a url path', function (done) {
+        overrideConfig({ url: 'http://example.com/ghost/blog/' });
+
+        bootstrap().then(function () {
+            done(expectedError);
+        }).catch(function (err) {
+            should.exist(err);
+            err.should.contain(rejectMessage);
+
+            done();
+        }).catch(done);
+    });
+
+    it('does not permit database config to be falsy', function (done) {
         // replace the config file with invalid data
-        overrideConfig({database: null});
+        overrideConfig({ database: false });
 
-        bootstrap().catch(function (error) {
-            error.should.include(rejectMessage);
-
-            // Next test
-            overrideConfig({database: {}});
-            return bootstrap();
-        }).catch(function (error) {
-            error.should.include(rejectMessage);
+        bootstrap().then(function () {
+            done(expectedError);
+        }).catch(function (err) {
+            should.exist(err);
+            err.should.contain(rejectMessage);
 
             done();
-        }).then(function () {
-            should.fail('no error was thrown when it should have been');
+        }).catch(done);
+    });
+
+    it('does not permit database config to be empty', function (done) {
+        // replace the config file with invalid data
+        overrideConfig({ database: {} });
+
+        bootstrap().then(function () {
+            done(expectedError);
+        }).catch(function (err) {
+            should.exist(err);
+            err.should.contain(rejectMessage);
+
             done();
-        });
+        }).catch(done);
     });
 
 
-    it('requires a socket or a host and port', function (done) {
-        // replace the config file with invalid data
-        overrideConfig({server: {socket: 'test'}});
+    it('requires server to be present', function (done) {
+        overrideConfig({ server: false });
 
         bootstrap().then(function (localConfig) {
+            done(expectedError);
+        }).catch(function (err) {
+            should.exist(err);
+            err.should.contain(rejectMessage);
+
+            done();
+        }).catch(done);
+    });
+
+    it('allows server to use a socket', function (done) {
+        overrideConfig({ server: { socket: 'test' } });
+
+        bootstrap().then(function (localConfig) {
+            should.exist(localConfig);
             localConfig.server.socket.should.equal('test');
 
-              // Next test
-            overrideConfig({server: null});
-            return bootstrap();
-        }).catch(function (error) {
-            error.should.include(rejectMessage);
+            done();
+        }).catch(done);
+    });
 
-            // Next test
-            overrideConfig({server: {host: null}});
-            return bootstrap();
-        }).catch(function (error) {
-            error.should.include(rejectMessage);
+    it('allows server to have a host and a port', function (done) {
+        overrideConfig({ server: { host: '127.0.0.1', port: '2368' } });
 
-            // Next test
-            overrideConfig({server: {port: null}});
-            return bootstrap();
-        }).catch(function (error) {
-            error.should.include(rejectMessage);
-
-            // Next test
-            overrideConfig({server: {host: null, port: null}});
-            return bootstrap();
-        }).catch(function (error) {
-            error.should.include(rejectMessage);
+        bootstrap().then(function (localConfig) {
+            should.exist(localConfig);
+            localConfig.server.host.should.equal('127.0.0.1');
+            localConfig.server.port.should.equal('2368');
 
             done();
-        }).then(function () {
-            should.fail('no error was thrown when it should have been');
+        }).catch(done);
+    });
+
+    it('rejects server if there is a host but no port', function (done) {
+        overrideConfig({ server: { host: '127.0.0.1' } });
+
+        bootstrap().then(function () {
+            done(expectedError);
+        }).catch(function (err) {
+            should.exist(err);
+            err.should.contain(rejectMessage);
+
             done();
-        });
+        }).catch(done);
+    });
+
+    it('rejects server if there is a port but no host', function (done) {
+        overrideConfig({ server: { port: '2368' } });
+
+        bootstrap().then(function () {
+            done(expectedError);
+        }).catch(function (err) {
+            should.exist(err);
+            err.should.contain(rejectMessage);
+
+            done();
+        }).catch(done);
+    });
+
+    it('rejects server if configuration is empty', function (done) {
+        overrideConfig({ server: {} });
+
+        bootstrap().then(function () {
+            done(expectedError);
+        }).catch(function (err) {
+            should.exist(err);
+            err.should.contain(rejectMessage);
+
+            done();
+        }).catch(done);
     });
 });
\ No newline at end of file
diff --git a/core/test/unit/mail_spec.js b/core/test/unit/mail_spec.js
index 3bfd9212cb..8df865b212 100644
--- a/core/test/unit/mail_spec.js
+++ b/core/test/unit/mail_spec.js
@@ -149,7 +149,7 @@ describe("Mail", function () {
             }).catch(function (err) {
                 err.should.be.an.instanceOf(Error);
                 done();
-            });
+            }).catch(done);
         });
     });
 
@@ -168,13 +168,12 @@ describe("Mail", function () {
         }).catch(done);
     });
 
-    it('should use from address as configured in config.js', function (done) {
+    it('should use from address as configured in config.js', function () {
         overrideConfig({mail:{fromaddress: 'static@example.com'}});
         mailer.fromAddress().should.equal('static@example.com');
-        done();
     });
 
-    it('should fall back to ghost@[blog.url] as from address', function (done) {
+    it('should fall back to ghost@[blog.url] as from address', function () {
         // Standard domain
         overrideConfig({url: 'http://default.com', mail:{fromaddress: null}});
         mailer.fromAddress().should.equal('ghost@default.com');
@@ -186,7 +185,5 @@ describe("Mail", function () {
         // Strip Port
         overrideConfig({url: 'http://default.com:2368/', mail:{}});
         mailer.fromAddress().should.equal('ghost@default.com');
-
-        done();
     });
 });
diff --git a/core/test/unit/middleware_spec.js b/core/test/unit/middleware_spec.js
index 43119dad24..eeb67a8f86 100644
--- a/core/test/unit/middleware_spec.js
+++ b/core/test/unit/middleware_spec.js
@@ -73,10 +73,9 @@ describe('Middleware', function () {
             };
         });
 
-        it('should return a json 401 error response', function (done) {
+        it('should return a json 401 error response', function () {
             middleware.authAPI(req, res, null);
             assert(res.json.calledWith(401, { error: 'Please sign in' }));
-            done();
         });
 
         it('should call next if a user exists in session', function (done) {
@@ -103,12 +102,11 @@ describe('Middleware', function () {
             };
         });
 
-        it('should redirect to dashboard', function (done) {
+        it('should redirect to dashboard', function () {
             req.session.user = {};
 
             middleware.redirectToDashboard(req, res, null);
             assert(res.redirect.calledWithMatch('/ghost/'));
-            done();
         });
 
         it('should call next if no user in session', function (done) {
diff --git a/core/test/unit/permissions_spec.js b/core/test/unit/permissions_spec.js
index 11db5ae4f6..5a9cadadd3 100644
--- a/core/test/unit/permissions_spec.js
+++ b/core/test/unit/permissions_spec.js
@@ -221,7 +221,7 @@ describe('Permissions', function () {
                     return canThisResult.edit.page(fakePage);
                 })
                 .then(function () {
-                    errors.logError(new Error("Allowed edit post without permission"));
+                    done(new Error('was able to edit post without permission'));
                 }).catch(done);
         });
 
@@ -287,9 +287,8 @@ describe('Permissions', function () {
                 })
                 .catch(function () {
                     permissableStub.restore();
-                    errors.logError(new Error("Did not allow testUser"));
 
-                    done();
+                    done(new Error('did not allow testUser'));
                 });
         });
 
@@ -386,10 +385,7 @@ describe('Permissions', function () {
                         .post(updatedPost.id)
                         .then(function () {
                             done(new Error("Allowed an edit of post 1"));
-                        })
-                        .catch(function () {
-                            done();
-                        });
+                        }).catch(done);
                 }).catch(done);
         });
 
@@ -412,9 +408,7 @@ describe('Permissions', function () {
                 .then(function () {
                     done(new Error("Should not allow editing post"));
                 })
-                .catch(function () {
-                    done();
-                });
+                .catch(done);
         });
 
         it('allows \'internal\' to be passed for internal requests', function (done) {