diff --git a/CHANGELOG.md b/CHANGELOG.md index f7549cb37c..d3c6deabf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ## Unreleased +### Added +* [`jsx-no-target-blank`]: add fixer ([#2862][] @Nokel81) + ### Fixed * [`jsx-no-constructed-context-values`]: avoid a crash with `as X` TS code ([#2894][] @ljharb) * [`jsx-no-constructed-context-values`]: avoid a crash with boolean shorthand ([#2895][] @ljharb) @@ -15,6 +18,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel [#2895]: https://github.com/yannickcr/eslint-plugin-react/issues/2895 [#2894]: https://github.com/yannickcr/eslint-plugin-react/issues/2894 [#2893]: https://github.com/yannickcr/eslint-plugin-react/pull/2893 +[#2862]: https://github.com/yannickcr/eslint-plugin-react/pull/2862 ## [7.22.0] - 2020.12.29 diff --git a/lib/rules/jsx-no-target-blank.js b/lib/rules/jsx-no-target-blank.js index 656cd3009f..1c7b6a7d98 100644 --- a/lib/rules/jsx-no-target-blank.js +++ b/lib/rules/jsx-no-target-blank.js @@ -12,22 +12,28 @@ const linkComponentsUtil = require('../util/linkComponents'); // Rule Definition // ------------------------------------------------------------------------------ -function lastIndexMatching(arr, condition) { - return arr.map(condition).lastIndexOf(true); +function findLastIndex(arr, condition) { + for (let i = arr.length - 1; i >= 0; i -= 1) { + if (condition(arr[i])) { + return i; + } + } + + return -1; } function attributeValuePossiblyBlank(attribute) { - if (!attribute.value) { + if (!attribute || !attribute.value) { return false; } const value = attribute.value; - if (value.type === 'Literal' && typeof value.value === 'string' && value.value.toLowerCase() === '_blank') { - return true; + if (value.type === 'Literal') { + return typeof value.value === 'string' && value.value.toLowerCase() === '_blank'; } if (value.type === 'JSXExpressionContainer') { const expr = value.expression; - if (expr.type === 'Literal' && typeof expr.value === 'string' && expr.value.toLowerCase() === '_blank') { - return true; + if (expr.type === 'Literal') { + return typeof expr.value === 'string' && expr.value.toLowerCase() === '_blank'; } if (expr.type === 'ConditionalExpression') { if (expr.alternate.type === 'Literal' && expr.alternate.value && expr.alternate.value.toLowerCase() === '_blank') { @@ -41,21 +47,15 @@ function attributeValuePossiblyBlank(attribute) { return false; } -function hasTargetBlank(node, warnOnSpreadAttributes, spreadAttributeIndex) { - const targetIndex = lastIndexMatching(node.attributes, (attr) => attr.name && attr.name.name === 'target'); - const foundTargetBlank = targetIndex !== -1 && attributeValuePossiblyBlank(node.attributes[targetIndex]); - return foundTargetBlank || (warnOnSpreadAttributes && targetIndex < spreadAttributeIndex); -} - function hasExternalLink(node, linkAttribute, warnOnSpreadAttributes, spreadAttributeIndex) { - const linkIndex = lastIndexMatching(node.attributes, (attr) => attr.name && attr.name.name === linkAttribute); + const linkIndex = findLastIndex(node.attributes, (attr) => attr.name && attr.name.name === linkAttribute); const foundExternalLink = linkIndex !== -1 && ((attr) => attr.value.type === 'Literal' && /^(?:\w+:|\/\/)/.test(attr.value.value))( node.attributes[linkIndex]); return foundExternalLink || (warnOnSpreadAttributes && linkIndex < spreadAttributeIndex); } function hasDynamicLink(node, linkAttribute) { - const dynamicLinkIndex = lastIndexMatching(node.attributes, (attr) => attr.name + const dynamicLinkIndex = findLastIndex(node.attributes, (attr) => attr.name && attr.name.name === linkAttribute && attr.value && attr.value.type === 'JSXExpressionContainer'); @@ -64,29 +64,36 @@ function hasDynamicLink(node, linkAttribute) { } } -function hasSecureRel(node, allowReferrer, warnOnSpreadAttributes, spreadAttributeIndex) { - const relIndex = lastIndexMatching(node.attributes, (attr) => (attr.type === 'JSXAttribute' && attr.name.name === 'rel')); +function getStringFromValue(value) { + if (value) { + if (value.type === 'Literal') { + return value.value; + } + if (value.type === 'JSXExpressionContainer') { + if (value.expression.type === 'TemplateLiteral') { + return value.expression.quasis[0].value.cooked; + } + return value.expression && value.expression.value; + } + } + return null; +} +function hasSecureRel(node, allowReferrer, warnOnSpreadAttributes, spreadAttributeIndex) { + const relIndex = findLastIndex(node.attributes, (attr) => (attr.type === 'JSXAttribute' && attr.name.name === 'rel')); if (relIndex === -1 || (warnOnSpreadAttributes && relIndex < spreadAttributeIndex)) { return false; } const relAttribute = node.attributes[relIndex]; - const value = relAttribute.value - && (( - relAttribute.value.type === 'Literal' - && relAttribute.value.value - ) || ( - relAttribute.value.type === 'JSXExpressionContainer' - && relAttribute.value.expression - && relAttribute.value.expression.value - )); + const value = getStringFromValue(relAttribute.value); const tags = value && typeof value === 'string' && value.toLowerCase().split(' '); return tags && (allowReferrer ? tags.indexOf('noopener') >= 0 : tags.indexOf('noreferrer') >= 0); } module.exports = { meta: { + fixable: 'code', docs: { description: 'Forbid `target="_blank"` attribute without `rel="noreferrer"`', category: 'Best Practices', @@ -123,9 +130,17 @@ module.exports = { return; } - const spreadAttributeIndex = lastIndexMatching(node.attributes, (attr) => (attr.type === 'JSXSpreadAttribute')); - if (!hasTargetBlank(node, warnOnSpreadAttributes, spreadAttributeIndex)) { - return; + const targetIndex = findLastIndex(node.attributes, (attr) => attr.name && attr.name.name === 'target'); + const spreadAttributeIndex = findLastIndex(node.attributes, (attr) => (attr.type === 'JSXSpreadAttribute')); + + if (!attributeValuePossiblyBlank(node.attributes[targetIndex])) { + const hasSpread = spreadAttributeIndex >= 0; + + if (warnOnSpreadAttributes && hasSpread) { + // continue to check below + } else if ((hasSpread && targetIndex < spreadAttributeIndex) || !hasSpread) { + return; + } } const linkAttribute = components.get(node.name.name); @@ -135,7 +150,48 @@ module.exports = { context.report({ node, message: 'Using target="_blank" without rel="noreferrer" ' - + 'is a security risk: see https://html.spec.whatwg.org/multipage/links.html#link-type-noopener' + + 'is a security risk: see https://html.spec.whatwg.org/multipage/links.html#link-type-noopener', + fix(fixer) { + // eslint 5 uses `node.attributes`; eslint 6+ uses `node.parent.attributes` + const nodeWithAttrs = node.parent.attributes ? node.parent : node; + // eslint 5 does not provide a `name` property on JSXSpreadElements + const relAttribute = nodeWithAttrs.attributes.find((attr) => attr.name && attr.name.name === 'rel'); + + if (targetIndex < spreadAttributeIndex || (spreadAttributeIndex >= 0 && !relAttribute)) { + return null; + } + + if (!relAttribute) { + return fixer.insertTextAfter(nodeWithAttrs.attributes.slice(-1)[0], ' rel="noreferrer"'); + } + + if (!relAttribute.value) { + return fixer.insertTextAfter(relAttribute, '="noreferrer"'); + } + + if (relAttribute.value.type === 'Literal') { + const parts = relAttribute.value.value + .split('noreferrer') + .filter(Boolean); + return fixer.replaceText(relAttribute.value, `"${parts.concat('noreferrer').join(' ')}"`); + } + + if (relAttribute.value.type === 'JSXExpressionContainer') { + if (relAttribute.value.expression.type === 'Literal') { + if (typeof relAttribute.value.expression.value === 'string') { + const parts = relAttribute.value.expression.value + .split('noreferrer') + .filter(Boolean); + return fixer.replaceText(relAttribute.value.expression, `"${parts.concat('noreferrer').join(' ')}"`); + } + + // for undefined, boolean, number, symbol, bigint, and null + return fixer.replaceText(relAttribute.value, '"noreferrer"'); + } + } + + return null; + } }); } } diff --git a/tests/lib/rules/jsx-no-target-blank.js b/tests/lib/rules/jsx-no-target-blank.js index dbe9a0e528..a4866eddf1 100644 --- a/tests/lib/rules/jsx-no-target-blank.js +++ b/tests/lib/rules/jsx-no-target-blank.js @@ -110,84 +110,155 @@ ruleTester.run('jsx-no-target-blank', rule, { code: '' } ], - invalid: [{ - code: '', - errors: defaultErrors - }, { - code: '', - errors: defaultErrors - }, { - code: '', - errors: defaultErrors - }, { - code: '', - errors: defaultErrors - }, { - code: '', - errors: defaultErrors - }, { - code: '', - errors: defaultErrors - }, { - code: '', - errors: defaultErrors - }, { - code: '', - errors: defaultErrors - }, { - code: '', - errors: defaultErrors - }, { - code: '', - errors: defaultErrors - }, { - code: '', - errors: defaultErrors - }, { - code: '', - errors: defaultErrors - }, { - code: '', - errors: defaultErrors - }, { - code: '', - errors: defaultErrors - }, { - code: '', - errors: defaultErrors - }, { - code: '', - options: [{enforceDynamicLinks: 'always'}], - errors: defaultErrors - }, { - code: '', - options: [{enforceDynamicLinks: 'always', warnOnSpreadAttributes: true}], - errors: defaultErrors - }, { - code: '', - options: [{enforceDynamicLinks: 'always', warnOnSpreadAttributes: true}], - errors: defaultErrors - }, { - code: '', - options: [{enforceDynamicLinks: 'always', warnOnSpreadAttributes: true}], - errors: defaultErrors - }, { - code: '', - options: [{enforceDynamicLinks: 'always', warnOnSpreadAttributes: true}], - errors: defaultErrors - }, { - code: '', - options: [{enforceDynamicLinks: 'always', warnOnSpreadAttributes: true}], - errors: defaultErrors - }, { - code: '', - options: [{enforceDynamicLinks: 'always'}], - settings: {linkComponents: ['Link']}, - errors: defaultErrors - }, { - code: '', - options: [{enforceDynamicLinks: 'always'}], - settings: {linkComponents: {name: 'Link', linkAttribute: 'to'}}, - errors: defaultErrors - }] + invalid: [ + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + errors: defaultErrors + }, + { + code: '', + output: '', + options: [{enforceDynamicLinks: 'always'}], + errors: defaultErrors + }, + { + code: '', + options: [{enforceDynamicLinks: 'always', warnOnSpreadAttributes: true}], + errors: defaultErrors + }, + { + code: '', + options: [{enforceDynamicLinks: 'always', warnOnSpreadAttributes: true}], + errors: defaultErrors + }, + { + code: '', + options: [{enforceDynamicLinks: 'always', warnOnSpreadAttributes: true}], + errors: defaultErrors + }, + { + code: '', + options: [{enforceDynamicLinks: 'always', warnOnSpreadAttributes: true}], + errors: defaultErrors + }, + { + code: '', + options: [{enforceDynamicLinks: 'always', warnOnSpreadAttributes: true}], + errors: defaultErrors + }, + { + code: '', + output: '', + options: [{enforceDynamicLinks: 'always'}], + settings: {linkComponents: ['Link']}, + errors: defaultErrors + }, + { + code: '', + output: '', + options: [{enforceDynamicLinks: 'always'}], + settings: {linkComponents: {name: 'Link', linkAttribute: 'to'}}, + errors: defaultErrors + } + ] });