diff --git a/CHANGELOG.md b/CHANGELOG.md index f7549cb37c..e87e60dbf5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel * [`jsx-no-constructed-context-values`]: add new rule which checks when the value passed to a Context Provider will cause needless rerenders ([#2763][] @dylanOshima) * [`jsx-indent-props`]: add `ignoreTernaryOperator` option ([#2846][] @SebastianZimmer) * [`jsx-no-target-blank`]: Add `warnOnSpreadAttributes` option ([#2855][] @michael-yx-wu) +* [`jsx-no-target-blank`]: add fixer ([#2862][] @Nokel81) ### Fixed * [`display-name`]/component detection: avoid a crash on anonymous components ([#2840][] @ljharb) @@ -56,6 +57,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel [#2871]: https://github.com/yannickcr/eslint-plugin-react/issues/2871 [#2870]: https://github.com/yannickcr/eslint-plugin-react/issues/2870 [#2869]: https://github.com/yannickcr/eslint-plugin-react/issues/2869 +[#2862]: https://github.com/yannickcr/eslint-plugin-react/pull/2862 [#2855]: https://github.com/yannickcr/eslint-plugin-react/pull/2855 [#2852]: https://github.com/yannickcr/eslint-plugin-react/pull/2852 [#2851]: https://github.com/yannickcr/eslint-plugin-react/issues/2851 diff --git a/lib/rules/jsx-no-target-blank.js b/lib/rules/jsx-no-target-blank.js index 656cd3009f..664036a625 100644 --- a/lib/rules/jsx-no-target-blank.js +++ b/lib/rules/jsx-no-target-blank.js @@ -12,50 +12,54 @@ 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 === 'JSXExpressionContainer') { - const expr = value.expression; - if (expr.type === 'Literal' && typeof expr.value === 'string' && expr.value.toLowerCase() === '_blank') { - return true; - } - if (expr.type === 'ConditionalExpression') { - if (expr.alternate.type === 'Literal' && expr.alternate.value && expr.alternate.value.toLowerCase() === '_blank') { - return true; - } - if (expr.consequent.type === 'Literal' && expr.consequent.value && expr.consequent.value.toLowerCase() === '_blank') { - return true; + switch (value.type) { + case 'Literal': + return typeof value.value === 'string' && value.value.toLowerCase() === '_blank'; + case 'JSXExpressionContainer': { + const expr = value.expression; + // eslint-disable-next-line default-case + switch (expr.type) { + case 'Literal': + return typeof expr.value === 'string' && expr.value.toLowerCase() === '_blank'; + case 'ConditionalExpression': + if (expr.alternate.type === 'Literal' && expr.alternate.value && expr.alternate.value.toLowerCase() === '_blank') { + return true; + } + if (expr.consequent.type === 'Literal' && expr.consequent.value && expr.consequent.value.toLowerCase() === '_blank') { + return true; + } } } + // eslint-disable-next-line no-fallthrough + default: + return false; } - 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 +68,37 @@ 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) { + switch (value && value.type) { + case 'Literal': + return value.value; + case 'JSXExpressionContainer': + switch (value.expression.type) { + case 'TemplateLiteral': + return value.expression.quasis[0].value.cooked; + default: + return value.expression && value.expression.value; + } + default: + 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 +135,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 +155,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 + } + ] });