From 3470786128ad72c7b63081e3fa9f1783b20f74b2 Mon Sep 17 00:00:00 2001 From: Alex Liu Date: Fri, 13 Jul 2018 18:21:10 -0700 Subject: [PATCH] fix: support parsing of valid falsy json --- lib/JsonClient.js | 7 +++- package.json | 2 +- test/index.js | 94 ++++++++++++++++++++++++++++++----------------- 3 files changed, 68 insertions(+), 35 deletions(-) diff --git a/lib/JsonClient.js b/lib/JsonClient.js index e3add92..f05d1ac 100644 --- a/lib/JsonClient.js +++ b/lib/JsonClient.js @@ -90,7 +90,12 @@ JsonClient.prototype.parse = function parse(err, req, res, callback) { parseErr = e; log.trace(parseErr, 'Invalid JSON in response'); } - obj = obj || (res && res.body) || {}; + + // empty string and undefined are not valid JSON, fall back on the + // original response body (string) or empty POJO in worst case. + if (obj === '' || typeof obj === 'undefined') { + obj = (res && res.body) || {}; + } // http errors take precedence over JSON.parse errors if (res && res.statusCode >= 400) { diff --git a/package.json b/package.json index fb0226d..7cd54fc 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,7 @@ "nock": "^9.0.13", "nsp": "^2.0.1", "proxyquire": "^1.8.0", - "restify": "^4.3.0", + "restify": "^7.2.1", "sinon": "^4.1.2" }, "dependencies": { diff --git a/test/index.js b/test/index.js index b214cc8..6b0618a 100644 --- a/test/index.js +++ b/test/index.js @@ -127,14 +127,16 @@ function sendWhitespace(req, res, next) { } function sendJsonZero(req, res, next) { - res.header('content-type', 'json'); - res.send(200, 0); + res.header('content-type', 'application/json'); + // to avoid any issues or bugs with restify formatters, call send raw + res.sendRaw('0'); next(); } function sendJsonFalse(req, res, next) { - res.header('content-type', 'json'); - res.send(200, false); + res.header('content-type', 'application/json'); + // to avoid any issues or bugs with restify formatters, call send raw + res.sendRaw('false'); next(); } @@ -152,8 +154,9 @@ function sendJsonInvalid500(req, res, next) { } function sendJsonNull(req, res, next) { - res.header('content-type', 'json'); - res.send(200, null); + res.header('content-type', 'application/json'); + // to avoid any issues or bugs with restify formatters, call send raw + res.sendRaw('null'); next(); } @@ -173,18 +176,6 @@ function getLog(name, stream, level) { })); } -function dtrace() { - var dtp; - - try { - var d = require('dtrace-provider'); - dtp = d.createDTraceProvider('restifyUnitTest'); - } catch (e) { - dtp = null; - } - return (dtp); -} - // --- Tests describe('restify-client tests', function () { @@ -192,17 +183,18 @@ describe('restify-client tests', function () { before(function (callback) { try { SERVER = restify.createServer({ - dtrace: dtrace, + // restify 7.x router has max param url char length of 100 + maxParamLength: 200, log: getLog('server'), handleUncaughtExceptions: false }); - SERVER.use(restify.acceptParser(['json', 'text/plain'])); - SERVER.use(restify.jsonp()); // Added for GH-778 - SERVER.use(restify.dateParser()); - SERVER.use(restify.authorizationParser()); - SERVER.use(restify.queryParser()); - SERVER.use(restify.bodyParser()); + SERVER.use(restify.plugins.acceptParser(['json', 'text/plain'])); + SERVER.use(restify.plugins.jsonp()); // Added for GH-778 + SERVER.use(restify.plugins.dateParser()); + SERVER.use(restify.plugins.authorizationParser()); + SERVER.use(restify.plugins.queryParser({ mapParams: true })); + SERVER.use(restify.plugins.bodyParser({ mapParams: true })); SERVER.get('/signed', sendSignature); SERVER.get('/whitespace/:count', sendWhitespace); @@ -236,6 +228,11 @@ describe('restify-client tests', function () { SERVER.del('/json/:name', sendJson); SERVER.opts('/json/:name', sendJson); + // we can reuse the sendJson* handlers when the string client makes + // a request against this endpoint - it will default to plain text. + SERVER.get('/str/zero', sendJsonZero); + SERVER.get('/str/false', sendJsonFalse); + SERVER.get('/str/null', sendJsonNull); SERVER.del('/str/:name', sendText); SERVER.get('/str/:name', sendText); SERVER.head('/str/:name', sendText); @@ -266,19 +263,16 @@ describe('restify-client tests', function () { JSON_CLIENT = clients.createJsonClient({ url: 'http://127.0.0.1:' + PORT, - dtrace: dtrace, retry: false, followRedirects: true }); STR_CLIENT = clients.createStringClient({ url: 'http://127.0.0.1:' + PORT, - dtrace: dtrace, retry: false, followRedirects: true }); RAW_CLIENT = clients.createClient({ url: 'http://127.0.0.1:' + PORT, - dtrace: dtrace, retry: false, headers: { accept: 'text/plain' @@ -286,7 +280,6 @@ describe('restify-client tests', function () { }); SAFE_STRINGIFY_CLIENT = clients.createJsonClient({ url: 'http://127.0.0.1:' + PORT, - dtrace: dtrace, retry: false, followRedirects: true, safeStringify: true @@ -691,7 +684,7 @@ describe('restify-client tests', function () { assert.ok(req); assert.ok(res); assert.strictEqual(res.body, '0'); - assert.strictEqual(obj, '0'); + assert.strictEqual(obj, 0); done(); }); }); @@ -703,7 +696,7 @@ describe('restify-client tests', function () { assert.ok(req); assert.ok(res); assert.strictEqual(res.body, 'false'); - assert.strictEqual(obj, 'false'); + assert.strictEqual(obj, false); done(); }); }); @@ -715,7 +708,43 @@ describe('restify-client tests', function () { assert.ok(req); assert.ok(res); assert.strictEqual(res.body, 'null'); - assert.strictEqual(obj, 'null'); + assert.strictEqual(obj, null); + done(); + }); + }); + + + it('GET string 0', function (done) { + STR_CLIENT.get('/str/zero', function (err, req, res, data) { + assert.ifError(err); + assert.ok(req); + assert.ok(res); + assert.strictEqual(res.body, '0'); + assert.strictEqual(data, '0'); + done(); + }); + }); + + + it('GET string false', function (done) { + STR_CLIENT.get('/str/false', function (err, req, res, data) { + assert.ifError(err); + assert.ok(req); + assert.ok(res); + assert.strictEqual(res.body, 'false'); + assert.strictEqual(data, 'false'); + done(); + }); + }); + + + it('GET string null', function (done) { + STR_CLIENT.get('/str/null', function (err, req, res, data) { + assert.ifError(err); + assert.ok(req); + assert.ok(res); + assert.strictEqual(res.body, 'null'); + assert.strictEqual(data, 'null'); done(); }); }); @@ -1364,7 +1393,6 @@ describe('restify-client tests', function () { it('should respect custom maxRedirects', function (done) { var client = clients.createStringClient({ url: 'http://127.0.0.1:' + PORT, - dtrace: dtrace, retry: false, followRedirects: true, maxRedirects: 2