Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

breaking: support parsing of valid falsy json #183

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

breaking: support parsing of valid falsy json #183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/JsonClient.js
View file
Open in desktop
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Expand Up @@ -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": {
Expand Down
94 changes: 61 additions & 33 deletions test/index.js
View file
Open in desktop
Expand Up @@ -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();
}

Expand All @@ -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();
}

Expand All @@ -173,36 +176,25 @@ 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 () {

before(function (callback) {
try {
SERVER = restify.createServer({
dtrace: dtrace,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7.x no longer takes dtrace as a function, it's now a boolean. Removed as it wasn't super important for the client tests.

// 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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -266,27 +263,23 @@ 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'
}
});
SAFE_STRINGIFY_CLIENT = clients.createJsonClient({
url: 'http://127.0.0.1:' + PORT,
dtrace: dtrace,
retry: false,
followRedirects: true,
safeStringify: true
Expand Down Expand Up @@ -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();
});
});
Expand All @@ -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();
});
});
Expand All @@ -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();
});
});
Expand Down Expand Up @@ -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
Expand Down