From ee07395eae8b0e9de6a5d4ec5c74cd71f3ecbe6a Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Wed, 23 Sep 2015 12:32:09 -0700 Subject: [PATCH] joyent/node-triton#30 `triton` commands blow up obtusely if getting HTML content back from cloudapi endpoints Fixes #30 --- lib/SaferJsonClient.js | 224 +++++++++++++++++++++++++++++++++++++++++ lib/cloudapi2.js | 4 +- package.json | 4 +- 3 files changed, 229 insertions(+), 3 deletions(-) create mode 100644 lib/SaferJsonClient.js diff --git a/lib/SaferJsonClient.js b/lib/SaferJsonClient.js new file mode 100644 index 0000000..092ef6d --- /dev/null +++ b/lib/SaferJsonClient.js @@ -0,0 +1,224 @@ +/* + * Copyright 2012 Mark Cavage, Inc. All rights reserved. + * Copyright (c) 2015, Joyent, Inc. + */ + +/* + * TODO: this should be a separate module. Both node-triton and + * node-docker-registry-client are using (slightly different versions of) this. + * + * Adapted from + * + * now at + * + * This subclasses the Restify StringClient to add the following features: + * + * 1. Extend the callback from + * callback(err, req, res, ); + * to: + * callback(err, req, res, , ); + * This allows one to work on the raw body for special case handling, if + * wanted. I'm not sure I'd propose this for restify core because it + * shouldn't add features that make it harder to go all streaming. + * + * 2. In restify.JsonClient, if the body is not parseable JSON, it log.trace's + * an error, and returns `{}` (see mcavage/restify#388). I don't particularly + * like that because it is ambiguous (and also disallows returning a JSON + * body that is false-y: `false`, `0`, `null`). + * + * Instead this client will do the following: + * (a) If the response is an error status (>=400), then return `undefined` + * for the body. This allows the caller to know if the body was parsed + * because `undefined` is not representable in JSON. + * (b) If the response is a success (<400), then return an + * InvalidContentError restify error. + * + * (TODO: I'd support this for restify code, but it *is* backward + * incompatible.) + * + * 3. `.write()` doesn't default a null `body` to `{}`. + * This change isn't because I came across the need for it, but because that + * just seems wrong. + * + * 4. Doesn't set `res.body` which restify's StringClient.parse seems to do + * ... as an accident of history I'm guessing? + */ + +/* jsl:ignore */ +'use strict'; +/* jsl:end */ + +var assert = require('assert-plus'); +var crypto = require('crypto'); +var strsplit = require('strsplit').strsplit; +var util = require('util'); +var zlib = require('zlib'); + +var errors = require('restify-errors'); +var codeToHttpError = errors.codeToHttpError; +var RestError = errors.RestError; +var StringClient = require('restify-clients').StringClient; + + +// --- API + +function SaferJsonClient(options) { + assert.object(options, 'options'); + + options.accept = 'application/json'; + options.name = options.name || 'SaferJsonClient'; + options.contentType = 'application/json'; + + StringClient.call(this, options); + + this._super = StringClient.prototype; +} +util.inherits(SaferJsonClient, StringClient); + + +SaferJsonClient.prototype.write = function write(options, body, callback) { + assert.object(body, 'body'); + + // This is change #3. + var resBody = JSON.stringify(body); + return (this._super.write.call(this, options, resBody, callback)); +}; + + +SaferJsonClient.prototype.parse = function parse(req, callback) { + function parseResponse(err, res) { + var chunks = []; // gunzipped response chunks (Buffer objects) + var len = 0; // accumulated count of chunk lengths + var contentMd5; + var contentMd5Hash; + var gz; + var resErr = err; + + function finish() { + var body = Buffer.concat(chunks, len); + if (res.log.trace()) { + res.log.trace({body: body.toString(), len: len}, + 'body received'); + } + + // Content-Length check + var contentLength = Number(res.headers['content-length']); + if (!isNaN(contentLength) && len !== contentLength) { + resErr = new errors.InvalidContentError(util.format( + 'Incomplete content: Content-Length:%s but got %s bytes', + contentLength, len)); + callback(resErr, req, res); + return; + } + + // Content-MD5 check. + if (contentMd5Hash && + contentMd5 !== contentMd5Hash.digest('base64')) + { + resErr = new errors.BadDigestError('Content-MD5'); + callback(resErr, req, res); + return; + } + + // Parse the body as JSON, if we can. + // Note: This regex-based trim works on a buffer. `trim()` doesn't. + var obj; + if (len && !/^\s*$/.test(body)) { // Skip all-whitespace body. + try { + obj = JSON.parse(body); + } catch (jsonErr) { + res.log.trace(jsonErr, 'Invalid JSON in response'); + if (!resErr) { + // TODO: Does this mask other error statuses? + resErr = new errors.InvalidContentError( + 'Invalid JSON in response'); + } + } + } + + // Special error handling. + if (resErr) { + resErr.message = body.toString('utf8'); + } + if (res && res.statusCode >= 400) { + // Upcast error to a RestError (if we can) + // Be nice and handle errors like + // { error: { code: '', message: '' } } + // in addition to { code: '', message: '' }. + if (obj && (obj.code || (obj.error && obj.error.code))) { + var _c = obj.code || + (obj.error ? obj.error.code : '') || + ''; + var _m = obj.message || + (obj.error ? obj.error.message : '') || + ''; + + resErr = new RestError({ + message: _m, + restCode: _c, + statusCode: res.statusCode + }); + resErr.name = resErr.restCode; + + if (!/Error$/.test(resErr.name)) { + resErr.name += 'Error'; + } + } else if (!resErr) { + resErr = codeToHttpError(res.statusCode, + obj.message || '', body); + } + } + if (resErr) { + resErr.body = obj; + } + + callback(resErr, req, res, obj, body); + } + + + if (!res) { + // Early out if we didn't even get a response. + callback(resErr, req); + return; + } + + // Content-MD5 setup. + contentMd5 = res.headers['content-md5']; + if (contentMd5 && req.method !== 'HEAD' && res.statusCode !== 206) { + contentMd5Hash = crypto.createHash('md5'); + } + + if (res.headers['content-encoding'] === 'gzip') { + gz = zlib.createGunzip(); + gz.on('data', function (chunk) { + chunks.push(chunk); + len += chunk.length; + }); + gz.once('end', finish); + res.once('end', gz.end.bind(gz)); + } else { + res.once('end', finish); + } + + res.on('data', function onData(chunk) { + if (contentMd5Hash) { + contentMd5Hash.update(chunk.toString('utf8')); + } + + if (gz) { + gz.write(chunk); + } else { + chunks.push(chunk); + len += chunk.length; + } + }); + } + + return (parseResponse); +}; + + + +// --- Exports + +module.exports = SaferJsonClient; diff --git a/lib/cloudapi2.js b/lib/cloudapi2.js index 55301c4..d83e61b 100644 --- a/lib/cloudapi2.js +++ b/lib/cloudapi2.js @@ -36,10 +36,10 @@ var format = require('util').format; var LOMStream = require('lomstream').LOMStream; var os = require('os'); var querystring = require('querystring'); -var restifyClients = require('restify-clients'); var vasync = require('vasync'); var errors = require('./errors'); +var SaferJsonClient = require('./SaferJsonClient'); @@ -122,7 +122,7 @@ function CloudApi(options) { // XXX relevant? //this.token = options.token; - this.client = restifyClients.createJsonClient(options); + this.client = new SaferJsonClient(options); } diff --git a/package.json b/package.json index 3a4129f..6f91663 100644 --- a/package.json +++ b/package.json @@ -16,8 +16,10 @@ "mkdirp": "0.5.1", "node-uuid": "1.4.3", "once": "1.3.2", - "restify-clients": "1.0.0", + "restify-clients": "1.1.0", + "restify-errors": "3.0.0", "smartdc-auth": "git+https://github.com/joyent/node-smartdc-auth.git#3be3c1e", + "strsplit": "1.0.0", "tabula": "1.6.1", "vasync": "1.6.3", "verror": "1.6.0"