Skip to content

Commit 7844d8e

Browse files
cutifulljharb
authored andcommitted
[Fix] jsx-no-target-blank: improve error messages
Show different error messages depending on whether referrer is allowed; clarify about `noreferrer` only being necessary in older browsers. Closes #3044.
1 parent 8785c16 commit 7844d8e

File tree

4 files changed

+14
-7
lines changed

4 files changed

+14
-7
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
88
### Fixed
99
* [`no-namespace`]: fix crash on non-string React.createElement name ([#3082] @ljharb)
1010
* [`no-namespace`]: avoid crash on non-string createElement values ([#3085] @ljharb)
11+
* [`jsx-no-target-blank`]: improve error messages ([#3088] @cutiful)
1112

1213
### Changed
1314
* [Docs] [`jsx-max-props-per-line`]: fix options example ([#3083] @MrRaiter)
1415

16+
[#3088]: https://github.com/yannickcr/eslint-plugin-react/pull/3088
1517
[#3085]: https://github.com/yannickcr/eslint-plugin-react/issue/3085
1618
[#3083]: https://github.com/yannickcr/eslint-plugin-react/pull/3083
1719
[#3082]: https://github.com/yannickcr/eslint-plugin-react/pull/3082

docs/rules/jsx-no-target-blank.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ This rule aims to prevent user generated link hrefs and form actions from creati
2020
...
2121
```
2222

23-
* `allowReferrer`: optional boolean. If `true` does not require `noreferrer`. Defaults to `false`.
24-
* `enabled`: for enabling the rule. 0=off, 1=warn, 2=error. Defaults to 0.
23+
* `allowReferrer`: optional boolean. If `true` does not require `noreferrer` (i. e. `noopener` alone is enough, this leaves IE vulnerable). Defaults to `false`.
24+
* `enabled`: for enabling the rule.
2525
* `enforceDynamicLinks`: optional string, 'always' or 'never'
2626
* `warnOnSpreadAttributes`: optional boolean. Defaults to `false`.
2727
* `enforceDynamicLinks` - enforce: optional string, 'always' or 'never'
@@ -125,6 +125,8 @@ This rule supports the ability to use custom components for forms. To enable thi
125125

126126
For links to a trusted host (e.g. internal links to your own site, or links to a another host you control, where you can be certain this security vulnerability does not exist), you may want to keep the HTTP Referer header for analytics purposes.
127127

128+
If you do not support Internet Explorer (any version), Chrome < 49, Opera < 36, Firefox < 52, desktop Safari < 10.1 or iOS Safari < 10.3, you may set `allowReferrer` to `true`, keep the HTTP Referer header and only add `rel="noopener"` to your links.
129+
128130
## When Not To Use It
129131

130132
If you do not have any external links or forms, you can disable this rule.

lib/rules/jsx-no-target-blank.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ function hasSecureRel(node, allowReferrer, warnOnSpreadAttributes, spreadAttribu
9797
}
9898

9999
const messages = {
100-
noTargetBlank: 'Using target="_blank" without rel="noreferrer" is a security risk: see https://html.spec.whatwg.org/multipage/links.html#link-type-noopener'
100+
noTargetBlankWithoutNoreferrer: 'Using target="_blank" without rel="noreferrer" (which implies rel="noopener") is a security risk in older browsers: see https://mathiasbynens.github.io/rel-noopener/#recommendations',
101+
noTargetBlankWithoutNoopener: 'Using target="_blank" without rel="noreferrer" or rel="noopener" (the former implies the latter and is preferred due to wider support) is a security risk: see https://mathiasbynens.github.io/rel-noopener/#recommendations'
101102
};
102103

103104
module.exports = {
@@ -173,7 +174,8 @@ module.exports = {
173174
const hasDangerousLink = hasExternalLink(node, linkAttribute, warnOnSpreadAttributes, spreadAttributeIndex)
174175
|| (enforceDynamicLinks === 'always' && hasDynamicLink(node, linkAttribute));
175176
if (hasDangerousLink && !hasSecureRel(node, allowReferrer, warnOnSpreadAttributes, spreadAttributeIndex)) {
176-
report(context, messages.noTargetBlank, 'noTargetBlank', {
177+
const messageId = allowReferrer ? 'noTargetBlankWithoutNoopener' : 'noTargetBlankWithoutNoreferrer';
178+
report(context, messages[messageId], messageId, {
177179
node,
178180
fix(fixer) {
179181
// eslint 5 uses `node.attributes`; eslint 6+ uses `node.parent.attributes`
@@ -244,7 +246,8 @@ module.exports = {
244246
hasExternalLink(node, formAttribute)
245247
|| (enforceDynamicLinks === 'always' && hasDynamicLink(node, formAttribute))
246248
) {
247-
report(context, messages.noTargetBlank, 'noTargetBlank', {
249+
const messageId = allowReferrer ? 'noTargetBlankWithoutNoopener' : 'noTargetBlankWithoutNoreferrer';
250+
report(context, messages[messageId], messageId, {
248251
node
249252
});
250253
}

tests/lib/rules/jsx-no-target-blank.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ const parserOptions = {
2525
// ------------------------------------------------------------------------------
2626

2727
const ruleTester = new RuleTester({parserOptions});
28-
const defaultErrors = [{messageId: 'noTargetBlank'}];
28+
const defaultErrors = [{messageId: 'noTargetBlankWithoutNoreferrer'}];
2929

3030
ruleTester.run('jsx-no-target-blank', rule, {
3131
valid: [
@@ -249,7 +249,7 @@ ruleTester.run('jsx-no-target-blank', rule, {
249249
code: '<a href="http://example.com/20" target="_blank"></a>',
250250
output: '<a href="http://example.com/20" target="_blank" rel="noreferrer"></a>',
251251
options: [{allowReferrer: true}],
252-
errors: defaultErrors
252+
errors: [{messageId: 'noTargetBlankWithoutNoopener'}]
253253
},
254254
{
255255
code: '<a target="_blank" href={ dynamicLink }></a>',

0 commit comments

Comments
 (0)