Skip to content

Commit

Permalink
tools: add check for using process.binding crypto
Browse files Browse the repository at this point in the history
Currently, when configuring --without-ssl any tests that use
process.binding('crypto') will not report a lint warning. This is
because the eslint check only generates a warning when using require.

This commit adds a check for using binding in addition to require.

PR-URL: nodejs#17867
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
danbev committed Feb 27, 2018
1 parent 0776a43 commit 1a678a6
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 2 deletions.
2 changes: 2 additions & 0 deletions test/parallel/test-http2-util-headers-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
// to pass to the internal binding layer.

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const assert = require('assert');
const { mapToHeaders } = require('internal/http2/util');

Expand Down
4 changes: 3 additions & 1 deletion test/parallel/test-http2-util-update-options-buffer.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// Flags: --expose-internals
'use strict';

require('../common');
const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

// Test coverage for the updateOptionsBuffer method used internally
// by the http2 implementation.
Expand Down
7 changes: 6 additions & 1 deletion tools/eslint-rules/crypto-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,18 @@ const utils = require('./rules-utils.js');
const msg = 'Please add a hasCrypto check to allow this test to be skipped ' +
'when Node is built "--without-ssl".';

const cryptoModules = ['crypto', 'http2'];
const requireModules = cryptoModules.concat(['tls', 'https']);
const bindingModules = cryptoModules.concat(['tls_wrap']);

module.exports = function(context) {
const missingCheckNodes = [];
const requireNodes = [];
var hasSkipCall = false;

function testCryptoUsage(node) {
if (utils.isRequired(node, ['crypto', 'tls', 'https', 'http2'])) {
if (utils.isRequired(node, requireModules) ||
utils.isBinding(node, bindingModules)) {
requireNodes.push(node);
}
}
Expand Down
12 changes: 12 additions & 0 deletions tools/eslint-rules/rules-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,18 @@ module.exports.isRequired = function(node, modules) {
modules.includes(node.arguments[0].value);
};

/**
* Returns true if any of the passed in modules are used in
* binding calls.
*/
module.exports.isBinding = function(node, modules) {
if (node.callee.object) {
return node.callee.object.name === 'process' &&
node.callee.property.name === 'binding' &&
modules.includes(node.arguments[0].value);
}
};

/**
* Returns true is the node accesses any property in the properties
* array on the 'common' object.
Expand Down

0 comments on commit 1a678a6

Please sign in to comment.