Skip to content

Commit 49e4bb6

Browse files
jaabergljharb
authored andcommitted
[New] jsx-no-target-blank: add forms option
Fixes #1143.
1 parent 30ae98b commit 49e4bb6

File tree

6 files changed

+221
-63
lines changed

6 files changed

+221
-63
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
88
### Added
99
* [`jsx-no-useless-fragments`]: add option to allow single expressions in fragments ([#3006][] @mattdarveniza)
1010
* add [`prefer-exact-props`] rule ([#1547][] @jomasti)
11+
* [`jsx-no-target-blank`]: add `forms` option ([#1617][] @jaaberg)
1112

1213
### Fixed
1314
* component detection: use `estraverse` to improve component detection ([#2992][] @Wesitos)
@@ -32,6 +33,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
3233
[#2998]: https://github.com/yannickcr/eslint-plugin-react/pull/2998
3334
[#2994]: https://github.com/yannickcr/eslint-plugin-react/pull/2994
3435
[#2992]: https://github.com/yannickcr/eslint-plugin-react/pull/2992
36+
[#1617]: https://github.com/yannickcr/eslint-plugin-react/pull/1617
3537
[#1547]: https://github.com/yannickcr/eslint-plugin-react/pull/1547
3638

3739
## [7.24.0] - 2021.05.27

README.md

+5
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ You should also specify settings that will be shared across all the plugin rules
6464
{"property": "observer", "object": "Mobx"},
6565
{"property": "observer", "object": "<pragma>"} // sets `object` to whatever value `settings.react.pragma` is set to
6666
],
67+
"formComponents": [
68+
// Components used as alternatives to <form> for forms, eg. <Form endpoint={ url } />
69+
"CustomForm",
70+
{"name": "Form", "formAttribute": "endpoint"}
71+
]
6772
"linkComponents": [
6873
// Components used as alternatives to <a> for linking, eg. <Link to={ url } />
6974
"Hyperlink",

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

+35-7
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,28 @@ This rules requires that you accompany `target='_blank'` attributes with `rel='n
55

66
## Rule Details
77

8-
This rule aims to prevent user generated links from creating security vulnerabilities by requiring `rel='noreferrer'` for external links, and optionally any dynamically generated links.
8+
This rule aims to prevent user generated link hrefs and form actions from creating security vulnerabilities by requiring `rel='noreferrer'` for external link hrefs and form actions, and optionally any dynamically generated link hrefs and form actions.
99

1010
## Rule Options
11+
1112
```json
1213
...
13-
"react/jsx-no-target-blank": [<enabled>, { "allowReferrer": <allow-referrer>, "enforceDynamicLinks": <enforce> }]
14+
"react/jsx-no-target-blank": [<enabled>, {
15+
"allowReferrer": <allow-referrer>,
16+
"enforceDynamicLinks": <enforce>,
17+
"links": <boolean>,
18+
"forms": <boolean>,
19+
}]
1420
...
1521
```
1622

17-
* allow-referrer: optional boolean. If `true` does not require `noreferrer`. Defaults to `false`.
18-
* enabled: for enabling the rule. 0=off, 1=warn, 2=error. Defaults to 0.
19-
* enforceDynamicLinks: optional string, 'always' or 'never'
20-
* warnOnSpreadAttributes: optional boolean. Defaults to `false`.
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.
25+
* `enforceDynamicLinks`: optional string, 'always' or 'never'
26+
* `warnOnSpreadAttributes`: optional boolean. Defaults to `false`.
27+
* `enforceDynamicLinks` - enforce: optional string, 'always' or 'never'
28+
* `links` - Prevent usage of unsafe `target='_blank'` inside links, defaults to `true`
29+
* `forms` - Prevent usage of unsafe `target='_blank'` inside forms, defaults to `false`
2130

2231
### `enforceDynamicLinks`
2332

@@ -74,6 +83,20 @@ Defaults to false. If false, this rule will ignore all spread attributes. If tru
7483
<a {...unsafeProps} href="/some-page"></a>
7584
```
7685

86+
### `links` / `forms`
87+
88+
When option `forms` is set to `true`, the following is considered an error:
89+
90+
```jsx
91+
var Hello = <form target="_blank" action="http://example.com/"></form>;
92+
```
93+
94+
When option `links` is set to `true`, the following is considered an error:
95+
96+
```jsx
97+
var Hello = <a target='_blank' href="http://example.com/"></form>
98+
```
99+
77100
### Custom link components
78101

79102
This rule supports the ability to use custom components for links, such as `<Link />` which is popular in libraries like `react-router`, `next.js` and `gatsby`. To enable this, define your custom link components in the global [shared settings](https://github.com/yannickcr/eslint-plugin-react/blob/master/README.md#configuration) under the `linkComponents` configuration area. Once configured, this rule will check those components as if they were `<a />` elements.
@@ -94,9 +117,14 @@ var Hello = <Link target="_blank" to="/absolute/path/in/the/host"></Link>
94117
var Hello = <Link />
95118
```
96119

120+
### Custom form components
121+
122+
This rule supports the ability to use custom components for forms. To enable this, define your custom form components in the global [shared settings](https://github.com/yannickcr/eslint-plugin-react/blob/master/README.md#configuration) under the `formComponents` configuration area. Once configured, this rule will check those components as if they were `<form />` elements.
123+
97124
## When To Override It
125+
98126
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.
99127

100128
## When Not To Use It
101129

102-
If you do not have any external links, you can disable this rule.
130+
If you do not have any external links or forms, you can disable this rule.

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

+102-56
Original file line numberDiff line numberDiff line change
@@ -121,87 +121,133 @@ module.exports = {
121121
},
122122
warnOnSpreadAttributes: {
123123
type: 'boolean'
124+
},
125+
links: {
126+
type: 'boolean',
127+
default: true
128+
},
129+
forms: {
130+
type: 'boolean',
131+
default: false
124132
}
125133
},
126134
additionalProperties: false
127135
}]
128136
},
129137

130138
create(context) {
131-
const configuration = context.options[0] || {};
132-
const allowReferrer = configuration.allowReferrer || false;
133-
const warnOnSpreadAttributes = configuration.warnOnSpreadAttributes || false;
139+
const configuration = Object.assign(
140+
{
141+
allowReferrer: false,
142+
warnOnSpreadAttributes: false,
143+
links: true,
144+
forms: false
145+
},
146+
context.options[0]
147+
);
148+
const allowReferrer = configuration.allowReferrer;
149+
const warnOnSpreadAttributes = configuration.warnOnSpreadAttributes;
134150
const enforceDynamicLinks = configuration.enforceDynamicLinks || 'always';
135-
const components = linkComponentsUtil.getLinkComponents(context);
151+
const linkComponents = linkComponentsUtil.getLinkComponents(context);
152+
const formComponents = linkComponentsUtil.getFormComponents(context);
136153

137154
return {
138155
JSXOpeningElement(node) {
139-
if (!components.has(node.name.name)) {
140-
return;
141-
}
142-
143156
const targetIndex = findLastIndex(node.attributes, (attr) => attr.name && attr.name.name === 'target');
144157
const spreadAttributeIndex = findLastIndex(node.attributes, (attr) => (attr.type === 'JSXSpreadAttribute'));
145158

146-
if (!attributeValuePossiblyBlank(node.attributes[targetIndex])) {
147-
const hasSpread = spreadAttributeIndex >= 0;
159+
if (linkComponents.has(node.name.name)) {
160+
if (!attributeValuePossiblyBlank(node.attributes[targetIndex])) {
161+
const hasSpread = spreadAttributeIndex >= 0;
148162

149-
if (warnOnSpreadAttributes && hasSpread) {
150-
// continue to check below
151-
} else if ((hasSpread && targetIndex < spreadAttributeIndex) || !hasSpread || !warnOnSpreadAttributes) {
152-
return;
163+
if (warnOnSpreadAttributes && hasSpread) {
164+
// continue to check below
165+
} else if ((hasSpread && targetIndex < spreadAttributeIndex) || !hasSpread || !warnOnSpreadAttributes) {
166+
return;
167+
}
153168
}
154-
}
155169

156-
const linkAttribute = components.get(node.name.name);
157-
const hasDangerousLink = hasExternalLink(node, linkAttribute, warnOnSpreadAttributes, spreadAttributeIndex)
158-
|| (enforceDynamicLinks === 'always' && hasDynamicLink(node, linkAttribute));
159-
if (hasDangerousLink && !hasSecureRel(node, allowReferrer, warnOnSpreadAttributes, spreadAttributeIndex)) {
160-
context.report({
161-
node,
162-
messageId: 'noTargetBlank',
163-
fix(fixer) {
164-
// eslint 5 uses `node.attributes`; eslint 6+ uses `node.parent.attributes`
165-
const nodeWithAttrs = node.parent.attributes ? node.parent : node;
166-
// eslint 5 does not provide a `name` property on JSXSpreadElements
167-
const relAttribute = nodeWithAttrs.attributes.find((attr) => attr.name && attr.name.name === 'rel');
168-
169-
if (targetIndex < spreadAttributeIndex || (spreadAttributeIndex >= 0 && !relAttribute)) {
170-
return null;
171-
}
170+
const linkAttribute = linkComponents.get(node.name.name);
171+
const hasDangerousLink = hasExternalLink(node, linkAttribute, warnOnSpreadAttributes, spreadAttributeIndex)
172+
|| (enforceDynamicLinks === 'always' && hasDynamicLink(node, linkAttribute));
173+
if (hasDangerousLink && !hasSecureRel(node, allowReferrer, warnOnSpreadAttributes, spreadAttributeIndex)) {
174+
context.report({
175+
node,
176+
messageId: 'noTargetBlank',
177+
fix(fixer) {
178+
// eslint 5 uses `node.attributes`; eslint 6+ uses `node.parent.attributes`
179+
const nodeWithAttrs = node.parent.attributes ? node.parent : node;
180+
// eslint 5 does not provide a `name` property on JSXSpreadElements
181+
const relAttribute = nodeWithAttrs.attributes.find((attr) => attr.name && attr.name.name === 'rel');
182+
183+
if (targetIndex < spreadAttributeIndex || (spreadAttributeIndex >= 0 && !relAttribute)) {
184+
return null;
185+
}
172186

173-
if (!relAttribute) {
174-
return fixer.insertTextAfter(nodeWithAttrs.attributes.slice(-1)[0], ' rel="noreferrer"');
175-
}
187+
if (!relAttribute) {
188+
return fixer.insertTextAfter(nodeWithAttrs.attributes.slice(-1)[0], ' rel="noreferrer"');
189+
}
176190

177-
if (!relAttribute.value) {
178-
return fixer.insertTextAfter(relAttribute, '="noreferrer"');
179-
}
191+
if (!relAttribute.value) {
192+
return fixer.insertTextAfter(relAttribute, '="noreferrer"');
193+
}
180194

181-
if (relAttribute.value.type === 'Literal') {
182-
const parts = relAttribute.value.value
183-
.split('noreferrer')
184-
.filter(Boolean);
185-
return fixer.replaceText(relAttribute.value, `"${parts.concat('noreferrer').join(' ')}"`);
186-
}
195+
if (relAttribute.value.type === 'Literal') {
196+
const parts = relAttribute.value.value
197+
.split('noreferrer')
198+
.filter(Boolean);
199+
return fixer.replaceText(relAttribute.value, `"${parts.concat('noreferrer').join(' ')}"`);
200+
}
187201

188-
if (relAttribute.value.type === 'JSXExpressionContainer') {
189-
if (relAttribute.value.expression.type === 'Literal') {
190-
if (typeof relAttribute.value.expression.value === 'string') {
191-
const parts = relAttribute.value.expression.value
192-
.split('noreferrer')
193-
.filter(Boolean);
194-
return fixer.replaceText(relAttribute.value.expression, `"${parts.concat('noreferrer').join(' ')}"`);
202+
if (relAttribute.value.type === 'JSXExpressionContainer') {
203+
if (relAttribute.value.expression.type === 'Literal') {
204+
if (typeof relAttribute.value.expression.value === 'string') {
205+
const parts = relAttribute.value.expression.value
206+
.split('noreferrer')
207+
.filter(Boolean);
208+
return fixer.replaceText(relAttribute.value.expression, `"${parts.concat('noreferrer').join(' ')}"`);
209+
}
210+
211+
// for undefined, boolean, number, symbol, bigint, and null
212+
return fixer.replaceText(relAttribute.value, '"noreferrer"');
195213
}
196-
197-
// for undefined, boolean, number, symbol, bigint, and null
198-
return fixer.replaceText(relAttribute.value, '"noreferrer"');
199214
}
200-
}
201215

202-
return null;
216+
return null;
217+
}
218+
});
219+
}
220+
}
221+
if (formComponents.has(node.name.name)) {
222+
if (!attributeValuePossiblyBlank(node.attributes[targetIndex])) {
223+
const hasSpread = spreadAttributeIndex >= 0;
224+
225+
if (warnOnSpreadAttributes && hasSpread) {
226+
// continue to check below
227+
} else if (
228+
(hasSpread && targetIndex < spreadAttributeIndex)
229+
|| !hasSpread
230+
|| !warnOnSpreadAttributes
231+
) {
232+
return;
203233
}
204-
});
234+
}
235+
236+
if (!configuration.forms || hasSecureRel(node)) {
237+
return;
238+
}
239+
240+
const formAttribute = formComponents.get(node.name.name);
241+
242+
if (
243+
hasExternalLink(node, formAttribute)
244+
|| (enforceDynamicLinks === 'always' && hasDynamicLink(node, formAttribute))
245+
) {
246+
context.report({
247+
node,
248+
messageId: 'noTargetBlank'
249+
});
250+
}
205251
}
206252
}
207253
};

lib/util/linkComponents.js

+19
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,24 @@
99
const DEFAULT_LINK_COMPONENTS = ['a'];
1010
const DEFAULT_LINK_ATTRIBUTE = 'href';
1111

12+
/** TODO: type {(string | { name: string, formAttribute: string })[]} */
13+
/** @type {any} */
14+
const DEFAULT_FORM_COMPONENTS = ['form'];
15+
const DEFAULT_FORM_ATTRIBUTE = 'action';
16+
17+
function getFormComponents(context) {
18+
const settings = context.settings || {};
19+
const formComponents = /** @type {typeof DEFAULT_FORM_COMPONENTS} */ (
20+
DEFAULT_FORM_COMPONENTS.concat(settings.formComponents || [])
21+
);
22+
return new Map(formComponents.map((value) => {
23+
if (typeof value === 'string') {
24+
return [value, DEFAULT_FORM_ATTRIBUTE];
25+
}
26+
return [value.name, value.formAttribute];
27+
}));
28+
}
29+
1230
function getLinkComponents(context) {
1331
const settings = context.settings || {};
1432
const linkComponents = /** @type {typeof DEFAULT_LINK_COMPONENTS} */ (
@@ -23,5 +41,6 @@ function getLinkComponents(context) {
2341
}
2442

2543
module.exports = {
44+
getFormComponents,
2645
getLinkComponents
2746
};

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

+58
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,26 @@ ruleTester.run('jsx-no-target-blank', rule, {
115115
},
116116
{
117117
code: '<a href="some-link" target="some-non-blank-target" {...otherProps}></a>'
118+
},
119+
{
120+
code: '<a target="_blank" href="/absolute/path"></a>',
121+
options: [{forms: false}]
122+
},
123+
{
124+
code: '<a target="_blank" href="/absolute/path"></a>',
125+
options: [{forms: false, links: true}]
126+
},
127+
{
128+
code: '<form action="http://example.com" target="_blank"></form>',
129+
options: []
130+
},
131+
{
132+
code: '<form action="http://example.com" target="_blank" rel="noopener noreferrer"></form>',
133+
options: [{forms: true}]
134+
},
135+
{
136+
code: '<form action="http://example.com" target="_blank" rel="noopener noreferrer"></form>',
137+
options: [{forms: true, links: false}]
118138
}
119139
],
120140
invalid: [
@@ -286,6 +306,44 @@ ruleTester.run('jsx-no-target-blank', rule, {
286306
options: [{
287307
warnOnSpreadAttributes: true
288308
}]
309+
},
310+
{
311+
code: '<a target="_blank" href="//example.com" rel></a>',
312+
output: '<a target="_blank" href="//example.com" rel="noreferrer"></a>',
313+
options: [{links: true}],
314+
errors: defaultErrors
315+
},
316+
{
317+
code: '<a target="_blank" href="//example.com" rel></a>',
318+
output: '<a target="_blank" href="//example.com" rel="noreferrer"></a>',
319+
options: [{links: true, forms: true}],
320+
errors: defaultErrors
321+
},
322+
{
323+
code: '<a target="_blank" href="//example.com" rel></a>',
324+
output: '<a target="_blank" href="//example.com" rel="noreferrer"></a>',
325+
options: [{links: true, forms: false}],
326+
errors: defaultErrors
327+
},
328+
{
329+
code: '<form method="POST" action="http://example.com" target="_blank"></form>',
330+
options: [{forms: true}],
331+
errors: defaultErrors
332+
},
333+
{
334+
code: '<form method="POST" action="http://example.com" rel="" target="_blank"></form>',
335+
options: [{forms: true}],
336+
errors: defaultErrors
337+
},
338+
{
339+
code: '<form method="POST" action="http://example.com" rel="noopenernoreferrer" target="_blank"></form>',
340+
options: [{forms: true}],
341+
errors: defaultErrors
342+
},
343+
{
344+
code: '<form method="POST" action="http://example.com" rel="noopenernoreferrer" target="_blank"></form>',
345+
options: [{forms: true, links: false}],
346+
errors: defaultErrors
289347
}
290348
]
291349
});

0 commit comments

Comments
 (0)