From 6cb844071517eac2ddd96b13949de3e4c2f6910a Mon Sep 17 00:00:00 2001 From: Peter Jaszkowiak Date: Fri, 30 Jun 2017 23:38:31 -0600 Subject: [PATCH 1/3] Fix #5793 Use a custom header instead --- public/src/ajaxify.js | 12 +++++++++++- src/controllers/errors.js | 2 +- src/controllers/helpers.js | 2 +- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/public/src/ajaxify.js b/public/src/ajaxify.js index 26c9950254..9ca9052dc9 100644 --- a/public/src/ajaxify.js +++ b/public/src/ajaxify.js @@ -293,11 +293,21 @@ $(document).ready(function () { headers: { 'X-Return-To': app.previousUrl, }, - success: function (data) { + success: function (data, textStatus, xhr) { if (!data) { return; } + if (xhr.getResponseHeader('X-Redirect')) { + return callback({ + data: { + status: 302, + responseJSON: data, + }, + textStatus: 'error', + }); + } + ajaxify.data = data; data.config = config; diff --git a/src/controllers/errors.js b/src/controllers/errors.js index 99614f9e87..177bcaa769 100644 --- a/src/controllers/errors.js +++ b/src/controllers/errors.js @@ -45,7 +45,7 @@ exports.handleErrors = function (err, req, res, next) { // eslint-disable-line n var status = parseInt(err.status, 10); if ((status === 302 || status === 308) && err.path) { - return res.locals.isAPI ? res.status(status).json(err.path) : res.redirect(err.path); + return res.locals.isAPI ? res.set('X-Redirect', err.path).status(200).json(err.path) : res.redirect(err.path); } winston.error(req.path + '\n', err.stack); diff --git a/src/controllers/helpers.js b/src/controllers/helpers.js index 761faafa82..8db1d6266f 100644 --- a/src/controllers/helpers.js +++ b/src/controllers/helpers.js @@ -53,7 +53,7 @@ helpers.notAllowed = function (req, res, error) { helpers.redirect = function (res, url) { if (res.locals.isAPI) { - res.status(308).json(url); + res.set('X-Redirect', url).status(200).json(url); } else { res.redirect(nconf.get('relative_path') + encodeURI(url)); } From f1c955e6fc532fba6bb03009157b0c25b1a22b69 Mon Sep 17 00:00:00 2001 From: Peter Jaszkowiak Date: Fri, 30 Jun 2017 23:38:52 -0600 Subject: [PATCH 2/3] Fix scrollStop not working sometimes --- public/src/modules/scrollStop.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/src/modules/scrollStop.js b/public/src/modules/scrollStop.js index 82d2df4de8..0e07641cc3 100644 --- a/public/src/modules/scrollStop.js +++ b/public/src/modules/scrollStop.js @@ -16,7 +16,7 @@ define('scrollStop', function () { $(element).on('mousewheel', function (e) { var scrollTop = this.scrollTop; var scrollHeight = this.scrollHeight; - var elementHeight = this.getBoundingClientRect().height; + var elementHeight = Math.round(this.getBoundingClientRect().height); if ( (e.originalEvent.deltaY < 0 && scrollTop === 0) || // scroll up From 1471fbdc14b789c801dd72e64999a7d4294e7489 Mon Sep 17 00:00:00 2001 From: Peter Jaszkowiak Date: Sun, 2 Jul 2017 16:26:31 -0600 Subject: [PATCH 3/3] Fix tests --- test/controllers.js | 29 ++++++++++++++++++----------- test/messaging.js | 7 ++++--- test/topics.js | 7 ++++--- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/test/controllers.js b/test/controllers.js index 1f73d4a7ae..34c515fc7a 100644 --- a/test/controllers.js +++ b/test/controllers.js @@ -808,7 +808,8 @@ describe('Controllers', function () { it('should redirect to account page with logged in user', function (done) { request(nconf.get('url') + '/api/login', { jar: jar, json: true }, function (err, res, body) { assert.ifError(err); - assert.equal(res.statusCode, 308); + assert.equal(res.statusCode, 200); + assert.equal(res.headers['x-redirect'], '/user/foo'); assert.equal(body, '/user/foo'); done(); }); @@ -825,7 +826,8 @@ describe('Controllers', function () { it('should redirect to userslug', function (done) { request(nconf.get('url') + '/api/uid/' + fooUid, { json: true }, function (err, res, body) { assert.ifError(err); - assert.equal(res.statusCode, 308); + assert.equal(res.statusCode, 200); + assert.equal(res.headers['x-redirect'], '/user/foo'); assert.equal(body, '/user/foo'); done(); }); @@ -1238,10 +1240,11 @@ describe('Controllers', function () { }); it('should return correct post path', function (done) { - request(nconf.get('url') + '/api/post/' + pid, function (err, res, body) { + request(nconf.get('url') + '/api/post/' + pid, { json: true }, function (err, res, body) { assert.ifError(err); - assert.equal(res.statusCode, 308); - assert.equal(body, '"/topic/1/test-topic-title/1"'); + assert.equal(res.statusCode, 200); + assert.equal(res.headers['x-redirect'], '/topic/1/test-topic-title/1'); + assert.equal(body, '/topic/1/test-topic-title/1'); done(); }); }); @@ -1411,7 +1414,8 @@ describe('Controllers', function () { request(nconf.get('url') + '/api/users', { json: true }, function (err, res, body) { assert.ifError(err); - assert.equal(res.statusCode, 308); + assert.equal(res.statusCode, 200); + assert.equal(res.headers['x-redirect'], '/api/popular'); assert(body, '/api/popular'); done(); }); @@ -1521,7 +1525,8 @@ describe('Controllers', function () { it('should redirect if topic index is negative', function (done) { request(nconf.get('url') + '/api/category/' + category.slug + '/-10', function (err, res) { assert.ifError(err); - assert.equal(res.statusCode, 308); + assert.equal(res.statusCode, 200); + assert.ok(res.headers['x-redirect']); done(); }); }); @@ -1627,7 +1632,8 @@ describe('Controllers', function () { function (category, next) { request(nconf.get('url') + '/api/category/' + category.slug, { jar: jar, json: true }, function (err, res, body) { assert.ifError(err); - assert.equal(res.statusCode, 308); + assert.equal(res.statusCode, 200); + assert.equal(res.headers['x-redirect'], 'https://nodebb.org'); assert.equal(body, 'https://nodebb.org'); next(); }); @@ -1754,10 +1760,11 @@ describe('Controllers', function () { }); it('should redirect if page is out of bounds', function (done) { - request(nconf.get('url') + '/api/unread?page=-1', { jar: jar }, function (err, res, body) { + request(nconf.get('url') + '/api/unread?page=-1', { jar: jar, json: true }, function (err, res, body) { assert.ifError(err); - assert.equal(res.statusCode, 308); - assert.equal(body, '"/unread?page=1"'); + assert.equal(res.statusCode, 200); + assert.equal(res.headers['x-redirect'], '/unread?page=1'); + assert.equal(body, '/unread?page=1'); done(); }); }); diff --git a/test/messaging.js b/test/messaging.js index d6426812fe..cc8283698b 100644 --- a/test/messaging.js +++ b/test/messaging.js @@ -591,10 +591,11 @@ describe('Messaging Library', function () { }); it('should redirect to chats page', function (done) { - request(nconf.get('url') + '/api/chats', { jar: jar }, function (err, response, body) { + request(nconf.get('url') + '/api/chats', { jar: jar, json: true }, function (err, res, body) { assert.ifError(err); - assert.equal(body, '"/user/herp/chats"'); - assert.equal(response.statusCode, 308); + assert.equal(res.statusCode, 200); + assert.equal(res.headers['x-redirect'], '/user/herp/chats'); + assert.equal(body, '/user/herp/chats'); done(); }); }); diff --git a/test/topics.js b/test/topics.js index 541000dd52..a1d0bda512 100644 --- a/test/topics.js +++ b/test/topics.js @@ -791,10 +791,11 @@ describe('Topic\'s', function () { }); it('should redirect if post index is out of range', function (done) { - request(nconf.get('url') + '/api/topic/' + topicData.slug + '/-1', function (err, response, body) { + request(nconf.get('url') + '/api/topic/' + topicData.slug + '/-1', { json: true }, function (err, res, body) { assert.ifError(err); - assert.equal(response.statusCode, 308); - assert.equal(body, '"/topic/13/topic-for-controller-test"'); + assert.equal(res.statusCode, 200); + assert.equal(res.headers['x-redirect'], '/topic/13/topic-for-controller-test'); + assert.equal(body, '/topic/13/topic-for-controller-test'); done(); }); });