Skip to content

Commit

Permalink
Add Joi-based request validation to BaseJsonService and rewrite [NPM]…
Browse files Browse the repository at this point in the history 
… and [node] badges (#1743)

When JSON responses come back, they are sometimes not in the format expected by the API. As a result we have a lot of defensive coding (expressions like `(data || {}).someProperty`) to avoid exceptions being thrown in badge processing. Often we rely on the `try` blocks that wrap so much of the badge-processing code, which catch all JavaScript exceptions and return some error result, usually **invalid**. The problem with this is that these `try` blocks catch all sorts of programmer errors too, so when we see **invalid** we don't know whether the API returned something unexpected, or we've made a mistake. We also spend a lot of time writing defensive tests around malformed responses, and creating and maintaining the defensive coding.

A better solution is to validate the API responses using declarative contracts. Here the programmer says exactly what they expect from the API. That way, if the response isn't what we expect we can just say it's an **invalid json response**. And if our code then throws an exception, well that's our mistake; when we catch that we can call it a **shields internal error**. It's also less code and less error-prone. Over time we may be confident enough in the contracts that we won't need so many tests of malformed responses. The contract doesn't need to describe the entire response, only the part that's needed. Unknown keys can simply be dropped, preventing unvalidated parts of the response from creeping into the code. Checking what's in our response before calling values on it also makes our code more secure.

I used Joi here, since we're already using it for testing. There may be another contracts library that's a better fit, though I think we could look at that later.

Those changes are in base.js.

The rest is a rewrite of the remaining NPM badges, including the extraction of an NpmBase class. Inspired by @chris48s's work in #1740, this class splits the service concerns into fetching, validation, transformation, and rendering. This is treated as a design pattern. See the PR discussion for more. There are two URL patterns, one which allows specifying a tag (used by e.g. the version badge `https://img.shields.io/npm/v/npm/next.svg`), and the other which does not accept a tag (e.g. the license badge `https://img.shields.io/npm/l/express.svg`). Subclasses like NpmLicense and NpmTypeDefinitions can specify the URL fragment, examples, the validation schema for the chunk of the package data they use, and a render function. The NpmVersion subclass uses a different endpoint, so it overrides the `handle` implementation from NpmBase.

The remaining services using BaseJsonService are shimmed, so they will keep working after the changes.
  • Loading branch information
@paulmelnikow
paulmelnikow authored Aug 8, 2018
1 parent e1affea commit e3b1005
Show file tree
Hide file tree
Showing 25 changed files with 1,126 additions and 1,293 deletions.
1,271 changes: 404 additions & 867 deletions lib/all-badge-examples.js
View file

Large diffs are not rendered by default.

46 changes: 0 additions & 46 deletions lib/npm-badge-helpers.js
View file

This file was deleted.

10 changes: 0 additions & 10 deletions lib/npm-badge-helpers.spec.js
View file

This file was deleted.

40 changes: 0 additions & 40 deletions lib/npm-provider.js
View file

This file was deleted.

12 changes: 3 additions & 9 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"glob": "^7.1.1",
"gm": "^1.23.0",
"is-css-color": "^1.0.0",
"joi": "^13.3.0",
"js-yaml": "^3.11.0",
"json-autosave": "~1.1.2",
"jsonpath": "~1.0.0",
Expand Down Expand Up @@ -132,7 +133,6 @@
"icedfrisby-nock": "^1.0.0",
"is-png": "^1.1.0",
"is-svg": "^3.0.0",
"joi": "^13.3.0",
"lodash.debounce": "^4.0.8",
"lodash.difference": "^4.5.0",
"lodash.mapvalues": "^4.6.0",
Expand Down
214 changes: 0 additions & 214 deletions server.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,6 @@ const {
const {
isSnapshotVersion: isNexusSnapshotVersion,
} = require('./lib/nexus-version');
const {
mapNpmDownloads,
} = require('./lib/npm-provider');
const {
defaultNpmRegistryUri,
makePackageDataUrl: makeNpmPackageDataUrl,
} = require('./lib/npm-badge-helpers');
const {
teamcityBadge,
} = require('./lib/teamcity-badge-helpers');
Expand Down Expand Up @@ -1714,213 +1707,6 @@ cache(function(data, match, sendBadge, request) {
});
}));

// npm weekly download integration.
mapNpmDownloads({ camp, cache }, 'dw', 'last-week');

// npm monthly download integration.
mapNpmDownloads({ camp, cache }, 'dm', 'last-month');

// npm yearly download integration
mapNpmDownloads({ camp, cache }, 'dy', 'last-year');

// npm total download integration.
camp.route(/^\/npm\/dt\/(.*)\.(svg|png|gif|jpg|json)$/,
cache(function (data, match, sendBadge, request) {
var pkg = encodeURIComponent(match[1]); // eg, "express" or "@user/express"
var format = match[2];
var apiUrl = 'https://api.npmjs.org/downloads/range/1000-01-01:3000-01-01/' + pkg; // use huge range, will need to fix this in year 3000 :)
var badgeData = getBadgeData('downloads', data);
request(apiUrl, function (err, res, buffer) {
if (checkErrorResponse(badgeData, err, res)) {
sendBadge(format, badgeData);
return;
}
try {
var totalDownloads = 0;

var downloads = JSON.parse(buffer).downloads || 0;
for (var index = 0; index < downloads.length; index++) {
totalDownloads = totalDownloads + downloads[index].downloads;
}

badgeData.text[1] = metric(totalDownloads);
if (totalDownloads === 0) {
badgeData.colorscheme = 'red';
} else {
badgeData.colorscheme = 'brightgreen';
}
sendBadge(format, badgeData);
} catch (e) {
badgeData.text[1] = 'invalid';
sendBadge(format, badgeData);
}
});
}));

// npm version integration.
camp.route(/^\/npm\/v\/(?:@([^/]+))?\/?([^/]*)\/?([^/]*)\.(svg|png|gif|jpg|json)$/,
cache({
queryParams: ['registry_uri'],
handler: function(queryParams, match, sendBadge, request) {
// e.g. cycle, core, next, svg
const [, scope, packageName, tag, format] = match;
const registryUri = queryParams.registry_uri || defaultNpmRegistryUri;
const pkg = encodeURIComponent(scope ? `@${scope}/${packageName}` : packageName);
const apiUrl = `${registryUri}/-/package/${pkg}/dist-tags`;

const name = tag ? `npm@${tag}` : 'npm';
const badgeData = getBadgeData(name, queryParams);
// Using the Accept header because of this bug:
// <https://github.com/npm/npmjs.org/issues/163>
request(apiUrl, { headers: { 'Accept': '*/*' } }, (err, res, buffer) => {
if (err != null) {
badgeData.text[1] = 'inaccessible';
sendBadge(format, badgeData);
return;
}
try {
const data = JSON.parse(buffer);
const version = data[tag || 'latest'];
badgeData.text[1] = versionText(version);
badgeData.colorscheme = versionColor(version);
sendBadge(format, badgeData);
} catch(e) {
badgeData.text[1] = 'invalid';
sendBadge(format, badgeData);
}
});
},
}));

// npm license integration.
camp.route(/^\/npm\/l\/(?:@([^/]+)\/)?([^/]+)\.(svg|png|gif|jpg|json)$/,
cache({
queryParams: ['registry_uri'],
handler: function(queryParams, match, sendBadge, request) {
// e.g. cycle, core, svg
const [, scope, packageName, format ] = match;
const apiUrl = makeNpmPackageDataUrl({
registryUrl: queryParams.registry_uri,
scope,
packageName,
});
const badgeData = getBadgeData('license', queryParams);
request(apiUrl, { headers: { 'Accept': '*/*' } }, function(err, res, buffer) {
if (err != null) {
badgeData.text[1] = 'inaccessible';
sendBadge(format, badgeData);
return;
}
if (res.statusCode === 404) {
badgeData.text[1] = 'package not found';
sendBadge(format, badgeData);
return;
}
try {
const data = JSON.parse(buffer);
let license;
if (scope === undefined) {
license = data.license;
} else {
const latestVersion = data['dist-tags'].latest;
license = data.versions[latestVersion].license;
}
if (Array.isArray(license)) {
license = license.join(', ');
} else if (typeof license == 'object') {
license = license.type;
}
if (license === undefined) {
badgeData.text[1] = 'missing';
badgeData.colorscheme = 'red';
} else {
badgeData.text[1] = license;
setBadgeColor(badgeData, licenseToColor(license));
}
sendBadge(format, badgeData);
} catch(e) {
badgeData.text[1] = 'invalid';
sendBadge(format, badgeData);
}
});
},
}));

// npm node version integration.
camp.route(/^\/node\/v\/(?:@([^/]+))?\/?([^/]*)\/?([^/]*)\.(svg|png|gif|jpg|json)$/,
cache({
queryParams: ['registry_uri'],
handler: function(queryParams, match, sendBadge, request) {
// e.g. @stdlib, stdlib, next, svg
const [, scope, packageName, tag, format] = match;
const apiUrl = makeNpmPackageDataUrl({
registryUrl: queryParams.registry_uri,
scope,
packageName,
});
const registryTag = tag || 'latest';
const name = tag ? `node@${tag}` : 'node';
const badgeData = getBadgeData(name, queryParams);
// Using the Accept header because of this bug:
// <https://github.com/npm/npmjs.org/issues/163>
request(apiUrl, { headers: { 'Accept': '*/*' } }, (err, res, buffer) => {
if (checkErrorResponse(badgeData, err, res, 'package not found')) {
sendBadge(format, badgeData);
return;
}
try {
const data = JSON.parse(buffer);
let releaseData;
if (scope === undefined) {
releaseData = data;
} else {
const version = data['dist-tags'][registryTag];
releaseData = data.versions[version];
}
const versionRange = (releaseData.engines || {}).node;
if (! versionRange) {
badgeData.text[1] = 'not specified';
sendBadge(format, badgeData);
return;
}
badgeData.text[1] = versionRange;
regularUpdate({
url: 'http://nodejs.org/dist/latest/SHASUMS256.txt',
intervalMillis: 24 * 3600 * 1000,
json: false,
scraper: shasums => {
// tarball index start, tarball index end
const taris = shasums.indexOf('node-v');
const tarie = shasums.indexOf('\n', taris);
const tarball = shasums.slice(taris, tarie);
const version = tarball.split('-')[1];
return version;
},
}, (err, version) => {
if (err != null) {
badgeData.text[1] = 'invalid';
sendBadge(format, badgeData);
return;
}
try {
if (semver.satisfies(version, versionRange)) {
badgeData.colorscheme = 'brightgreen';
} else if (semver.gtr(version, versionRange)) {
badgeData.colorscheme = 'yellow';
} else {
badgeData.colorscheme = 'orange';
}
} catch(e) { }
sendBadge(format, badgeData);
});
} catch(e) {
badgeData.text[1] = 'invalid';
sendBadge(format, badgeData);
}
});
},
}));

// Anaconda Cloud / conda package manager integration
camp.route(/^\/conda\/([dvp]n?)\/([^/]+)\/([^/]+)\.(svg|png|gif|jpg|json)$/,
cache(function(queryData, match, sendBadge, request) {
Expand Down
Loading

0 comments on commit e3b1005

Please sign in to comment.