Skip to content

Commit

Permalink
[Fix] jsx-no-target-blank: False negative when rel attribute is ass…
Browse files Browse the repository at this point in the history
…igned using ConditionalExpression
  • Loading branch information
V2dha authored and ljharb committed Jul 13, 2022
1 parent 337e41a commit b7f388b
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 9 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -21,6 +21,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
* [`display-name`], component detection: fix HOF returning null as Components ([#3347][] @jxm-math)
* [`forbid-prop-types`]: Ignore objects that are not of type React.PropTypes ([#3326][] @TildaDares)
* [`display-name`], component detection: fix false positive for HOF returning only nulls and literals ([#3305][] @golopot)
* [`jsx-no-target-blank`]: False negative when rel attribute is assigned using ConditionalExpression ([#3332][] @V2dha)

### Changed
* [Refactor] [`jsx-indent-props`]: improved readability of the checkNodesIndent function ([#3315][] @caroline223)
Expand All @@ -39,6 +40,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
[#3339]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3339
[#3338]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3338
[#3335]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3335
[#3332]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3332
[#3331]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3331
[#3328]: https://github.com/jsx-eslint/eslint-plugin-react/issues/3328
[#3327]: https://github.com/jsx-eslint/eslint-plugin-react/issues/3327
Expand Down
22 changes: 15 additions & 7 deletions lib/rules/jsx-no-target-blank.js
Expand Up @@ -74,7 +74,12 @@ function getStringFromValue(value) {
if (value.expression.type === 'TemplateLiteral') {
return value.expression.quasis[0].value.cooked;
}
return value.expression && value.expression.value;
const expr = value.expression;
return expr && (
expr.type === 'ConditionalExpression'
? [expr.consequent.value, expr.alternate.value]
: expr.value
);
}
}
return null;
Expand All @@ -88,12 +93,15 @@ function hasSecureRel(node, allowReferrer, warnOnSpreadAttributes, spreadAttribu

const relAttribute = node.attributes[relIndex];
const value = getStringFromValue(relAttribute.value);
const tags = value && typeof value === 'string' && value.toLowerCase().split(' ');
const noreferrer = tags && tags.indexOf('noreferrer') >= 0;
if (noreferrer) {
return true;
}
return allowReferrer && tags && tags.indexOf('noopener') >= 0;
return [].concat(value).every((item) => {
const tags = typeof item === 'string' ? item.toLowerCase().split(' ') : false;
const noreferrer = tags && tags.indexOf('noreferrer') >= 0;
if (noreferrer) {
return true;
}
const noopener = tags && tags.indexOf('noopener') >= 0;
return allowReferrer && noopener;
});
}

const messages = {
Expand Down
51 changes: 49 additions & 2 deletions tests/lib/rules/jsx-no-target-blank.js
Expand Up @@ -28,6 +28,7 @@ const parserOptions = {

const ruleTester = new RuleTester({ parserOptions });
const defaultErrors = [{ messageId: 'noTargetBlankWithoutNoreferrer' }];
const allowReferrerErrors = [{ messageId: 'noTargetBlankWithoutNoopener' }];

ruleTester.run('jsx-no-target-blank', rule, {
valid: parsers.all([
Expand Down Expand Up @@ -141,6 +142,19 @@ ruleTester.run('jsx-no-target-blank', rule, {
{
code: '<a href target="_blank"/>',
},
{
code: '<a href={href} target={isExternal ? "_blank" : undefined} rel="noopener noreferrer" />',
},
{
code: '<a href={href} target={isExternal ? undefined : "_blank"} rel={isExternal ? "noreferrer" : "noopener noreferrer"} />',
},
{
code: '<a href={href} target={isExternal ? undefined : "_blank"} rel={isExternal ? "noreferrer noopener" : "noreferrer"} />',
},
{
code: '<a href={href} target="_blank" rel={isExternal ? "noreferrer" : "noopener"} />',
options: [{ allowReferrer: true }],
},
]),
invalid: parsers.all([
{
Expand Down Expand Up @@ -251,13 +265,13 @@ ruleTester.run('jsx-no-target-blank', rule, {
code: '<a href="https://example.com/20" target="_blank" rel></a>',
output: '<a href="https://example.com/20" target="_blank" rel="noopener"></a>',
options: [{ allowReferrer: true }],
errors: [{ messageId: 'noTargetBlankWithoutNoopener' }],
errors: allowReferrerErrors,
},
{
code: '<a href="https://example.com/20" target="_blank"></a>',
output: '<a href="https://example.com/20" target="_blank" rel="noopener"></a>',
options: [{ allowReferrer: true }],
errors: [{ messageId: 'noTargetBlankWithoutNoopener' }],
errors: allowReferrerErrors,
},
{
code: '<a target="_blank" href={ dynamicLink }></a>',
Expand Down Expand Up @@ -352,5 +366,38 @@ ruleTester.run('jsx-no-target-blank', rule, {
options: [{ forms: true, links: false }],
errors: defaultErrors,
},
{
code: '<a href={href} target="_blank" rel={isExternal ? "undefined" : "undefined"} />',
errors: defaultErrors,
},
{
code: '<a href={href} target="_blank" rel={isExternal ? "noopener" : undefined} />',
errors: defaultErrors,
},
{
code: '<a href={href} target="_blank" rel={isExternal ? "undefined" : "noopener"} />',
errors: defaultErrors,
},
{
code: '<a href={href} target={isExternal ? "_blank" : undefined} rel={isExternal ? "noopener noreferrer" : undefined} />',
errors: defaultErrors,
},
{
code: '<a href={href} target={isExternal ? "_blank" : undefined} rel={isExternal ? undefined : "noopener noreferrer"} />',
errors: defaultErrors,
},
{
code: '<a href={href} target="_blank" rel={isExternal ? 3 : "noopener noreferrer"} />',
errors: defaultErrors,
},
{
code: '<a href={href} target="_blank" rel={isExternal ? "noopener noreferrer" : "3"} />',
errors: defaultErrors,
},
{
code: '<a href={href} target="_blank" rel={isExternal ? "noopener" : "2"} />',
options: [{ allowReferrer: true }],
errors: allowReferrerErrors,
},
]),
});

0 comments on commit b7f388b

Please sign in to comment.